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

Drop -Y flags that change language semantics #6505

Merged
merged 1 commit into from May 4, 2018

Conversation

Projects
None yet
@adriaanm
Copy link
Member

commented Apr 9, 2018

removes:

  • -Yno-adapted-args
  • -Xstrict-inference
  • -Xfull-lubs
  • -Yoverride-objects
  • -Yoverride-vars
  • -Yinfer-argument-types
  • -Yvirtpatmat

See scala/scala-dev#430

This is the first cut. Open for discussion.

@scala-jenkins scala-jenkins added this to the 2.13.0-M4 milestone Apr 9, 2018

@adriaanm

This comment has been minimized.

Copy link
Member Author

commented Apr 10, 2018

The last commit failed, largely because of check file changes: https://scala-ci.typesafe.com/job/scala-2.13.x-validate-main/741/testReport/ (it's sooooo nice to have a page like this to link to!)

My understanding is that we should put the FINAL bit back on objects, since its removal was a half-finished change to support the -Yoverride-objects flag.

@@ -203,7 +199,6 @@ trait ScalaSettings extends AbsScalaSettings
val Ynogenericsig = BooleanSetting ("-Yno-generic-signatures", "Suppress generation of generic signatures for Java.")
val noimports = BooleanSetting ("-Yno-imports", "Compile without importing scala.*, java.lang.*, or Predef.")
val nopredef = BooleanSetting ("-Yno-predef", "Compile without importing Predef.")
val noAdaptedArgs = BooleanSetting ("-Yno-adapted-args", "Do not adapt an argument list (either by inserting () or creating a tuple) to match the receiver.")

This comment has been minimized.

Copy link
@eed3si9n

eed3si9n Apr 10, 2018

Member

Kind of sad to see this one go. I only learned about it recently, but I thought it was a good cop.

This comment has been minimized.

Copy link
@adriaanm

adriaanm Apr 10, 2018

Author Member

This PR is part provocation to hear any passionate defenses of the defendants :-)

This comment has been minimized.

Copy link
@milessabin

milessabin Apr 10, 2018

Contributor

I'm quite happy to see that one go. As Scala sharp edges go it's one of the least harmful (you're unlikely to silently miscompile because of it) and most useful (see various bits of arity-polymorphic sugar in shapeless and thereabouts).

This comment has been minimized.

Copy link
@eed3si9n

eed3si9n Apr 10, 2018

Member

Here's an easy example of things compiling:

scala> case class W[A](elem: A)(implicit ordering: Ordering[A])
defined class W

scala> W(1, 2, 3, 4)
res0: W[(Int, Int, Int, Int)] = W((1,2,3,4))

And to quote @lrytz in scala/bug#8342:

Auto-tupling is not mentioned at all in the spec.

Here's another voice from the wild:

AGGGHGHGHGHGHGHGHGHGHGHGHGH!
Scala auto-tupling. Why on earth does this “feature” even exist?

To which @tpolecat responds "-Yno-adapted-args."

I am not so much for or against the removal of -Yno-adapted-args per se, but I'd request a more principled solution toward this general problem. Adaptations.scala is already riddled with flags (settings.future, settings.noAdaptedArgs, settings.warnAdaptedArgs).

option 1

Here's a proposal. Remove zero-arg eta expansion in 2.13 (or under -Xsource:2.14), and also remove auto-tupling under -Xsource:2.14 if Dotty can go along with it.

This comment has been minimized.

Copy link
@adriaanm

adriaanm Apr 10, 2018

Author Member

Here's a proposal. Remove zero-arg eta expansion in 2.13

Yup: #6475

remove auto-tupling

Would be nice...

This comment has been minimized.

Copy link
@adriaanm

adriaanm Apr 10, 2018

Author Member

In any case, our guiding principle is alignment with Dotty, so, looks like we can at least deprecate auto-tupling in most cases in 2.13 (drop it in 2.14), and implement the same approach as dotty by 2.14.

This comment has been minimized.

Copy link
@joroKr21

joroKr21 Apr 10, 2018

Contributor

Dotty still has the regular auto tupling which I don't care much about. But the function parameter tupling is something different that doesn't exist in scalac and I find it quite useful.

This comment has been minimized.

Copy link
@adriaanm

adriaanm Apr 10, 2018

Author Member

Ah, thanks for the correction. Wasn't aware they still had auto-tupling :-( I would really like to drop that, while I agree the one for function literals seems useful.

This comment has been minimized.

Copy link
@eed3si9n

eed3si9n Apr 10, 2018

Member

In an effort for reconciliation, I've opened a thread here https://contributors.scala-lang.org/t/lets-drop-auto-tupling/1799, please comment and subscribe.

This comment has been minimized.

Copy link
@milessabin

milessabin Apr 11, 2018

Contributor

@eed3si9n sure, you can construct examples of that sort. It's a lot harder to find examples where that's a problem in actual practice. They exist, but they're rare.

@puffnfresh

This comment has been minimized.

Copy link
Member

commented Apr 10, 2018

-Yno-adapted-args is very important to the projects I work on in Atlassian.

-Xfuture also changes semantics (e.g. the withFilter fallback) in a very useful way. Will it also be removed?

@adriaanm

This comment has been minimized.

Copy link
Member Author

commented Apr 10, 2018

I'm not for auto-tupling (see above), but I'm against flags that fragment the language. They cause confusion, reduce test coverage, complicate code,... The intent is to keep only -Xsource, to have the compiler do its best to emulate one version in the past or future (which subsumes -Xfuture).

I'd like to understand both your use cases better so we can see about supporting them by default. If we can convince the dotty folks to drop auto-tupling, I'd be happy to deprecate in 2.13 and remove in 2.14. withFilter is already the only supported method for desugaring if in a for in 2.12 -- the fallback to filter has been removed (#5252).

@szeiger szeiger modified the milestones: 2.13.0-M4, 2.13.0-M5 May 2, 2018

Drop flags that change language semantics
To reduce the many possible combination of these flags,
the only supported mechanism that affects the outcome of
type checking is `-Xsource`, which instructs the compiler
to mimic the given version as much as possible. We only
support this for one prior and one subsequent version
(e.g., 2.13 recognizes 2.12 and 2.14 source levels).

The following flags have been dropped:
  - `-Xfull-lubs`
  - `-Xstrict-inference`
  - `-Yinfer-argument-types`
  - `-Yno-adapted-args`
  - `-Yoverride-objects`
  - `-Yoverride-vars`
  - `-Yvirtpatmat`

We're exceptionally dropping a few obscure `-X` flags
without prior deprecation. `-Y` flags are always at our whim.

@adriaanm adriaanm force-pushed the adriaanm:drop-why-flags branch from 0c6d24f to a82a56e May 3, 2018

@adriaanm

This comment has been minimized.

Copy link
Member Author

commented May 3, 2018

I've dropped the commit about restoring the acc_final flag for nested objects and rebased (will create a ticket). Would like to consider merging this for M4. (See scala/scala-dev#497)

@adriaanm adriaanm modified the milestones: 2.13.0-M5, 2.13.0-M4 May 3, 2018

@dwijnand

This comment has been minimized.

Copy link
Member

commented May 3, 2018

Does it makes sense, for now, to retain the auto-tupling flag, given there's at least some interest by Martin in dropping it in Scala 3?

@adriaanm

This comment has been minimized.

Copy link
Member Author

commented May 3, 2018

Once we figure out the deprecation story, we'll make it (the deprecation path) available under -Xsource and -deprecation.

@SethTisue

This comment has been minimized.

Copy link
Member

commented May 3, 2018

for the record, I'm confident -Xstrict-inference can go, I did due diligence on it over at #6593 before Adriaan pointed out it the removal is included here already.

@adriaanm

This comment has been minimized.

Copy link
Member Author

commented May 4, 2018

I note your hesitation on auto-tupling, @dwijnand. I'd like for us to accept slight regressions in the process of cleaning this up. I've logged this as scala/scala-dev#496.

@adriaanm adriaanm merged commit 926e8f9 into scala:2.13.x May 4, 2018

2 checks passed

cla @adriaanm signed the Scala CLA. Thanks!
Details
validate-main [1170] SUCCESS. Took 36 min.
Details
@som-snytt

This comment has been minimized.

Copy link
Contributor

commented May 5, 2018

Now all my PRs with new -Y options have merge conflicts.

I only just noticed that the letter Y represents a fork and is really the best symbol for options that change semantics.

Similarly, -X should be reserved for chiasmus options that invert the meaning of your code.

@SethTisue

This comment has been minimized.

Copy link
Member

commented Aug 24, 2018

I'm (still) confused why -Yno-adapted-args got the axe. I've opened a separate ticket on it; let's discuss there: scala/bug#11110

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.