On 10/27/2015 10:33 PM, deadalnix wrote:
> On Saturday, 24 October 2015 at 17:25:23 UTC, tsbockman wrote:
>> While improving the DMD front-end's constant folding:
>>     https://github.com/D-Programming-Language/dmd/pull/5229
>> I found out about DMD issue 14835:
>>     https://issues.dlang.org/show_bug.cgi?id=14835
>>
>> Briefly:
>> ///////////////////////////////
>> module main;
>>
>> import std.stdio;
>>
>> void reachIf(bool x)()
>> {
>>     if(!x)
>>         return;
>>     writeln("reached"); // Warning: statement is not reachable
>> }
>>
>> void main(string[] args) {
>>     reachIf!true();  // prints "reached"
>>     reachIf!false(); // triggers warning
>> }
>> ///////////////////////////////
>>
>> This is, I think, a big problem.
>
> It is. This is the optimizer leaking into semantic. As far as language
> definition is concerned, this is a runtime if and changing it to a
> compile time one is an optimization.
>
> If this was a static if, then generating a error is reasonable, but with
> a regular if, it isn't.
>

if statements with constant conditions perhaps shouldn't be eliminated in the frontend, at least for reachability analysis within templated functions.

On 28/10/2015 8:29 AM, tsbockman wrote:
> On Tuesday, 27 October 2015 at 21:14:26 UTC, Timon Gehr wrote:
>> On 10/27/2015 09:18 PM, tsbockman wrote:
>>> I don't think any dead code is being generated,
>>
>> This seems to be a misunderstanding. I mean generated using mixins or
>> template instantiation. Sure, it will usually be removed, but why
>> generate and semantically analyze it in the first place.
>
> Forcing me to add `static if`s with redundant and potentially complex
> predicates just to make my code do the exact same thing it would have
> done anyway is a violation of "Don't Repeat Yourself", with all of the
> usual consequences:
>
> * The more places the same logic is expressed, the more chances I have
> to get it wrong and cause a bug.
>
> * Even if I get it right the first time, a redundant predicate could get
> out of sync with the rest of the code later, causing a bug.
>
> * It's a waste of my time, which is more valuable than my computer's time.
>
> * It clutters up the code with statements which add little (useful)
> information.
>

I personally like the style of that code, and agree that it allows less repetition.  But it does this at the cost of intentionally introducing dead code in some instantiations.  If you enable the compiler warnings about dead code, they have to trigger here because it doesn't know if you introduced dead code intentionally or not.  As is often the case with warnings, if you want your code to compile with them you sometimes need to avoid otherwise completely valid constructs.

Here's a similar example:

bool func(T, T val)()
{
    if (val < 0)
        return true;
    return false;
}

func!(uint, 7);
func!(int, 7);

When instantiated with uint, the first return is unreachable because unsigned numbers cannot be negative.  When val == 7, it's also unreachable because 7 is not less than 0.  Which instantiations should give the warning?

> Another perspective, though, which I picked up from someone (Andrei
> Alexandrescu, I think?) in the D community, is to consider template
> parameters simply as additional function arguments, which happen to
> be evaluated at compile time. In many cases, the timing of their
> evaluation is just an implementation detail - a performance
> optimization (or de-optimization, as the case may be).

This is a great way to learn how to use templates, but there are limits to how well this simplification corresponds to reality and this is one of them.  Parameters inhibit optimizations and analysis in ways that compile-time constants do not.

On Tuesday, 27 October 2015 at 21:55:28 UTC, Timon Gehr wrote:
> On 10/27/2015 10:29 PM, tsbockman wrote:
>> On Tuesday, 27 October 2015 at 21:14:26 UTC, Timon Gehr wrote:
>>> On 10/27/2015 09:18 PM, tsbockman wrote:
>>>> I don't think any dead code is being generated,
>>>
>>> This seems to be a misunderstanding. I mean generated using mixins or
>>> template instantiation. Sure, it will usually be removed, but why
>>> generate and semantically analyze it in the first place.
>>
>> Forcing me to ...
>
> This post is attacking a straw man. I'm not suggesting any of this.

Well then what *are* you suggesting? Can you describe an easy way for the user to avoid or suppress this warning without introducing redundant logic into their code?

On Tuesday, 27 October 2015 at 21:33:04 UTC, deadalnix wrote:
> If this was a static if, then generating a error is reasonable, but with a regular if, it isn't.

As Daniel Murphy pointed out earlier, a surefire way to *prevent* the warning is to use `static if`; code hidden by a `static if` will never trigger it. I believe this is correct, and by design.

(But not all control flow statements have static equivalents, so this solution can only be applied to some code. Even if we had `static switch`, `static foreach`, `static goto`, etc., I doubt that forcing the user to segregate all compile-time logic from the run-time logic in that way is desirable.)

Whether the logic is explicitly `static` (compile time) or not, the warning should be issued if and only if the flagged code is unreachable with all possible input combinations - including both compile-time and run-time.

On Wednesday, 28 October 2015 at 03:38:37 UTC, Daniel Murphy wrote:
> I personally like the style of that code, and agree that it allows less repetition.  But it does this at the cost of intentionally introducing dead code in some instantiations.  If you enable the compiler warnings about dead code, they have to trigger here because it doesn't know if you introduced dead code intentionally or not.  As is often the case with warnings, if you want your code to compile with them you sometimes need to avoid otherwise completely valid constructs.
>
> Here's a similar example:
>
> bool func(T, T val)()
> {
>     if (val < 0)
>         return true;
>     return false;
> }
>
> func!(uint, 7);
> func!(int, 7);
>
> When instantiated with uint, the first return is unreachable because unsigned numbers cannot be negative.  When val == 7, it's also unreachable because 7 is not less than 0.  Which instantiations should give the warning?

I would say none, since *the template* contains no unreachable code, and the compiler can easily trim unreachable code from any *instantiation* which needs it, without bothering me about it.

I would only be interested in a warning if the compiler wasn't able to trim the dead code, but as far as I can tell the only case in which the compiler doesn't trim it, is the case where it doesn't realize it's unreachable - in which case it can't warn me, either.

> > Another perspective, though, which I picked up from someone
> (Andrei
> > Alexandrescu, I think?) in the D community, is to consider
> template
> > parameters simply as additional function arguments, which
> happen to
> > be evaluated at compile time. In many cases, the timing of
> their
> > evaluation is just an implementation detail - a performance
> > optimization (or de-optimization, as the case may be).
>
> This is a great way to learn how to use templates, but there are limits to how well this simplification corresponds to reality and this is one of them.  Parameters inhibit optimizations and analysis in ways that compile-time constants do not.

It's not intended as a simplification for people who can't handle the true complexity of templates - the difference is philosophical. It's a recognition of the fundamental unity of run-time and compile-time computation, the same idea which motivates CTFE.

If most people actually *want* these warnings, then great - there's no bug.
But, if most find the warnings conflict with how they want to use templates, as I do - why not just change it?

The "reality" of D templates is whatever the D community chooses to make it, subject to technical feasibility.

On 28/10/2015 4:29 PM, tsbockman wrote:
>
> I would say none, since *the template* contains no unreachable code, and
> the compiler can easily trim unreachable code from any *instantiation*
> which needs it, without bothering me about it.

If it's unreachable or not depends on what the template is instantiated with, there is no clear concept of unreachable code without knowing the template parameters.

bool func(T)(T value) if (isUnsigned!T)
{
    if (value < 0)
        return true;
    return false;
}

Here the first return is definitely dead code for any instantiation, but to know this the compiler would have to reverse-engineer properties from the template constraints, which is not generally possible.

>
> I would only be interested in a warning if the compiler wasn't able to
> trim the dead code, but as far as I can tell the only case in which the
> compiler doesn't trim it, is the case where it doesn't realize it's
> unreachable - in which case it can't warn me, either.

Well of course...

> It's not intended as a simplification for people who can't handle the
> true complexity of templates - the difference is philosophical. It's a
> recognition of the fundamental unity of run-time and compile-time
> computation, the same idea which motivates CTFE.

IIRC it's intended to avoid scaring people off reading TDPL by avoiding the work 'template'.

> If most people actually *want* these warnings, then great - there's no bug.
> But, if most find the warnings conflict with how they want to use
> templates, as I do - why not just change it?

I don't want these warnings, so I don't generally build with warnings enabled.

> The "reality" of D templates is whatever the D community chooses to make
> it, subject to technical feasibility.

As one of the core compiler devs, I'm saying it sounds infeasible.  I don't think either of your suggested solutions are implementable. Templates just do not work that way.

> 1. Defer "not reachable" warnings until compilation has been
> completed, and only issue the warning if *all* instantiations of the
> statement were unreachable.

The exact set of instantiations depends on the current module being compiled, so module A can still get an unreachable code warning even if in an instantiation from module B the code is reachable.

> 2. For semantic analysis purposes, first instantiate each template
> using dummy parameters with the widest possible VRP ranges. Only
> statements found to be "not reachable" in this dummy run should
> actually generate warnings.

Will not work with compile-time introspection.

In some trivial cases code can be found to be unreachable without doing semantic analysis, and therefore can be done before template instantiation.  Being limited, I doubt this is of much value.

On 28/10/2015 4:02 PM, tsbockman wrote:
> (But not all control flow statements have static equivalents, so this
> solution can only be applied to some code. Even if we had `static
> switch`, `static foreach`, `static goto`, etc., I doubt that forcing the
> user to segregate all compile-time logic from the run-time logic in that
> way is desirable.)

Nobody is forcing anyone to do this.  Warnings are opt-in.

> Whether the logic is explicitly `static` (compile time) or not, the
> warning should be issued if and only if the flagged code is unreachable
> with all possible input combinations - including both compile-time and
> run-time.

In D's compilation model it is not possible to know all possible instantiations at compilation time.

On Wednesday, 28 October 2015 at 10:03:44 UTC, Daniel Murphy wrote:
> On 28/10/2015 4:02 PM, tsbockman wrote:
>> (But not all control flow statements have static equivalents, so this
>> solution can only be applied to some code. Even if we had `static
>> switch`, `static foreach`, `static goto`, etc., I doubt that forcing the
>> user to segregate all compile-time logic from the run-time logic in that
>> way is desirable.)
>
> Nobody is forcing anyone to do this.  Warnings are opt-in.

That's true, but it's generally good practice to compile with warnings enabled, and it's bad practice to compile with warnings and not fix them. So, ultimately, a warning isn't all that different from an error. If push comes to shove, then you can compile -wi instead of -w and leave the warning in, or you can choose to not compile with warnings at all, so it's not quite the same as an error, but it's effectively the same if you're following what's generally considered good programming practices (which is part of why I really wish that Walter had never given in and added warnings to the compiler).

- Jonathan M Davis

On Wednesday, 28 October 2015 at 10:02:01 UTC, Daniel Murphy wrote:
> If it's unreachable or not depends on what the template is instantiated with, there is no clear concept of unreachable code without knowing the template parameters.

If a statement in a template is reachable in at least one possible instantiation of that template, it is not "unreachable". What is unclear about this as a concept?

Just because implementing this definition perfectly in the compiler is difficult, doesn't mean the concept is unclear.

> bool func(T)(T value) if (isUnsigned!T)
> {
>     if (value < 0)
>         return true;
>     return false;
> }
>
> Here the first return is definitely dead code for any instantiation, but to know this the compiler would have to reverse-engineer properties from the template constraints, which is not generally possible.

Detection of all dead code will never be "generally possible", with or without templates involved - if it was you could trivially solve the halting problem.

If false negatives are unacceptable, then we should remove the warning entirely, as achieving that goal is mathematically impossible on a normal computer.

> [snip]
>
> As one of the core compiler devs, I'm saying it sounds infeasible.  I don't think either of your suggested solutions are implementable. Templates just do not work that way.
>
> > 1. Defer "not reachable" warnings until compilation has been
> > completed, and only issue the warning if *all* instantiations
> of the
> > statement were unreachable.
>
> The exact set of instantiations depends on the current module being compiled, so module A can still get an unreachable code warning even if in an instantiation from module B the code is reachable.

I know that solution #1 is far from perfect - it would, however, significantly reduce the frequency of false positives. As I said above, perfection is not the goal, since it is not even theoretically possible due to the halting problem.

> > 2. For semantic analysis purposes, first instantiate each
> template
> > using dummy parameters with the widest possible VRP ranges.
> Only
> > statements found to be "not reachable" in this dummy run
> should
> > actually generate warnings.
>
> Will not work with compile-time introspection.

I'm not sure precisely what you're referring to here. I know solution #2 would be a royal pain to implement, though, even if it is possible.

> In some trivial cases code can be found to be unreachable without doing semantic analysis, and therefore can be done before template instantiation.  Being limited, I doubt this is of much value.

Perhaps the warning should just be removed then.

On 10/28/15 6:02 AM, Daniel Murphy wrote:
> On 28/10/2015 4:29 PM, tsbockman wrote:
>>
>> I would say none, since *the template* contains no unreachable code, and
>> the compiler can easily trim unreachable code from any *instantiation*
>> which needs it, without bothering me about it.
>
> If it's unreachable or not depends on what the template is instantiated
> with, there is no clear concept of unreachable code without knowing the
> template parameters.

You're not thinking from the user's point of view. It's a statement. I wrote it because I want it to work in certain instantiations. The compiler is incorrectly complaining that it can't be reached, because I can reach it by calling it a different way (easy to prove).

The warning should either be eliminated, or fixed so it's accurate. This reminds me of people adding empty try/catch clauses to Java to shut the compiler up about the checked exceptions.

>
> bool func(T)(T value) if (isUnsigned!T)
> {
>      if (value < 0)
>          return true;
>      return false;
> }
>
> Here the first return is definitely dead code for any instantiation, but
> to know this the compiler would have to reverse-engineer properties from
> the template constraints, which is not generally possible.

I'm OK with the compiler giving up and not warning here. This warning is an optimization, and a *warning*, not an error.

>> If most people actually *want* these warnings, then great - there's no
>> bug.
>> But, if most find the warnings conflict with how they want to use
>> templates, as I do - why not just change it?
>
> I don't want these warnings, so I don't generally build with warnings
> enabled.

As will others who do want warnings, but don't want to deal with the compiler making false accusations :)

>> The "reality" of D templates is whatever the D community chooses to make
>> it, subject to technical feasibility.
>
> As one of the core compiler devs, I'm saying it sounds infeasible.  I
> don't think either of your suggested solutions are implementable.
> Templates just do not work that way.

What about disabling unreachability detection if a static branch (either explicit or implied by const folding) depends on a template variable?

If we can't fix this, I think we're better off without the warning.

-Steve