Bug 16521 – Wrong code generation with switch + static foreach

The following code: ``` void main () { sformat(0, uint.max); } void sformat (Args...) (int index, Args args) { JT: switch (index) { foreach (idx, unused; args) { case idx: assert(unused == args[idx], "Borken compiler"); break JT; } default: assert(0); } } ``` Triggers the assert. Obviously `unused` and `args[idx]` should always be the same. Here's the generated ASM: ``` 000000000042237c <D main>: 42237c: 55 push %rbp 42237d: 48 8b ec mov %rsp,%rbp 422380: 31 f6 xor %esi,%esi 422382: bf ff ff ff ff mov $0xffffffff,%edi 422387: e8 08 00 00 00 callq 422394 <test.sformat!(uint).sformat(int, uint)> 42238c: 31 c0 xor %eax,%eax 42238e: 5d pop %rbp 42238f: c3 retq 422390: 0f 1f 40 00 nopl 0x0(%rax) 0000000000422394 <test.sformat!(uint).sformat(int, uint)>: 422394: 55 push %rbp 422395: 48 8b ec mov %rsp,%rbp 422398: 48 83 ec 20 sub $0x20,%rsp 42239c: 89 7d f8 mov %edi,-0x8(%rbp) 42239f: 85 f6 test %esi,%esi 4223a1: 74 02 je 4223a5 <test.sformat!(uint).sformat(int, uint)+0x11> 4223a3: eb 37 jmp 4223dc <test.sformat!(uint).sformat(int, uint)+0x48> 4223a5: 8b 45 f0 mov -0x10(%rbp),%eax ; <== Access to unitialized variable `unused` 4223a8: 3b 45 f8 cmp -0x8(%rbp),%eax 4223ab: 74 2d je 4223da <test.sformat!(uint).sformat(int, uint)+0x46> 4223ad: 41 b8 0d 00 00 00 mov $0xd,%r8d 4223b3: b9 00 54 44 00 mov $0x445400,%ecx 4223b8: b8 06 00 00 00 mov $0x6,%eax 4223bd: 48 89 c2 mov %rax,%rdx 4223c0: 48 89 55 e8 mov %rdx,-0x18(%rbp) 4223c4: ba 07 54 44 00 mov $0x445407,%edx 4223c9: bf 0f 00 00 00 mov $0xf,%edi 4223ce: 48 89 d6 mov %rdx,%rsi 4223d1: 48 8b 55 e8 mov -0x18(%rbp),%rdx 4223d5: e8 ce 00 00 00 callq 4224a8 <_d_assert_msg> 4223da: eb 0a jmp 4223e6 <test.sformat!(uint).sformat(int, uint)+0x52> 4223dc: bf 12 00 00 00 mov $0x12,%edi 4223e1: e8 2e 00 00 00 callq 422414 <test.__assert(int)> 4223e6: c9 leaveq 4223e7: c3 retq 4223e8: 0f 1f 40 00 nopl 0x0(%rax) ``` Tested with v2.070.0 and v2.071.1 The latest master (cce7909) hints at the bug: test.d(8): Deprecation: 'switch' skips declaration of variable test.sformat!uint.sformat.unused at test.d(10)

A bit of debugging proved that the following code is generated: ``` [{ enum ulong idx = 0; uint unused = _param_1; case idx: { assert(unused == args[idx], "Borken compiler"); break JT; } } ] ``` Note: Using `ref` results in the program segfaulting because it will then dereference an uninitialized `ref`.

I hit the same bug today: ``` import std.traits; struct S { int a, b, c, d, e, f; } void main() { S s; foreach (name; ["a", "b", "c"]) { switch (name) { foreach (i, ref field; s.tupleof) { case __traits(identifier, S.tupleof[i]): field = 0; // s.tupleof[i] = 0; works break; } default: break; } } } ```

I see the same deprecation warning in my code, but I'm not using the actual tuple value, just the index. Static foreach would be handy here...

There is no such thing as static foreach. You are using tuple foreach which will force an unrolled loop. The provided code should error!

I don't agree that this code should error. This is a well known and common D idiom (simple GH search shows examples in Phobos, such: https://github.com/dlang/phobos/blob/master/std/algorithm/sorting.d#L1101), up to the point that there are also merged switch/foreach loops constructs where the label is applied to switch, so the `break` inside foreach would be considered switch break.

Just because something is used in phobos does not it is correct. this code is highly dubious because it goats people into believing that tuple foreach actually works. The right way to do this is to build a string for the switch and mix it in.

I'm not sure who ever got convinced by this code that `static foreach` works. This is just very helpful and clear approach, and it should grant `static foreach` becoming a real thing, because it is useful. While that is still not implemented, this code should work, as I see this as a bug/regression (https://github.com/rejectedsoftware/vibe.d/blob/6f37e694cc77063769bc4c9a42160627103e8354/web/vibe/web/rest.d#L1367 - yet another example, plus dozens on forums, etc).

guys, the static foreach comment was a *wish* for this, not a recommendation to use some already existing thing. It would be nice to do: static foreach(idx; 0 .. args.length) Instead of the goofy "unused" symbol thing. In my code, I'm using Args, not args, so I have no idea why a variable for each tuple item should even need to be created. It might even be a type! Note that the "deprecation" is what I'm concerned about, as I have only this one way to make my switch statement out of a tuple (sorry Stefan, but string mixin is a *vastly inferior* option compared to this mechanism), and some future version of dmd I worry is going to flag this as an error, even though I never use the variable I didn't want to declare but was forced to.

looks like a duplicate of my issue https://issues.dlang.org/show_bug.cgi?id=17218

*** Issue 17218 has been marked as a duplicate of this issue. ***

FWIW this works with static foreach --- void main () { sformat(0, uint.max); } void sformat (Args...) (int index, Args args) { JT: switch (index) { static foreach (idx, unused; args) { case idx: assert(unused == args[idx], "Borken compiler"); break JT; } default: assert(0); } } --- https://run.dlang.io/is/eJvNFB

Any update on this?

(In reply to Imperatorn from comment #12) > Any update on this? What do you mean? It's resolved as invalid. The code no longer compiles (for a while now).