Skip to content

Conversation

@adriaanm
Copy link
Contributor

@adriaanm adriaanm commented Jul 28, 2016

For backwards compatiblity with 2.11, we already
don't adapt a zero-arg method value to a SAM.
In 2.13, we won't do any eta-expansion for zero-arg method values,
but we should deprecate first.

JIRA: https://issues.scala-lang.org/browse/SI-7187

@adriaanm
Copy link
Contributor Author

The tests are not passing! Looks like there's an issue with sbt reporting a failed run to jenkins.

@adriaanm
Copy link
Contributor Author

So, are we okay deprecating this fine piece of code?

    def inverter(input: Wire, output: Wire): Unit = {
      def invertAction() = {
        val inputSig = input.getSignal;
        afterDelay(InverterDelay) {() => output.setSignal(!inputSig) };
      }
      input addAction invertAction
    }

adriaanm added 2 commits July 29, 2016 12:23
... and not grep's exit code, which would mean the test suite's
result is determined by whether grep fails or not,
instead of partest/junit's hard work
For backwards compatiblity with 2.11, we already
don't adapt a zero-arg method value to a SAM.
In 2.13, we won't do any eta-expansion for zero-arg method values,
but we should deprecate first.
@adriaanm adriaanm changed the title SI-7187 deprecate eta-expansion of zero-arg method values SI-7187 deprecate eta-expansion of zero-arg method values [ci: last-only] Jul 29, 2016
@adriaanm adriaanm modified the milestone: 2.12.0-RC1 Jul 30, 2016
@lrytz
Copy link
Member

lrytz commented Aug 8, 2016

not sure.. the example doesn't look bad.

i guess the goal is to fix some other examples that are unwanted / trip up users. can we narrow them down more?

@szeiger
Copy link
Contributor

szeiger commented Aug 8, 2016

I assume addAction takes an argument of type () => Unit. It could use a by-name parameter => Unit instead. Considering that both do pretty much the same and the former already doesn't work with SAMs, I think it makes sense to deprecate it.

@lrytz
Copy link
Member

lrytz commented Aug 9, 2016

Do you mean to deprecate () => Unit generally?

@adriaanm
Copy link
Contributor Author

adriaanm commented Aug 9, 2016

No, I meant eta-expanding to that function type (and others with an empty argument list) :-)

@lrytz
Copy link
Member

lrytz commented Aug 9, 2016

I was referring to stefan's comment above mine - maybe I misunderstood.

@adriaanm
Copy link
Contributor Author

adriaanm commented Aug 9, 2016

We clearly need more threading :-)

@szeiger
Copy link
Contributor

szeiger commented Aug 9, 2016

No, only eta-expanding these types. I generally use types of the form () => T instead of => T when I want explicit control over when they are evaluated. It's useful to have the type system enforce that.

@lrytz
Copy link
Member

lrytz commented Aug 9, 2016

For me, I would naturally define addAction with an argument type () => Unit. By-name parameters can also be dangerous, recent example: #5157 (comment)

@szeiger
Copy link
Contributor

szeiger commented Aug 9, 2016

Yes, this is probably one of those cases where you want to be explicit. So you'd have to write:

def inverter(input: Wire, output: Wire): Unit = {
  def invertAction() = {
    val inputSig = input.getSignal;
    afterDelay(InverterDelay) {() => output.setSignal(!inputSig) };
  }
  input.addAction(invertAction _)
}

I think I'd prefer this version:

def inverter(input: Wire, output: Wire): Unit = {
  val invertAction = () => {
    val inputSig = input.getSignal;
    afterDelay(InverterDelay) {() => output.setSignal(!inputSig) };
  }
  input addAction invertAction
}

Both of these should still be allowed, right?

@lrytz
Copy link
Member

lrytz commented Aug 9, 2016

right. that is probably a fair price to pay. didn't we also have recent discussions about deprecating explicit eta-expansion through (invertAction _)?

@adriaanm
Copy link
Contributor Author

adriaanm commented Aug 9, 2016

Thanks for making my question more explicit, Stefan. I would agree the val invertAction more clearly conveys the intent here. Certainly, that one should be allowed. Regarding explicit eta-conversion, I don't remember a discussion on deprecating that one. I would think it's fine to write invertAction _ to mean () => invertAction()?

@lrytz
Copy link
Member

lrytz commented Aug 10, 2016

OK, I read through the ticket and some more history, I agree we should get this deprecation in.

The current patch also warns on explicit eta-expansion:

scala> def f(): Unit = {}
f: ()Unit

scala> f _
<console>:13: warning: Eta-expansion of zero-argument method values is deprecated. Did you intend to write f()?
       f _
       ^
res0: () => Unit = $$Lambda$1718/1023344953@7c29adc8

@lrytz
Copy link
Member

lrytz commented Aug 10, 2016

added distinction between f and f _ in #5327

@lrytz lrytz closed this Aug 10, 2016
@SethTisue SethTisue removed this from the 2.12.0-RC1 milestone Sep 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants