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.

Sign up for GitHub

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

Jump to bottom

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
Labels
area:pattern-matching itype:enhancement

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
@Duhemm
Fix scala#5079: Publish dotty-doc before scripted
9b87af9 
This is required because some scripted tests use dotty-doc.

Fixes scala#5070
@allanrenucci
Copy link
Contributor Author

allanrenucci commented Oct 2, 2018 •
edited
Loading

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
@griggt
Fix scala#9776: Don't issue a @switch warning when fewer than 4 cases
64ec380 
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 griggt mentioned this issue Sep 25, 2020
Merged
griggt added a commit to griggt/dotty that referenced this issue Sep 25, 2020
@griggt
Fix scala#9776: Don't issue a @switch warning when fewer than 4 cases
f2a2c04 
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
@griggt
Fix scala#9776: Don't issue a @switch warning when fewer than 4 cases
e298e21 
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
@griggt @michelou
Fix scala#9776: Don't issue a @switch warning when fewer than 4 cases
aa0b9b4 
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
Assignees
No one assigned
Labels
area:pattern-matching itype:enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@sjrd @nicolasstucki @allanrenucci