[Issue 18712] [Reg 2.072] bogus "switch skips declaration" error with case in mixin

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=18712

Iain Buclaw  changed:

   What|Removed |Added

 CC||ibuc...@gdcproject.org
  Component|dlang.org   |dmd

--

[Issue 18712] [Reg 2.072] bogus "switch skips declaration" error with case in mixin

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=18712

Iain Buclaw  changed:

   What|Removed |Added

   Severity|normal  |regression

--

[Issue 18712] [Reg 2.072] bogus "switch skips declaration" error with case in mixin

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=18712

berni44  changed:

   What|Removed |Added

 CC||bugzi...@d-ecke.de

--- Comment #9 from berni44  ---
See also https://github.com/dlang/dlang.org/pull/2512

--

[Issue 18712] [Reg 2.072] bogus "switch skips declaration" error with case in mixin

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=18712

--- Comment #8 from Rainer Schuetze  ---
I think that the report should not be closed if it is actually undesired and
unexpected behavior, just because it cannot easily be fixed.

--

[Issue 18712] [Reg 2.072] bogus "switch skips declaration" error with case in mixin

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=18712

--- Comment #7 from Rainer Schuetze  ---
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.

--

[Issue 18712] [Reg 2.072] bogus "switch skips declaration" error with case in mixin

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=18712

Walter Bright  changed:

   What|Removed |Added

 Status|RESOLVED|REOPENED
  Component|dmd |dlang.org
 Resolution|WONTFIX |---
   Severity|regression  |normal

--- Comment #6 from Walter Bright  ---
(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.

--

[Issue 18712] [Reg 2.072] bogus "switch skips declaration" error with case in mixin

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=18712

--- Comment #5 from Rainer Schuetze  ---
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.

--

[Issue 18712] [Reg 2.072] bogus "switch skips declaration" error with case in mixin

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=18712

Walter Bright  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |WONTFIX

--- Comment #4 from Walter Bright  ---
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

--

[Issue 18712] [Reg 2.072] bogus "switch skips declaration" error with case in mixin

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=18712

Walter Bright  changed:

   What|Removed |Added

 CC||bugzi...@digitalmars.com

--- Comment #3 from Walter Bright  ---
If

mixin("case 0:");

is replaced with:

{ case 0: }

it also fails in the same way.

--

[Issue 18712] [Reg 2.072] bogus "switch skips declaration" error with case in mixin

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=18712

--- Comment #2 from Martin Nowak  ---
Also see issue 16254

--

[Issue 18712] [Reg 2.072] bogus "switch skips declaration" error with case in mixin

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=18712

Martin Nowak  changed:

   What|Removed |Added

 CC||c...@dawg.eu
Summary|bogus "switch skips |[Reg 2.072] bogus "switch
   |declaration" error with |skips declaration" error
   |case in mixin   |with case in mixin
 OS|Windows |All

--- Comment #1 from Martin Nowak  ---
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.

--