Bug 14210 – invalid merging of template instances

Status
RESOLVED
Resolution
FIXED
Severity
major
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2015-02-20T17:53:00Z
Last change time
2017-07-19T17:38:45Z
Keywords
pull
Assigned to
nobody
Creator
ketmar

Comments

Comment #0 by ketmar — 2015-02-20T17:53:27Z
compiler erroneously merges templates with different delegate types. let's run the following code: === test.d === void test(DT) (string name, DT dg) { import std.stdio; writeln(":: ", name); writeln(typeof(dg).stringof); } void main () { test("001", (int a=40) => a+2); test("000", (int a) => a+0); } === output: :: 001 int function(int a = 40) pure nothrow @nogc @safe :: 000 int function(int a = 40) pure nothrow @nogc @safe now let's change `main` to: === void main () { test("000", (int a) => a+0); test("001", (int a=40) => a+2); } === output: :: 000 int function(int a) pure nothrow @nogc @safe :: 001 int function(int a) pure nothrow @nogc @safe both results are obviously wrong, 'cause compiler should not merge two instantiations. yet compiler checks for merge possibility by checking mangled names, and both types mangles to the same string. this breaks automatic CTFE wrapping of functions for some scripting engines, and possibly many more cases.
Comment #1 by ketmar — 2015-02-20T17:56:02Z
p.s. to make it even clearer: === void main () { test("000", (int a) => a+0); test("001", (int b=40) => b+2); } === output: :: 000 int function(int a) pure nothrow @nogc @safe :: 001 int function(int a) pure nothrow @nogc @safe === void main () { test("001", (int b=40) => b+2); test("000", (int a) => a+0); } === output: :: 001 int function(int b = 40) pure nothrow @nogc @safe :: 000 int function(int b = 40) pure nothrow @nogc @safe now THIS is very, very bad, as it not only messes with default arg value, but renaming the arg itself.
Comment #2 by k.hara.pg — 2015-02-21T02:22:53Z
(In reply to Ketmar Dark from comment #0) > compiler erroneously merges templates with different delegate types. let's > run the following code: > [snip] > > both results are obviously wrong, 'cause compiler should not merge two > instantiations. yet compiler checks for merge possibility by checking > mangled names, and both types mangles to the same string. > > this breaks automatic CTFE wrapping of functions for some scripting engines, > and possibly many more cases. In D, default arguments of a function pointer/delegate type is not a part of type. I's mostly fixed in the PR: https://github.com/D-Programming-Language/dmd/pull/1102 In a nutshell, instantiated templates cannot take a type with default arguments. They should be stripped off in the instantiation. In the OP code, the two 'test' function template call should instantiate only one function with DT = int function(int) pure nothrow @nogc @safe.
Comment #3 by ketmar — 2015-02-21T02:48:53Z
it's ok to instantiate the same template (as the function signatures are the same). but it's not ok to merge them for CTFE. actually, i have code like this: === import std.traits : isSomeFunction; Cell opIndexAssign(DT) (DT dg, string name) if (isSomeFunction!DT) { import std.traits : ParameterTypeTuple, ParameterDefaultValueTuple, ReturnType; alias args = ParameterTypeTuple!DT; alias retType = ReturnType!DT; Cell cfn; // default args can be taken only from delegate itself, not from the type, hence this hack alias defaultArguments = ParameterDefaultValueTuple!dg; cfn new CellFnX!(DT, defaultArguments)(dg); this[name] = cfn; return cfn; } private class CellFnX(DT, Defs…) : Cell { DT dg; this (DT adg) { super(aismacro); dg = adg; } override Cell execute (Milf eng, CellCons args) { … ParameterTypeTuple!DT arguments; foreach (auto idx, ref arg; arguments) { static if (!is(Defs[idx] == void)) { arg = Defs[idx]; } else { arg = Cell.from(args.car); args = args.cdr; } } } } === this is CTFE wrapper generator, and with merging like now *each* registered delegate with the same parameter types will get the same default values. this kills the whole CTFE wrapper generation idea.
Comment #4 by k.hara.pg — 2015-02-21T03:15:50Z
(In reply to Ketmar Dark from comment #3) > this is CTFE wrapper generator, and with merging like now *each* registered > delegate with the same parameter types will get the same default values. > this kills the whole CTFE wrapper generation idea. Unfortunately the code does not work as you expected. Default arguments won't be encoded in type, so you cannot capture them via type. Instead of that, you need to handle a function symbol. void foo(alias f)() { import std.traits; pragma(msg, ParameterDefaultValueTuple!f); // directly give a function symbol } void main() { foo!((int a=40) => a+2); // tuple(40) foo!((int a) => a+2); // (void) }
Comment #5 by k.hara.pg — 2015-02-21T03:16:25Z
Comment #6 by ketmar — 2015-02-21T03:33:31Z
(In reply to Kenji Hara from comment #4) > (In reply to Ketmar Dark from comment #3) > > this is CTFE wrapper generator, and with merging like now *each* registered > > delegate with the same parameter types will get the same default values. > > this kills the whole CTFE wrapper generation idea. > > Unfortunately the code does not work as you expected. Default arguments > won't be encoded in type, so you cannot capture them via type. > > Instead of that, you need to handle a function symbol. > > void foo(alias f)() > { > import std.traits; > pragma(msg, ParameterDefaultValueTuple!f); // directly give a function > symbol > } > void main() > { > foo!((int a=40) => a+2); // tuple(40) > foo!((int a) => a+2); // (void) > } yep, that was exactly the thing i wanted to avoid… i wanted to use `eng["func"] = (int a=40) => a+2;` instead of `eng["func"] = buildWrapper!((int a=40) => a+2);`. i was tricked by the fact that `typeof(dg)` knows about default args (hence the hack with `ParameterDefaultValueTuple!dg`), and i was sure that different default values means "different declared types" for dg, with "same actual types" (or something like that, i don't know how to word it right). and your patch broke even that hack. (sobs) ah, well, i guess i can't get everything i want. at least i found a bug. thank you.
Comment #7 by github-bugzilla — 2015-02-22T06:39:51Z
Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/43b3f1ed2451e9d8980e271cba2d5e58acdc4707 fix Issue 14210 - invalid merging of template instances https://github.com/D-Programming-Language/dmd/commit/00cbcb3dec972a85bf402677c67f0f3d93a99ed4 Merge pull request #4426 from 9rnsr/fix14210 Issue 14210 - invalid merging of template instances
Comment #8 by github-bugzilla — 2015-06-17T21:01:02Z
Comment #9 by github-bugzilla — 2017-07-19T17:38:45Z
Commits pushed to dmd-cxx at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/43b3f1ed2451e9d8980e271cba2d5e58acdc4707 fix Issue 14210 - invalid merging of template instances https://github.com/dlang/dmd/commit/00cbcb3dec972a85bf402677c67f0f3d93a99ed4 Merge pull request #4426 from 9rnsr/fix14210