With dmd master this code produces an error:
int test(int n)
{
switch(n)
{
mixin("case 0:");
int x = 1;
return x;
case 1:
int y = 2;
return y;
default:
return -1;
}
}
swtch.d(4): Error: switch skips declaration of variable swtch.test.x at swtch.d(7)
Workaround: move the default case before the mixin.
This was a deprecation before but went unnoticed because the code compiled with -d as it is otherwise flooded with deprecation warnings (hint: deprecation should either be shown just once or be disabled selectively).
Comment #1 by code — 2018-04-11T11:33:24Z
Digger says https://github.com/dlang/dmd/pull/5869.
And btw yes, deprecation warnings should be deduplicated before printing, maybe simply by hashing the error and the source code location.
Comment #2 by code — 2018-04-11T11:35:33Z
Also see issue 16254
Comment #3 by bugzilla — 2018-05-14T14:21:36Z
If
mixin("case 0:");
is replaced with:
{ case 0: }
it also fails in the same way.
Comment #4 by bugzilla — 2018-05-14T19:31:40Z
Here's what's happening. After a case/default statement, the parser makes the code between one case/default and the next into a scope. So it looks like:
int test(int n)
{
switch(n)
{
mixin("case 0:");
int x = 1;
return x;
case 1:
{
int y = 2;
return y;
}
default:
{
return -1;
}
}
}
and, of course, now the error message makes sense. `x` is visible to the following two scopes, whether or not the error message is generated. Any time the case/default is not directly in the switch body (not nested via { } or a mixin) the implicit { } scope is not generated. Oops.
Try putting { } in various combinations, and you'll see how it all comes unglued.
I can't think of any solution that 1) works in all cases and 2) doesn't break a lot of existing code. So we're just stuck with it. Fortunately, there is a workaround. Recode the switch like this:
int test(int n)
{
switch(n)
{
mixin("case 0:");
{
int x = 1;
return x;
}
case 1:
int y = 2;
return y;
default:
return -1;
}
}
I'm going to close this as WONTFIX. If anyone has a brainwave on how to make it work in all cases without breaking code, reopen with proof.
Note that the fundamental problem is a combination of:
1. allowing case/default statements to appear inside nested scopes
2. implicit generation of scopes between case/default pairs
Comment #5 by r.sagitario — 2018-05-15T07:03:44Z
Thanks for analyzing. I wouldn't be too concerned about possible breakage with a fix, because it's a regression (and a very recent if you compiled the code with -d).
It seems adding the implicit scope block in the parser is too early, as it doesn't know about mixins. OTOH it could be clarified in the spec that 'case' in mixins do not share the scope with trailing code.
Comment #6 by bugzilla — 2018-05-15T19:05:52Z
(In reply to Rainer Schuetze from comment #5)
> Thanks for analyzing. I wouldn't be too concerned about possible breakage
> with a fix, because it's a regression (and a very recent if you compiled the
> code with -d).
It's not really a regression. The weirdness with the scope was always there, it's just that nobody noticed it. When it was implemented, I'm sure nobody thought about this behavior.
> It seems adding the implicit scope block in the parser is too early, as it
> doesn't know about mixins. OTOH it could be clarified in the spec that
> 'case' in mixins do not share the scope with trailing code.
A spec clarification is indeed in order:
https://github.com/dlang/dlang.org/pull/2368
Reopening as a spec issue. It is more than just mixins, as the other example I posted here shows.
I investigated not adding the scope in the parser, but in the semantic() phase. That just introduces even more subtle issues.
Comment #7 by r.sagitario — 2018-05-16T06:49:37Z
I just noticed that in this code:
int test(int n)
{
switch(n)
{
case -1:
int b = -1;
mixin("case 0:");
int x = 1;
return x;
case 1:
int y = 2;
return y;
default:
return -1;
}
}
The scope of b extends until the end of `case 0`, too.
Comment #8 by r.sagitario — 2018-05-16T07:01:52Z
I think that the report should not be closed if it is actually undesired and unexpected behavior, just because it cannot easily be fixed.