Gradual warning for deprecated nonlocal return #15303
Conversation
|
I would have preferred a test for the warning and also for the printer. |
| "Non local returns are no longer supported; use scala.util.control.NonLocalReturns instead", | ||
| tree.srcPos, | ||
| from = future) | ||
| warnFrom = `3.0`, |
There was a problem hiding this comment.
| warnFrom = `3.0`, | |
| warnFrom = `3.2`, |
There was a problem hiding this comment.
It used to warn if you weren't in a "Scala 2" mode or frame of mind. That is 3.0-migration now? But I see that this warns under 3.0-migration. I expected X-migration semantics: "error or migration warning", and "warning or free pass if migrating at X".
There was a problem hiding this comment.
Pushed that conjecture.
| @@ -74,7 +74,7 @@ object report: | |||
|
|
|||
| def gradualErrorOrMigrationWarning(msg: Message, pos: SrcPos = NoSourcePosition, warnFrom: SourceVersion, errorFrom: SourceVersion)(using Context): Unit = | |||
| if sourceVersion.isAtLeast(errorFrom) then errorOrMigrationWarning(msg, pos, errorFrom) | |||
| else if sourceVersion.isAtLeast(warnFrom) then warning(msg, pos) | |||
| else if sourceVersion.ordinal >= warnFrom.ordinal then warning(msg, pos) | |||
There was a problem hiding this comment.
As an example, this change would disable the new refutable pattern binding warnings under -source 3.2-migration (but the offending code is rewritten under -rewrite -source 3.2-migration). Is that the desired behavior?
There was a problem hiding this comment.
I saw that usage but hadn't quite decoded it yet, so the behavior is not desired in particular. At least the PR is in draft mode.
At a glance, I may have intended it. Mostly because there is no Scala 2 mode for the current use case. Maybe that is a red herring and there is no exclusion (for nonlocal return if I'm migrating from Scala 2) or as suggested just defer it to 3.2.
There was a problem hiding this comment.
Or I read too much into -migration or misread it. It means don't error yet, just warn me migratorily. Then gradual means warn even before that, so does -migration mean anything? Is it just for when rewrites kick in? As in, "help me migrate".
There was a problem hiding this comment.
The only I resisted the suggestion to warn at 3.2 was that I thought I read that there is a use case for keeping source at 3.0. I don't remember the use case around library compatibility. Anyone doing that would miss out on this niche case warning.
There was a problem hiding this comment.
so I tried to compile a file that has a non-local return with 3.0.0 and there was no warning or anything, so I think compiling with 3.2.0 but using -source:3.0 should still not make a warning. Regardless you made the change I requested :)
There was a problem hiding this comment.
Yes, deprecation was in 0.16, errorOrMigration at 3.1 in 0.24 for -migration scheme, and at future in 3.0.0-RC1. The docs make plain the feature is deprecated rather than dropped outright, but introducing the warning in 3.2 is sufficiently effective.
som-snytt
force-pushed
the
tweak/warn-nonlocal-return
branch
from
82fb399 to
5f833b3
Compare
|
Back-tweaked to warn from |
|
@odersky does this need SIP approval? |
|
@bishabosha No I think that was already approved in the last batch. |
Follow up #15298
warns for nonlocal returns from
-source:3.2It's still true that it errors only on
-source:future.