make D great again

[adamdruppe]

No description provided.

[dlang-bot]

Thanks for your pull request and interest in making D better, @adamdruppe! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	10532	enhancement	Silence some unreachable statement warnings when in a static foreach
✓	14835	major	Constant folding should not affect front end flow analysis
✓	20522	enhancement	Spurious statement unreachable warning caused by undefined variable

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#15568"

[thewilsonator]

Description + changelog/issue# please.

[adamdruppe]

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

less directly:

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

It is hard to use D's metaprogramming capabilities without this warning issuing a false positive and forcing you to degrade the code to work around it.

[thewilsonator]

So if this fixes any of those, the commit message should say so, you know the drill.

[rikkimax]

Alternative: disable it for scopes that use static if and static foreach.

[0xEAB]

This warning sounds great and useful in theory, but:
Practice shows it’s an obstacle most of the time. (Even when we ignore the false positives!)

I can think of maybe one occasion where it actually helped me prevent a buggy program from compiling. On the other hand I could regularly tell tales how it gets in my way.

Unlike with bigger C projects (at least a bunch that I’ve run into), most D projects are written in a “no warnings” way. This is established well enough that even DUB defaults to emitting the -w flag (“Treat warnings as errors”). So, false positive or not, what happens when you run into the “statement not reachable” warning? Yeah, you go and edit your build recipe. Inconvenient :/

—
When it’s not a false positive, it’s still often useless, because code works as intended.

So, let’s say, I’ve hit the warning because I’ve added temporary debugging code to my program. Maybe a quick assert(false); to stop execution from going beyond a certain point, a patchy throw statement with a different msg or a return statement to short-circuit things with a different return value. Or whatever… I know what I’m doing here (hopefully), I won’t forget to remove the temp code. But I’ll trigger the warning.
Well, time to interrupt my flow. Gotta add -wi for the debugging session. Just to remove it later (or forget to do so).

Bonus points if you know the DUB equivalent for -wi by heart. I don’t. I’ll sit there and have to look it up; again and again.

[pbackus]

This isn't just a problem with static foreach and static if. One of the most annoying instances of this warning I've run into was in code of the form

unittest
{
    assert(cond1);
    assert(cond2);
}

If the compiler is able to constant-fold cond1 to false, it will declare the second assertion unreachable. Under default dub settings, this halts compilation, which makes it impossible to run the rest of the test suite until that specific assertion is fixed.

[0xEAB]

Another interesting finding is that the warning is not emitted for code like this:

return;
assert(false);

You wouldn’t (have to) special case this if the thing weren’t kinda low-value in the first place.

[ljmf00]

This is pretty much a linting rule that we are forced to use without just disabling all the warnings. I think this is worth keeping, if we have a way to select which rule to warn us.

[tgehr]

Test suite has to be updated.

[dkorpel]

Discussed in the September monthly meeting, remove warning but leave code in for now

[thewilsonator]

again, changelog entry or issues fixed please.

[thewilsonator]

merge conflict

[ibuclaw]

Third commit does more than convert tabs to spaces.

Maybe squash the commits and make use of the Co-Authored-By trailer?

[dkorpel]

Third commit does more than convert tabs to spaces.

Yeah something went wrong when rebasing on top of #15623

[ljmf00]

Don't forget to merge with squash.