Fix Issue 18916 - ICE using Typedef and __LINE__

[JinShil]

No description provided.

[dlang-bot]

Thanks for your pull request, @JinShil!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	18916	major	ICE using Typedef and LINE

Testing this PR locally

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

dub fetch digger
dub run digger -- build "stable + dmd#8310"

[jacob-carlborg]

Tests are failing, not sure if it's related.

[JinShil]

Yeah, I need some clarification from @WalterBright first. I sent him an e-mail. Here's the issue:

Consider the following D and C++ code for comparison.

D Code

import std.stdio;

void foo(int line = __LINE__)   // line 3
{
    writeln(line);
}

void main()
{
    foo();  // line 10
}

C++ Code

#include <stdio.h>

void foo(int line = __LINE__)  // line 3
{
    printf("%d\n", line);
}

int main()
{
    foo();   // line 10
}

Currently D prints 10 due to https://issues.dlang.org/show_bug.cgi?id=15668, but C++ prints 3.

[jacob-carlborg]

Currently D prints 10 due to https://issues.dlang.org/show_bug.cgi?id=15668, but C++ prints 3.

As far as I understand, it's supposed to print 10 in D.

https://dlang.org/spec/expression.html#specialkeywords

[JinShil]

I wonder if it should print 10 when __LINE__ is assigned to a compile-time parameter, but 3 if assigned to a runtime parameter.

[jacob-carlborg]

No, same behavior regardless if it's runtime or compile time parameter.

[WalterBright]

D is working as intended. It's much more useful than the C++ behavior, which is actually intended to behave like D does, as LINE is really intended for use inside preprocessor macros.

[JinShil]

Ok then, what should the output of this be?

import std.stdio;
import std.conv;

string bar(int i)  // line 4
{ 
    return i.to!string(); 
}

void foo(string line = bar(__LINE__))  // line 9
{
    writeln(line);
}

void main()
{
    foo();    // line 16
}

[JinShil]

Let me put it this way:

There is no real difference between...

import std.stdio;

void foo(int line = int(__LINE__))
{
    writeln(line);
}

void main()
{
   foo();
}

... and ...

import std.stdio;

void foo(int line = __LINE__)
{
    writeln(line);
}

void main()
{
    foo();
}

... but the two result in different behavior.

[WalterBright]

the two result in different behavior

Yes. The cause is a bit subtle. D does semantic analysis from the bottom up, yet the detection of LINE for a default parameter value happens from the top down. Only the top node is looked at. One could argue it should walk the tree looking for LINE, but that starts introducing weird problems and interactions with the bottom up nature of semantic analysis.

Changing the behavior will not only be highly likely to introduce bugs, it can break existing code in silent and unexpected ways. For example, currently one can get the C++ behavior with int(__LINE__). I believe this has value, as people have complained about it not matching C++, and this is an easy workaround.

So leave its current behavior as is (while fixing the seg fault, of course!).

[JinShil]

@WalterBright, Is this what you had in mind?

struct Line
{
    int line;
    alias line this;

    this(int line)
    {
        this.line = line;
    }
}

void foo(Line line1 = __LINE__, int line2 = __LINE__, int line3 = int(__LINE__))   // line 12
{
    assert(line1 == 12);
    assert(line2 == 21);
    assert(line3 == 12);
}

void main()
{
    foo();   // line 21
}

The problem is Line line1 = __LINE__ gets rewritten to Line(__LINE__), so evaluates to line 12 in the example above. If you think it should be line 21, I need a little more guidance; in that case, how should Line line1 = __LINE__ be rewritten?

[rainers]

currently one can get the C++ behavior with int(LINE). I believe this has value, as people have complained about it not matching C++, and this is an easy workaround.

I don't think that behaviour is desirable. AFAICT it also violates the spec: "__LINE__ expands to the source line number at the point of instantiation", but thats free to interpretation. The "special keywords" used as defaults are just part of a PrimaryExpression according to the grammar. Granted, they are listed as a TemplateValueParameterDefault specifically, but not for function parameter defaults.

If the semantics are changed, C-style __LINE__ can be achieved by defining int C__FILE__(int ln = __LINE__) { return ln; } and using that as the default argument.

But actually making it work is probably a larger operation:

• the current root cause is that the DefaultInitExp expressions are only created when they are the only argument to a parameter, otherwise they are resolved in the parser.
• if that's changed, resolveLoc needs to converted to a visitor that walks the expression recursively. Right now, it mostly stops at the top level, but not for unary expressions!
• there is currently some inline expansion and const folding happening before calling resolveLoc, that might also get into the way