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

Better support for guards in switches #5070

Open
allanrenucci opened this issue Sep 3, 2018 · 3 comments
Open

Better support for guards in switches #5070

allanrenucci opened this issue Sep 3, 2018 · 3 comments

Comments

@allanrenucci
Copy link
Contributor

#4982 Added some support for guards in switches. However this still doesn't work:

import scala.annotation.switch

class Test {
  def succ_guard(c: Int) = (c: @switch) match { // warning: Could not emit switch for @switch annotated match
    case 1 | 2 | 3   => true
    case x if x == 4 => true
    case _           => false
  }
}
Duhemm added a commit to dotty-staging/dotty that referenced this issue Sep 5, 2018
This is required because some scripted tests use dotty-doc.

Fixes scala#5070
@allanrenucci
Copy link
Contributor Author

allanrenucci commented Oct 2, 2018

It actually gets translated to a switch. In this specific case, the warning is spurious. The condition to emit the warning is wrong:
https://github.com/lampepfl/dotty/blob/e4735d2f758ac5701484be7bc068ee32defac74f/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala#L917

@sjrd
Copy link
Member

sjrd commented Oct 2, 2018

Try adding more cases without guard after the one with a guard.

@sjrd
Copy link
Member

sjrd commented Oct 2, 2018

Dumping the document I created with @allanrenucci earlier today: https://gist.github.com/sjrd/16e9cb6c7d7b9a2093cf1521f93f2209

griggt added a commit to griggt/dotty that referenced this issue Sep 25, 2020
In the spirit of scala/scala#4148, which fixed SI-8731.

The consensus seems to be that the intent of the @switch annotation is to
warn when the generated bytecode may be poor, not merely if the compiler
elects to not emit a tableswitch/lookupswitch.

Note that the case threshold for determining whether a switch is emitted
is implementation dependent, and currently varies between scalac and dotc.

Also note that this implementation will not warn in instances where a
switch will never be emitted (e.g. because the match is on a non-integral
type) but the number of cases is below the warning threshold. This behavior
is consistent with scalac, but may be surprising to the user if another
case is added later and a warning suddenly appears.

Not all spurious @switch warnings are addressed by this commit, see
issue scala#5070 for an example.
griggt added a commit to griggt/dotty that referenced this issue Sep 25, 2020
In the spirit of scala/scala#4418, which fixed SI-8731.

The consensus seems to be that the intent of the @switch annotation is to
warn when the generated bytecode may be poor, not merely if the compiler
elects to not emit a tableswitch/lookupswitch.

Note that the case threshold for determining whether a switch is emitted
is implementation dependent, and currently varies between scalac and dotc.

Also note that this implementation will not warn in instances where a
switch will never be emitted (e.g. because the match is on a non-integral
type) but the number of cases is below the warning threshold. This behavior
is consistent with scalac, but may be surprising to the user if another
case is added later and a warning suddenly appears.

Not all spurious @switch warnings are addressed by this commit, see
issue scala#5070 for an example.
griggt added a commit to griggt/dotty that referenced this issue Jan 11, 2021
In the spirit of scala/scala#4418, which fixed SI-8731.

The consensus seems to be that the intent of the @switch annotation is to
warn when the generated bytecode may be poor, not merely if the compiler
elects to not emit a tableswitch/lookupswitch.

Note that the case threshold for determining whether a switch is emitted
is implementation dependent, and currently varies between scalac and dotc.

Also note that this implementation will not warn in instances where a
switch will never be emitted (e.g. because the match is on a non-integral
type) but the number of cases is below the warning threshold. This behavior
is consistent with scalac, but may be surprising to the user if another
case is added later and a warning suddenly appears.

Not all spurious @switch warnings are addressed by this commit, see
issue scala#5070 for an example.
michelou pushed a commit to michelou/dotty that referenced this issue Mar 15, 2021
In the spirit of scala/scala#4418, which fixed SI-8731.

The consensus seems to be that the intent of the @switch annotation is to
warn when the generated bytecode may be poor, not merely if the compiler
elects to not emit a tableswitch/lookupswitch.

Note that the case threshold for determining whether a switch is emitted
is implementation dependent, and currently varies between scalac and dotc.

Also note that this implementation will not warn in instances where a
switch will never be emitted (e.g. because the match is on a non-integral
type) but the number of cases is below the warning threshold. This behavior
is consistent with scalac, but may be surprising to the user if another
case is added later and a warning suddenly appears.

Not all spurious @switch warnings are addressed by this commit, see
issue scala#5070 for an example.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants