8016 – Methods defined in external object files when template alias parameter is involved

D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 8016 - Methods defined in external object files when template alias parameter is involved
Summary: Methods defined in external object files when template alias parameter is inv...
Status: RESOLVED INVALID
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P2 normal
Assignee: No Owner
URL:
Keywords: wrong-code
Depends on:
Blocks:
Reported: 2012年05月02日 10:12 UTC by Leandro Lucarella
Modified: 2012年05月09日 08:07 UTC (History)
2 users (show)

See Also:


Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this issue.
Description Leandro Lucarella 2012年05月02日 10:12:57 UTC
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.
Comment 1 Don 2012年05月03日 00:39:41 UTC
As for bug 7745, with D2, it doesn't need -release -inline.
Comment 2 Don 2012年05月03日 02:04:54 UTC
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 
Comment 3 Leandro Lucarella 2012年05月03日 03:21:07 UTC
> 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).
Comment 4 Leandro Lucarella 2012年05月03日 03:45:02 UTC
Added bug 8023.
Comment 5 Leandro Lucarella 2012年05月03日 03:54:49 UTC
Test case for the test suite:
https://github.com/D-Programming-Language/dmd/pull/921 
Comment 6 Leandro Lucarella 2012年05月03日 04:24:18 UTC
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).
Comment 7 github-bugzilla 2012年05月03日 11:55:02 UTC
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.
Comment 8 github-bugzilla 2012年05月03日 13:28:18 UTC
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
Comment 9 Walter Bright 2012年05月03日 20:19:02 UTC
The test cases were reverted because they failed in the auto-tester.
Comment 10 Leandro Lucarella 2012年05月04日 02:00:44 UTC
(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.


AltStyle によって変換されたページ (->オリジナル) /