Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eta-expand 0-arity method if expected type is Function0 #7660

Merged
merged 3 commits into from
Feb 11, 2019

Conversation

adriaanm
Copy link
Contributor

@adriaanm adriaanm commented Jan 17, 2019

Reverse course from the deprecation introduced in 2.12.

(4.3) condition for eta-expansion by -Xsource level:

  • until 2.13:
  • 2.14:
    • for arity > 0: unconditional
    • for arity == 0: a function-ish type of arity 0 is expected (including SAM)

warnings:

  • 2.12: eta-expansion of zero-arg methods was deprecated (eta expansion should not precede empty application  bug#7187)
  • 2.13: deprecation dropped in favor of setting the scene for uniform eta-expansion in 3.0
    warning still available under -Xlint:eta-zero
  • 2.14: expected type is a SAM that is not annotated with @FunctionalInterface
    if it's a Java-defined interface (under -Xlint:eta-sam)

(4.2) condition for auto-application by -Xsource level:

  • until 2.14: none (assuming condition for (4.3) was not met)

  • in 3.0: meth.isJavaDefined
    TODO decide -- currently the condition is more involved to give slack to
    Scala methods overriding Java-defined ones; I think we should resolve that
    by introducing slack in overriding e.g. a Java-defined def toString()
    by a Scala-defined def toString. This also works better for dealing
    with accessors overriding Java-defined methods.

    The current strategy in methodSig is problematic:

    // Add a () parameter section if this overrides some method with () parameters
    val vparamSymssOrEmptyParamsFromOverride =

    This means an accessor that overrides a Java-defined method gets a
    MethodType instead of a NullaryMethodType, which breaks lots of
    assumptions about accessors

@dwijnand
Copy link
Member

Do you need to redo #7614 too?

@adriaanm
Copy link
Contributor Author

Yep, thanks!

@som-snytt
Copy link
Contributor

The PR to undo is the one where -Ymoors undoes the other PRs and forks the language.

@adriaanm
Copy link
Contributor Author

PRs to implement -Yunmoorsed welcome :-p

@adriaanm
Copy link
Contributor Author

adriaanm commented Feb 4, 2019

Interesting issues in future-spec -- the other two test fails are due to a bug this introduces under -Xsource:3.0

@adriaanm

This comment has been minimized.

@adriaanm adriaanm force-pushed the eta-zero branch 3 times, most recently from 8b47517 to 0da9315 Compare February 5, 2019 20:51
@adriaanm adriaanm assigned retronym and unassigned adriaanm Feb 6, 2019
@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Feb 7, 2019
Reverse course from the deprecation introduced in 2.12.

(4.3) condition for eta-expansion by -Xsource level:

  - until 2.13:
    - for arity > 0: function or sam type is expected
    - for arity == 0: Function0 is expected -- SAM types do not eta-expand
      because it could be an accidental SAM scala/bug#9489
  - 2.14:
    - for arity > 0: unconditional
    - for arity == 0: a function-ish type of arity 0 is expected (including SAM)

warnings:
  - 2.12: eta-expansion of zero-arg methods was deprecated (scala/bug#7187)
  - 2.13: deprecation dropped in favor of setting the scene for uniform eta-expansion in 3.0
          warning still available under -Xlint:eta-zero
  - 2.14: expected type is a SAM that is not annotated with `@FunctionalInterface`
          if it's a Java-defined interface (under `-Xlint:eta-sam`)

(4.2) condition for auto-application by -Xsource level:

  - until 2.14: none (assuming condition for (4.3) was not met)
  - in 3.0: `meth.isJavaDefined`
    TODO decide -- currently the condition is more involved to give slack to
    Scala methods overriding Java-defined ones; I think we should resolve that
    by introducing slack in overriding e.g. a Java-defined `def toString()`
    by a Scala-defined `def toString`. This also works better for dealing
    with accessors overriding Java-defined methods.

    The current strategy in methodSig is problematic:
    > // Add a () parameter section if this overrides some method with () parameters
    > val vparamSymssOrEmptyParamsFromOverride =

    This means an accessor that overrides a Java-defined method gets a
    MethodType instead of a `NullaryMethodType`, which breaks lots of
    assumptions about accessors
@adriaanm

This comment has been minimized.

@adriaanm
Copy link
Contributor Author

adriaanm commented Feb 10, 2019

I think I addressed all review comments.

@adriaanm adriaanm merged commit dbf7081 into scala:2.13.x Feb 11, 2019
@SethTisue
Copy link
Member

@adriaanm we should un-deprecate in 2.12.9, then?

@adriaanm
Copy link
Contributor Author

Good point: #7740

SethTisue added a commit to scalacommunitybuild/scalaz that referenced this pull request Feb 26, 2019
needed in recent 2.13 nightlies because of the combination of
scala/scala#7660 and scala/scala#7263
@som-snytt
Copy link
Contributor

It warns on usual usages of varianceInType. I'm not sure if the code comment is about whether to require the annotation, or whether it's too annoying to warn.

Implicits.scala:701:78: Eta-expansion performed to meet expected type scala.tools.nsc.Variance.Extractor[Implicits.this.global.Symbol], which is SAM-equivalent to Implicits.this.global.Symbol => scala.reflect.internal.Variance,
[warn] even though trait Extractor is not annotated with `@FunctionalInterface`;
[warn] to suppress warning, add the annotation or write out the equivalent function literal.
[warn]                 val targs = solvedTypes(tvars, allUndetparams, varianceInType(wildPt), upper = false, lubDepth(tpInstantiated :: wildPt :: Nil))

eed3si9n added a commit to eed3si9n/scala that referenced this pull request Jun 2, 2020
Ref scala bug 12006

scalagh-7660 added `-Xsource:2.14` mode that eta-expands on SAM types. This confuses `URL#openStream` to be treated as SAM when `Function0[InputStream]` is expected, and get a weird error message:

       error: type mismatch;
        found   : java.io.InputStream
        required: Int
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
6 participants