On Tuesday, 27 October 2015 at 16:05:31 UTC, tsbockman wrote:
> On Tuesday, 27 October 2015 at 13:23:51 UTC, Timon Gehr wrote:
>> Versions of the same statement in different instantiations are independent. Templates are just a restricted form of hygienic macros.
>
> That's how the current implementation works, but that doesn't mean the warning is actually *helpful*.

Well, arguably that's exactly what templates _are_ regardless of the implementation - i.e. a template is just a template to generate code, not really actual code in and of itself. Now, that being said, I'm not sure that warning about unreachable code in a templated function is particularly helpful - particularly if it forces you to jump through hoops to make the compiler shut up about it. Heck, if anything, I find that warning/error annoying in regular code, because it tends to get in the way of debugging while developing. It wouldn't really hurt my feelings any if we just removed it from the compiler entirely - though I completely agree that you don't really want to have unreachable code left in your production code.

- Jonathan M Davis

On 10/27/15 9:23 AM, Timon Gehr wrote:
> On 10/27/2015 01:35 PM, Steven Schveighoffer wrote:
>>>
>>> Easy to fix:
>>>
>>> void reachIf(bool x)()
>>> {
>>>       static if(!x)
>>>           return;
>>>       else
>>>           writeln("reached");
>>> }
>>>
>>> The warning is correct, and incredibly annoying.
>>>
>>
>> Easy to fix, but the warning is incorrect, the statement is reachable if
>> you use reachIf!true.
>>
>> A statement is not a compiled piece of code.
>>
>
> Versions of the same statement in different instantiations are
> independent. Templates are just a restricted form of hygienic macros.
>

I understand how the compiler treats it. But it still doesn't make the warning true. In some cases, the statement is reachable, the compiler is unhelpfully pointing out cases where it was unreachable.

It would be if the compiler had a function like this:

int foo(bool x)
{
   if(x == 5) return 1;
   return 0;
}

And you compile this function:

void main()
{
   foo(5);
}

with -inline, the compiler complained. It's unhelpful that you are telling me that you are not generating code for my statement *in this instance*.

If you want to tell me that you would *never* generate code for my statement in *any* instance, that is helpful.

-Steve

On 10/27/2015 06:35 PM, Steven Schveighoffer wrote:
> I understand how the compiler treats it.  In some cases, the statement
> is reachable, the compiler is unhelpfully pointing out cases where it
> was unreachable.

The reachable statement is not the same as the statement that is flagged unreachable. This is not an implementation detail and it is not 'incorrect'. It might be changed if the consensus is that it is unhelpful. Personally, I usually avoid generating dead code.

On Tuesday, 27 October 2015 at 19:30:08 UTC, Timon Gehr wrote:
> On 10/27/2015 06:35 PM, Steven Schveighoffer wrote:
>> I understand how the compiler treats it.  In some cases, the statement
>> is reachable, the compiler is unhelpfully pointing out cases where it
>> was unreachable.
>
> The reachable statement is not the same as the statement that is flagged unreachable. This is not an implementation detail and it is not 'incorrect'. It might be changed if the consensus is that it is unhelpful. Personally, I usually avoid generating dead code.

I don't think any dead code is being generated, in the cases where the compiler knows enough to issue the "statement is not reachable" warning. My testing indicates that when the compiler decides code is unreachable, it just removes it.

(I found this out because of a bug in the compiler:
    http://forum.dlang.org/thread/qjmikijfluaniwnxhigp@forum.dlang.org)

Any fix for issue 14835 should still allow the compiler to detect and remove dead code in template instantiations - but in cases where the code could be reached with a different set of template parameters, it should be removed silently, without the warning.

This would make it consistent with the behaviour of inlining, as Steven Schveighoffer pointed out.

On Tuesday, 27 October 2015 at 17:23:21 UTC, Jonathan M Davis wrote:
> On Tuesday, 27 October 2015 at 16:05:31 UTC, tsbockman wrote:
>> On Tuesday, 27 October 2015 at 13:23:51 UTC, Timon Gehr wrote:
>>> Versions of the same statement in different instantiations are independent. Templates are just a restricted form of hygienic macros.
>>
>> That's how the current implementation works, but that doesn't mean the warning is actually *helpful*.
>
> Well, arguably that's exactly what templates _are_ regardless of the implementation - i.e. a template is just a template to generate code, not really actual code in and of itself.

That is one (valid) way of thinking about templates.

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).

Thinking about templates in this way leads naturally to Steven Schveighoffer's comparison with function inlining, above.

> Now, that being said, I'm not sure that warning about unreachable code in a templated function is particularly helpful - particularly if it forces you to jump through hoops to make the compiler shut up about it. Heck, if anything, I find that warning/error annoying in regular code, because it tends to get in the way of debugging while developing. It wouldn't really hurt my feelings any if we just removed it from the compiler entirely - though I completely agree that you don't really want to have unreachable code left in your production code.
>
> - Jonathan M Davis

I find the warning helpful; it just needs to be limited to cases in which the code is actually unreachable. Removing it entirely wouldn't be a disaster, but we'd have to think carefully about whether the payoff of improvements to VRP and constant folding was really worth it.

Do you have an opinion regarding my solution #1 - defer "not reachable" warnings until all instantiations of a template have been analyzed, and only issue the warning if it applies to every one of them?

On 10/27/15 12:04 PM, tsbockman wrote:
> On Tuesday, 27 October 2015 at 12:28:38 UTC, Steven Schveighoffer wrote:
>> On 10/26/15 1:08 PM, tsbockman wrote:
>>> My solution #2 is the same as the one you proposed - the dummy parameter
>>> stuff is my vague proposal for a way to actual implement this behavior
>>> in the compiler.
>>>
>>> The halting problem is no more of an issue than it is for the compiler
>>> today. (That is to say, the halting problem leads to false negatives,
>>> but not false positives.)
>>
>> OK, as long as the default behavior isn't to reject the code, I
>> suppose this is simply an optimization.
>
> Right.
>
>> But the problem I see is that only static ifs that reduce to static
>> if(true) would be considered to cause a short-circuit (e.g. someint <=
>> int.max). Unless the compiler can VRP according to the template
>> constraint, which may be possible in simple circumstances, but not in
>> all circumstances.
>
> I think the next piece of Lionello Lunesu's PR which I am updating might
> actually do that; I'll have to check later. Regardless, you are correct
> solution #1 will substantially reduce the opportunities for constant
> folding.

I'm not a compiler dev, so I'm not sure how it works or should work. I'm looking at this from a user standpoint.

I didn't consider that the compiler is likely already using VRP to determine whether a line of code is unnecessary. If it's a matter of just keeping track of 2 VRP ranges (the actual VRP and the VRP just for checking reachability) for each line of code, then perhaps it's just easier to extend VRP. I don't know the answer.

All I was saying is that it's not necessary to add on an additional layer if it's not present.

I defer to the actual devs as to what is easier.

-Steve

On 10/27/2015 09:18 PM, tsbockman wrote:
> On Tuesday, 27 October 2015 at 19:30:08 UTC, Timon Gehr wrote:
>> On 10/27/2015 06:35 PM, Steven Schveighoffer wrote:
>>> I understand how the compiler treats it.  In some cases, the statement
>>> is reachable, the compiler is unhelpfully pointing out cases where it
>>> was unreachable.
>>
>> The reachable statement is not the same as the statement that is
>> flagged unreachable. This is not an implementation detail and it is
>> not 'incorrect'. It might be changed if the consensus is that it is
>> unhelpful. Personally, I usually avoid generating dead code.
>
> 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.

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.

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.

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.