Summary: | Methods defined in external object files when template alias parameter is involved | ||
---|---|---|---|
Product: | D | Reporter: | Leandro Lucarella <leandro.lucarella> |
Component: | dmd | Assignee: | No Owner <nobody> |
Status: | RESOLVED INVALID | ||
Severity: | normal | CC: | bugzilla, clugdbug |
Priority: | P2 | Keywords: | wrong-code |
Version: | D2 | ||
Hardware: | All | ||
OS: | All | ||
See Also: |
http://d.puremagic.com/issues/show_bug.cgi?id=7745 http://d.puremagic.com/issues/show_bug.cgi?id=8023 |
This looks like a reincarnation of bug 7745, but is different. Testcase: m1.d --- module m1; import m2; private void t(alias Code)() { return Code(); } void f() { t!( () { } )(); } // The forceSemanti() and static assert is only needed to reproduce it in D2 bool forceSemantic() { f(); return true; } static assert(forceSemantic()); --- m2.d --- module m2; import m1; --- dmd -c -inline -release -g m2.d && nm m2.o | grep '_D2m11fFZv$' 00000000 T _D2m11fFZv Which means that _D2m11fFZv (AKA m1.f()) is included in the text (code) section of m2.o, but it shouldn't, it should only be in m1.d (which I didn't even compile), otherwise when linking both m1.o and m2.o you'll get a "multiple definition" error. Symbol _D2m11fFZv should have U (undefined) or maybe at most v/W (weak linkage). Please note that the compiler flags -inline -release has to be used to reproduce the bug, removing either makes it go away. -O is irrelevant. Confirmed in both 32 and 64 bits. This regression was introduced by commit "fix Issue 2962 - ICE(glue.c) or bad codegen passing variable as template value parameter": D1: https://github.com/D-Programming-Language/dmd/commit/d5a33a1 D2: https://github.com/D-Programming-Language/dmd/commit/900c537 https://github.com/D-Programming-Language/dmd/commit/900c537 Again, if an appropriate fix is not found soon enough, maybe reverting the patch introducing the problem is better than introducing a regression.
As for bug 7745, with D2, it doesn't need -release -inline.
Here's what happens. When parsing m2, it imports m1. The static assert forces it to run semantic on f(). This instantiates template t. t gets added to the list of instantiated templates *of module m2*. Before the fix for bug 2962 was made, template t was written to m2.o with weak linkage. This was strictly unnecessary, but relatively harmless because of the weak linkage. But the template that was written may have been wrong, if it hit bug 2962. Now that bug 2962 is fixed, it writes f. This pull request changes it so that it only writes f if it is part of a template, and will therefore have weak linkage. That still fixes all test cases in bug 2962. https://github.com/D-Programming-Language/dmd/pull/920
> https://github.com/D-Programming-Language/dmd/pull/920
Looks like this test case is exposing more bugs than we were aware of!
This pull request fixes the original problem I had (compiling tango CDGC), which is the test case without the forceSemantic() trick.
Using the forceSemantic() trick doesn't work in D1 but doesn't look like a regression, at least is present in 1.070 and 1.071.
The patch only fixes the problem partially though, with the patch you don't get the multiple definition error but you get an undefined reference error instead, so is a step forward but not a fix. But again, this is not a regression in D2, the problem is present at least before the fix for 2962.
I'll report another bug with the D1-only bug that's fixed by this pull request, so we can close that one when the pull is merged, but can keep this one open (but without the regression severity).
Added bug 8023.
Test case for the test suite: https://github.com/D-Programming-Language/dmd/pull/921
SomeMore findings. By the way, -inline -release is not really needed to reproduce this bug (it was only needed for the D1 original bug, without forceSemantic(), see bug 8023 for details).
Commit pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/d420faebda45eb27596697594a61ecd2a6d3b3d7 Fix issue 8016 Regression Methods defined in external object files when template alias parameter is involved Make the fix for bug 2962 a bit less aggressive. Still passes all test cases for 2962.
Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/fb797c20e97ccfe18e0ec483dc8373e8028991de Add test case for bug 8016 https://github.com/D-Programming-Language/dmd/commit/731fb84247f170d7de8ae091f90c61134e548c1e Merge pull request #921 from llucax/test8016 Add test case for bug 8016
The test cases were reverted because they failed in the auto-tester.
(In reply to comment #9) > The test cases were reverted because they failed in the auto-tester. This is because that patch only partially fix this issue, it seems that there are 2 issues in this test case and only one is being fix in the patch. The patch completely fixes bug 8023 though. So please re-apply pull #922 (now pull #923: https://github.com/D-Programming-Language/dmd/pull/923). Wait for the autotester to run to make sure is passing now that the fix for 8023 is merged. I'm sorry I made such a mess of this bug report, but it was quite tricky because I kept discovering bugs exposed by it. I will close this bug as INVALID and open a new one with a proper description of the issue that is left after the patch.