D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 3553 - ICE when a function argument defaults to __LINE__
Summary: ICE when a function argument defaults to __LINE__
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: Other Windows
: P2 normal
Assignee: No Owner
URL:
Keywords: ice-on-valid-code, patch
Depends on:
Blocks:
 
Reported: 2009-11-26 15:46 UTC by Sergey Gromov
Modified: 2015-06-09 01:27 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Sergey Gromov 2009-11-26 15:46:08 UTC
The following code:

------8<---------
void foo(T)(size_t line = __LINE__) {}
void main() { foo!char(); }
------8<---------

when compiled, causes an internal compiler error:

>dmd2 -c test.d
__LINE__
Internal error: e2ir.c 652

Workaround: declare line as int.
Comment 1 Don 2009-11-26 16:33:26 UTC
It doesn't need to be a template. It crashes whenever there's an implicit cast from __LINE__ to any non-int type, in a function default argument.

void foo(uint line = __LINE__) {}
void main() { foo(); }
Comment 2 Don 2009-11-26 23:55:23 UTC
There are two issues. One is that this use of __LINE__ is a very special case.

The code in Parser::parseDefaultInitExp() in parse.c is unlikely to be correct -- it only treats a default of __LINE__ specially; __LINE__+0 does NOT become a LineInitExp.

But, without changing that, the immediate problem is in expression.c, functionParameters(), around line 670.

The lineInitExp only gets resolved if it's unaltered. But, it might
have been implicitly cast. This patch fixes that.
A more complete fix would change parse.c to allow arbitrary use of __LINE__, and to resolve recursively in expression.c looking for LineInitExp's. But that's a pretty obscure feature; this patch is enough to fix the ICE.

Index: expression.c
===================================================================
--- expression.c	(revision 267)
+++ expression.c	(working copy)
@@ -667,7 +667,13 @@
 		}
 		arg = p->defaultArg;
 #if DMDV2
-		if (arg->op == TOKdefault)
+		if (arg->op == TOKcast && ((CastExp*)arg)->e1->op == TOKdefault)
+		{   // The default value may have been implicitly cast
+		    arg = arg->copy();
+		    CastExp * def = (CastExp*)arg;
+		    def->e1 = ((DefaultInitExp *)(def->e1))->resolve(loc, sc);
+		}
+		else if (arg->op == TOKdefault)
 		{   DefaultInitExp *de = (DefaultInitExp *)arg;
 		    arg = de->resolve(loc, sc);
 		}
Comment 3 Brad Roberts 2009-11-27 00:22:00 UTC
Hrm.. what's that entire block of code in the 'if' doing?  TOKfile and TOKline are handled down through the layers of expression parsing (primarily in parsePrimaryExp) just fine, from a quick study of the code.  I haven't tried just killing that block to see what happens.  Might it be better to change the two parts of parsePrimaryExp to create FileInitExp and LineInitExp's rather than StringExp and IntegerExp's respectively?
Comment 4 Don 2009-12-01 11:17:08 UTC
(In reply to comment #3)
> Hrm.. what's that entire block of code in the 'if' doing?  TOKfile and TOKline
> are handled down through the layers of expression parsing (primarily in
> parsePrimaryExp) just fine, from a quick study of the code.

That block is dealing with __LINE__ as a default parameter. In that case only, __LINE is the line that the function is instantiated in, rather than the line it appeared in. So the other uses of TOKfile and TOKline are completely different.

> Might it be better to change the
> two parts of parsePrimaryExp to create FileInitExp and LineInitExp's rather
> than StringExp and IntegerExp's respectively?

FileInitExp, LineInitExp are only handled when they're a default function parameter. Lots of stuff would need to change if they were used throughout.

Basically, the whole thing seems to be a quick hack to get int line = __LINE__ working as a default parameter, which is by far the most important use case. Dealing with the more complicated, obscure cases would be *much* more work.
Comment 5 Walter Bright 2009-12-03 10:58:28 UTC
I think this is better done with a virtual function. Changeset 280.
Comment 6 Don 2009-12-03 11:41:26 UTC
(In reply to comment #5)
> I think this is better done with a virtual function. Changeset 280.

Great! Why not BinExp::resolveLoc() too?
Comment 7 Walter Bright 2009-12-03 12:01:33 UTC
Mainly because the way it is parsed, I don't think it could ever happen at the moment.
Comment 8 Walter Bright 2009-12-06 00:55:20 UTC
Fixed dmd 2.037