Make Using lazy #7415

NthPortal · 2018-11-11T21:47:45Z

Make Using lazy, allowing map and filter to be
called before the resource is released.

NthPortal · 2018-11-11T21:48:33Z

Bikeshedding welcome on better names for class Using and its use() method.

Possible other names for class Using (i.e. the return type of Using.apply):

Usage (h/t @dwijnand)
ResourceUse

Possible other names for use():

run()
consume() (h/t @dwijnand)

dwijnand · 2018-11-11T22:14:35Z

👨‍🎨 Maybe scala-arm's ManagedResource? 🤔

julienrf · 2018-11-11T22:12:52Z

src/library/scala/util/Using.scala

+      val res = op(r)
+      if (p(res)) res
+      else throw new NoSuchElementException("Predicate does not hold for " + res)
+    })


This is not really related to this PR, but is it a good idea to have filter if it throws an exception? I think we should not have it.

It seems relatively related to this PR to me.

The question is, if not NoSuchElementException, what does the eventual Try contain?

(Side note: I tried out an optimisation where it uses a cached ControlThrowable initially, and then only turns it into a NoSuchElementException if you ask for a Try back. If you call foreach, it just catches the cached throwable and does nothing. Other than being a bit confusing from a maintenance perspective, I didn't see any downsides.)

The question is, if not NoSuchElementException, what does the eventual Try contain?

Any exception that can be thrown here:

scala/src/library/scala/util/Using.scala

Lines 181 to 217 in 56c45cd

def resource[R: Resource, A](resource: R)(body: R => A): A = {

if (resource == null) throw new NullPointerException("null resource")

@inline def safeAddSuppressed(t: Throwable, suppressed: Throwable): Unit = {

// don't `addSuppressed` to something which is a `ControlThrowable`

if (!t.isInstanceOf[ControlThrowable]) t.addSuppressed(suppressed)

}

var primary: Throwable = null

try {

body(resource)

} catch {

case t: Throwable =>

primary = t

null.asInstanceOf[A] // compiler doesn't know `finally` will throw

} finally {

if (primary eq null) implicitly[Resource[R]].release(resource)

else {

var toThrow = primary

try {

implicitly[Resource[R]].release(resource)

} catch {

case other: Throwable =>

if (NonFatal(primary) && !NonFatal(other)) {

// `other` is fatal, `primary` is not

toThrow = other

safeAddSuppressed(other, primary)

} else {

// `toThrow` is already `primary`

safeAddSuppressed(primary, other)

}

} finally {

throw toThrow

}

}

}

}

I'm fine with removing filter. If we decide it's a good idea later, it can always be added back.

He-Pin · 2018-11-12T21:45:14Z

@NthPortal useAndRelease？

使用 FastHub 从我的 Galaxy Note8 发送

Make Using lazy, allowing `map` and `filter` to be called before the resource is released.

NthPortal · 2018-11-18T20:02:11Z

@dwijnand I'm happy with ManagedResource as long as @jsuereth is okay with me plagiarising the name ;)

NthPortal · 2018-11-18T20:03:45Z

While we're repainting the shed, thoughts on renaming Using.Resource to Using.Release or Using.Releaser?

som-snytt · 2018-11-18T20:42:51Z

If you called it Usage, you could use Josh's line from arm:

For more information on usage, see Usage.

Ichoran · 2018-11-19T20:45:24Z

I always prefer the loan pattern to programmer-implements-linear-types-in-their-head. So I'm not a fan.

(Loan pattern would look like

Using.Manager{ use =>
  for {
    a <- use(foo)
    b <- use(bar)
    c <- use(baz)
  } yield bippy(a, b, quux(c))
}

You can screw this up too by assigning intermediates to some mutable value, but it isn't nearly so error-prone for people who aren't used to lazy evaluation.

NthPortal · 2018-11-22T21:05:05Z

@Ichoran in your example, all the resources get released at the Using.Manager block, correct? similar to the Scope described by @densh in this Scala World talk (gist)?

I'm not opposed to that, though I'm curious as to others' thoughts on whether this is slowly exceeding the scope of what should be in the standard library. Originally, this was just meant to be a simple Try wrapper around Using.resource.

NthPortal · 2018-11-22T23:35:37Z

@Ichoran once it's managed externally, is there any reason for use(foo) to return a monad-ish thing and not just return the resource?

Ichoran · 2018-11-24T09:47:58Z

@NthPortal - Without coding it up I can't be sure, but I don't think the monadish thing is needed any longer.

NthPortal · 2018-11-27T07:25:31Z

Superseded by #7468

NthPortal requested a review from Ichoran November 11, 2018 21:47

scala-jenkins added this to the 2.13.0-RC1 milestone Nov 11, 2018

julienrf reviewed Nov 11, 2018

View reviewed changes

NthPortal mentioned this pull request Nov 18, 2018

Remove Using#map and Using#flatMap #7389

Closed

NthPortal force-pushed the topic/using-lazy/PR1 branch 3 times, most recently from 25ab5f9 to 5800501 Compare November 18, 2018 19:37

Make Using lazy

5bd2aae

Make Using lazy, allowing `map` and `filter` to be called before the resource is released.

NthPortal force-pushed the topic/using-lazy/PR1 branch from c912b95 to 5bd2aae Compare November 18, 2018 19:41

[squash] fix visibility

0508a5c

NthPortal mentioned this pull request Nov 27, 2018

Change Using to implement the loan pattern #7468

Merged

NthPortal closed this Nov 27, 2018

SethTisue removed this from the 2.13.0-RC1 milestone Nov 27, 2018

NthPortal deleted the topic/using-lazy/PR1 branch December 3, 2018 05:38

	def resource[R: Resource, A](resource: R)(body: R => A): A = {
	if (resource == null) throw new NullPointerException("null resource")

	@inline def safeAddSuppressed(t: Throwable, suppressed: Throwable): Unit = {
	// don't `addSuppressed` to something which is a `ControlThrowable`
	if (!t.isInstanceOf[ControlThrowable]) t.addSuppressed(suppressed)
	}

	var primary: Throwable = null
	try {
	body(resource)
	} catch {
	case t: Throwable =>
	primary = t
	null.asInstanceOf[A] // compiler doesn't know `finally` will throw
	} finally {
	if (primary eq null) implicitly[Resource[R]].release(resource)
	else {
	var toThrow = primary
	try {
	implicitly[Resource[R]].release(resource)
	} catch {
	case other: Throwable =>
	if (NonFatal(primary) && !NonFatal(other)) {
	// `other` is fatal, `primary` is not
	toThrow = other
	safeAddSuppressed(other, primary)
	} else {
	// `toThrow` is already `primary`
	safeAddSuppressed(primary, other)
	}
	} finally {
	throw toThrow
	}
	}
	}
	}

Make Using lazy #7415

Make Using lazy #7415

Uh oh!

Conversation

NthPortal commented Nov 11, 2018

Uh oh!

NthPortal commented Nov 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dwijnand commented Nov 11, 2018

Uh oh!

julienrf Nov 11, 2018

Choose a reason for hiding this comment

Uh oh!

NthPortal Nov 11, 2018

Choose a reason for hiding this comment

Uh oh!

julienrf Nov 12, 2018

Choose a reason for hiding this comment

Uh oh!

NthPortal Nov 18, 2018

Choose a reason for hiding this comment

Uh oh!

He-Pin commented Nov 12, 2018

Uh oh!

NthPortal commented Nov 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NthPortal commented Nov 18, 2018

Uh oh!

som-snytt commented Nov 18, 2018

Uh oh!

Ichoran commented Nov 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NthPortal commented Nov 22, 2018

Uh oh!

NthPortal commented Nov 22, 2018

Uh oh!

Ichoran commented Nov 24, 2018

Uh oh!

NthPortal commented Nov 27, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

NthPortal commented Nov 11, 2018 •

edited

Loading

NthPortal commented Nov 18, 2018 •

edited

Loading

Ichoran commented Nov 19, 2018 •

edited

Loading