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

reconsider removal of -Yno-adapted-args in 2.13 #11110

Closed
SethTisue opened this issue Aug 24, 2018 · 21 comments
Closed

reconsider removal of -Yno-adapted-args in 2.13 #11110

SethTisue opened this issue Aug 24, 2018 · 21 comments
Assignees
Milestone

Comments

@SethTisue
Copy link
Member

SethTisue commented Aug 24, 2018

I'm confused why -Yno-adapted-args got the axe (in scala/scala#6505, under the umbrella of
scala/scala-dev#430)

might this have been overzealous? -Yno-adapted-args is both widely used and useful IMO

and it only causes "language fragmentation" in the sense that it makes some code — code that has been producing compiler warnings since Scala 2.11 (EDIT: under -Xlint, that is, not by default) — give a hard error instead of a warning

if we were to get rid of argument adaptation entirely, as I expect to happen in Scala 2.14 and/or Scala 3, then it will make sense to remove the flag at that time. but I don't see that it makes sense to remove now

@Jasper-M
Copy link

and it only causes "language fragmentation" in the sense that it makes some code — code that has been producing compiler warnings since Scala 2.11 — give a hard error instead of a warning

There doesn't seem to be a warning without the flag, though.

@hrhino
Copy link

hrhino commented Aug 27, 2018

You need to use -Ywarn-adapted-args for the warning, which apparently made the cut, so that and -Xfatal-warnings have a similar effect in conjunction.

That said, I always turn on -Yno-adapted-args in my project, and I'll be sad to see it go. (Isn't there talk of removing it completely in Dotty?)

@dwijnand
Copy link
Member

Duplicate of scala/scala-dev#496, I'd say.

@SethTisue
Copy link
Member Author

SethTisue commented Aug 27, 2018

(Isn't there talk of removing it completely in Dotty?)

yes, this "feature" is on its slow way out of existence.

You need to use -Ywarn-adapted-args for the warning

ah, right, thanks for the correction. note that this warning has been part of -Xlint since 2.11, though, so it hasn't been only behind that one obscure flag.

Duplicate of scala/scala-dev#496, I'd say.

I see your point, but that one's targeted for 2.13.1. I opened this ticket specifically to ask what we're going to do for 2.13.0-RC1/2.13.0, since changes have already been made in this area in the milestones.

@dwijnand
Copy link
Member

I think that one should target RC1.

@adriaanm
Copy link
Contributor

adriaanm commented Jan 8, 2019

For migration to dotty, we can adjust adaption use -Xsource. Since we're out of time for RC1, re-scheduling for 2.13.1. If someone manages to fix this for the RC1, that's fine too, but we won't hold up the RCs for this.

@adriaanm adriaanm modified the milestones: 2.13.0-RC1, 2.13.1 Jan 8, 2019
@SethTisue
Copy link
Member Author

@SethTisue SethTisue removed their assignment Mar 6, 2019
@szeiger
Copy link

szeiger commented Aug 28, 2019

@adriaanm Does it even make sense to revert this in 2.13.1 (or later) now that 2.13.0 shipped without the option?

@SethTisue
Copy link
Member Author

SethTisue commented Aug 28, 2019

(IMO yes. most open-source libraries are already on 2.13, but most Scala users aren't yet)

@adriaanm
Copy link
Contributor

As useful as this is, I don't want to regress on our policy, nor remove the pressure to actually implement the migration under -Xsource. For now, the warning + fatal warning will have to suffice.

@szeiger
Copy link

szeiger commented Aug 29, 2019

I don't follow. Are you proposing that certain source levels enable or disable the warnings automatically? If so, which ones?

@adriaanm
Copy link
Contributor

No, I wasn't thinking of the warnings. They are still available, right? I'm referring to the change to the adapt logic that actually prevented the insertion of () or tupling of args.

A "future" (i.e. 2.14 or 3.0) source level will implement (a refinement of) the policy implemented by -Yno-adapt-args, and I feel we should focus our energy on getting that done.

We're talking about not inserting () (except for java-defined methods) and not auto-tupling args (except for, erm, what were the conditions again? I guess only for cases where a Tuple was expected or where we know only a single argument is allowed, e.g. infix methods).

@szeiger
Copy link

szeiger commented Aug 29, 2019

So you're saying we should not restore -Yno-adapt-args on principle? Judging by the changes in scala/scala#6505 it should only take a few lines of code to get it back. Or maybe there were more changes in the meantime that would make this infeasible?

@adriaanm
Copy link
Contributor

You're right, it would not be a lot of work to bring back, but then there's less incentive to switch to the -Xsource mechanism, which provides a more coherent upgrade-prep experience.

@eed3si9n
Copy link
Member

There's apparently a single use case where auto-tupling is useful, so I suggested in https://contributors.scala-lang.org/t/lets-drop-auto-tupling/1799/14?u=eed3si9n that we create an opt-in marker called scala.reflect.ParamList[_]:

def foo[A](params: A): Unit = println(params.toString)
def bar[A: ParamList](params: A): Unit = println(params.toString)

so

foo(1, "hello", Option(3)) // won't compile
bar(1, "hello", Option(3)) // prints (1, "hello", Option(3))

@szeiger
Copy link

szeiger commented Aug 30, 2019

While we're still considering things, I'm rescheduling this for 2.13.2.

@szeiger szeiger modified the milestones: 2.13.1, 2.13.2 Aug 30, 2019
@SethTisue SethTisue self-assigned this Feb 6, 2020
@SethTisue SethTisue modified the milestones: 2.13.2, 2.13.3 Feb 6, 2020
@SethTisue
Copy link
Member Author

given that we still have -Ywarn-adapted-args, and now that we have configurable warnings, I think this qualifies as fixed

@SethTisue SethTisue modified the milestones: 2.13.3, 2.13.2 Apr 12, 2020
@yanns
Copy link

yanns commented Apr 17, 2020

I also wanted to have something like -Yno-adapted-args in scala 2.13.

given that we still have  -Ywarn-adapted-args

For me it does not work:

[error] bad option: '-Ywarn-adapted-args'

The new configurable warning convention does not work:

[error] bad option: '-Wadapted-args'

I could use -Xlint:adapted-args

@SethTisue
Copy link
Member Author

SethTisue commented Apr 17, 2020

I could use -Xlint:adapted-args

right. thank you for the correction

but note also that it's included in -Xlint's defaults, so it doesn't normally need to be individually enabled:

Welcome to Scala 2.13.2-bin-a789d81 (OpenJDK 64-Bit Server VM, Java 1.8.0_242).
scala> println(1, 2)
(1,2)
scala> :replay -Xlint
replay> println(1, 2)
               ^
        warning: adapted the argument list to the expected 2-tuple: add additional parens instead
                signature: Predef.println(x: Any): Unit
          given arguments: 1, 2
         after adaptation: Predef.println((1, 2): (Int, Int))
(1,2)

as for the turning the warning into an error, -Werror will turn all warnings into errors, and the incantation for turning just this particular warning into an error is -Wconf:cat=lint-adapted-args:e:

scala> :reset -Xlint -Wconf:cat=lint-adapted-args:e
Resetting interpreter state.
Forgetting all expression results and named terms: $intp

scala> println(1,2)
              ^
       error: adapted the argument list to the expected 2-tuple: add additional parens instead
               signature: Predef.println(x: Any): Unit
         given arguments: 1, 2
        after adaptation: Predef.println((1, 2): (Int, Int))

note that I'm running a nightly build, since 2.13.2 isn't released yet

@SethTisue
Copy link
Member Author

SethTisue commented Apr 17, 2020

[error] bad option: '-Wadapted-args'

yeah — it isn't clear to me why some things are -W flags and other things are under -Xlint instead. @som-snytt is there some rule there?

@som-snytt
Copy link

Warnings that are not noisy and also "recommended" can be promoted to lint.

E.g., -Wunused:params is not enabled by -Xlint:unused (as a very refined case).

-Wdead-code has false positives, so is not linted.

The borderline promotable warnings IMHO are numeric-widen and value-discard.

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

No branches or pull requests

9 participants