D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 2029 - Typesafe variadic functions don't work in CTFE
Summary: Typesafe variadic functions don't work in CTFE
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D1 (retired)
Hardware: x86 Windows
: P2 normal
Assignee: No Owner
URL:
Keywords: patch, rejects-valid
Depends on:
Blocks:
 
Reported: 2008-04-24 16:36 UTC by Bartosz Milewski
Modified: 2014-02-16 15:26 UTC (History)
5 users (show)

See Also:


Attachments
draft patch for fixing typesafe variadic and ctfe (2.44 KB, patch)
2009-11-18 03:40 UTC, Tomas Lindquist Olsen
Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description Bartosz Milewski 2008-04-24 16:36:56 UTC
The following doesn't compile:

string foo (string [] a...)
{
    string result = "";
    foreach (s; a)
        result ~= s;
    return result;
}

mixin (foo ("int ", "x;"));

tError: cannot evaluate foo(cast(invariant(char)[][])((invariant(char)[][2u] __arrayArg244 = void;
) , (__arrayArg244[0u]) = "int " , (__arrayArg244[1u]) = "x;" , __arrayArg244))
at compile time
attribute argument to mixin must be a string, not (foo(cast(invariant(char)[][])
((invariant(char)[][2u] __arrayArg244 = void;
) , (__arrayArg244[0u]) = "int " , (__arrayArg244[1u]) = "x;" , __arrayArg244)))
Comment 1 Tomas Lindquist Olsen 2009-09-15 07:15:10 UTC
I'd like this to work too :) I guess I should make a patch ...
Comment 2 Don 2009-09-15 07:30:43 UTC
(In reply to comment #1)
> I'd like this to work too :) I guess I should make a patch ...

I had a quick go at it before the last release. I didn't include it because it required changes outside of interpret.c. So I did member functions instead <g>.
Should work on D1 as well.
Comment 3 Don 2009-09-15 07:42:03 UTC
(In reply to comment #2)
> (In reply to comment #1)
> > I'd like this to work too :) I guess I should make a patch ...
> 
> I had a quick go at it before the last release. I didn't include it because it
> required changes outside of interpret.c. So I did member functions instead <g>.
> Should work on D1 as well.

To clarify: you can just comment out the lines near the top of interpret.c that displays the error message about typesafe variadics. But then you find that it doesn't compile, because it's trying to modify a global variable __arrayArg1.
(The CTFE error messages make it a lot easier to understand what's happening <g>).
Something similar happens with struct constructors, and I think it's wrong. It's more difficult to know how to fix it without breaking something else.
Comment 4 Tomas Lindquist Olsen 2009-11-18 03:40:17 UTC
Created attachment 501 [details]
draft patch for fixing typesafe variadic and ctfe
Comment 5 Tomas Lindquist Olsen 2009-11-18 03:46:13 UTC
I've attached a draft patch that replaces the typesafe variadic ast rewrites to something that works in any scope (and with ctfe).

without the patch the following function:

    void foo(int[] arr ...)

when called:

    foo(1,2,3);

is rewritten as:

    int[3] _args;
    (_args[0] = 1, _args[1] = 2, _args[2] = 3, foo(_args[]));

..

In global scope, this breaks, as you cannot assign to globals from ctfe, and in normal ctfe, breaks somehow as well.

This patch changes the rewrite to:

    foo([1,2,3]);

with each element being implicitly converted to the array element type.

One downside is that codegen always allocates array literals on the heap, so it degrades performance a bit, but that needs to be fixed for http://d.puremagic.com/issues/show_bug.cgi?id=2356 as well (though it's probably a slightly different issue), and generally I'm not getting an impression that it's a concern (performance is secondary).

In any case I added a new "scopedLiteral" field to ArrayLiteralExp so that codegen knows it's allowed to put the array on the stack if it wants to.

Comments appreciated.
Comment 6 Tomas Lindquist Olsen 2009-11-18 03:48:50 UTC
I forgot to mention, the patch is against the latest dmd-1.x branch
Comment 7 Tomas Lindquist Olsen 2009-11-21 06:21:44 UTC
So... Noone cares?

... This is a *draft*. I'm looking for feedback on how to better fix this, while the patch isn't perfect, it does fix the bug.
Comment 8 Don 2009-11-25 21:23:48 UTC
Thanks, this is helpful. The problem is, that this isn't an isolated bug: the struct constructor bug has the same root cause. I think we need to fix both of them at once.
Comment 9 Don 2009-12-23 01:34:48 UTC
The solution is much, much simpler. The problem is that CTFE doesn't support casting array literals to their base type, even though it's a no-op!


constfold.c, inside Expression *Cast(Type *type, Type *to, Expression *e1),
line 1095:
------------
    /* Allow casting from one string type to another
     */
    if (e1->op == TOKstring)
    {
	if (tb->ty == Tarray && typeb->ty == Tarray &&
	    tb->nextOf()->size() == typeb->nextOf()->size())
	{
	    return expType(to, e1);
	}
    }
    
+    if (e1->op == TOKarrayliteral && typeb == tb)
+        return e1;

    if (e1->isConst() != 1)
	return EXP_CANT_INTERPRET;
--------
And then in interpret.c, we can remove the error message (line 116):

    TypeFunction *tf = (TypeFunction *)tb;
    Type *tret = tf->next->toBasetype();
-    if (tf->varargs)
-    {	cantInterpret = 1;
-	error("Variadic functions are not yet implemented in CTFE");
-	return NULL;
-    }
    // Ensure there are no lazy parameters
    if (tf->parameters)
Comment 10 Don 2009-12-23 01:50:30 UTC
Oops, we still don't support C-style variadics. The patch to interpret.c should be:

 -    if (tf->varargs)
+    if (tf->varargs && arguments && parameters && arguments->dim != parameters->dim)
    {	cantInterpret = 1;
-	error("Variadic functions are not yet implemented in CTFE");
+	error("C-style Variadic functions are not yet implemented in CTFE");
	return NULL;
    }
Comment 11 Walter Bright 2009-12-29 03:04:14 UTC
Changeset 318
Comment 12 Walter Bright 2009-12-29 03:08:36 UTC
(In reply to comment #11)
> Changeset 318

Wrong changeset, oops! 318 is for bugzilla 282
Comment 14 Walter Bright 2009-12-31 11:11:17 UTC
Fixed dmd 1.054 and 2.038