fix Issue 18966 - extern(C++) constructor should match C++ semantics …#8362
fix Issue 18966 - extern(C++) constructor should match C++ semantics …#8362dlang-bot merged 1 commit intodlang:stablefrom
Conversation
|
Thanks for your pull request, @rainers! Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub fetch digger
dub run digger -- build "stable + dmd#8362" |
|
LGTM :) |
|
Hey @MartinNowak, I saw you branched the beta for the next release. |
|
If reviewers could expedite this PR, it should be in 2.081 along with all the other extern(C++) work this cycle. |
wilzbach
left a comment
wilzbach
left a comment
There was a problem hiding this comment.
The changes look good to me based on the diff, but I'm not that familiar with C++, s.t. I would feel comfortable with approving it.
…assigning vtable set vtbl after exlicit or implicit call to super() in C++ constructors
|
Rebased to stable. |
|
Someone else should have a look at the backend part of the commit, but the changes make sense and I see no reason not to trust Rainer to get this right. Having a known-buggy implementation in 2.081 is worse than having a unreviewed one, so merging now to make sure this isn't lost. |
As per dlang#8362, this function also needs calling from the backend.
There's a new need to access a class' vtable symbol, see dlang/dmd#8362. Use it as alias to the actual vtable symbol with different type (dummy: `i8*`, actual: `[N x i8*]`) and mangled name. I tried matching the special symbol's mangled name and using an appropriate static array front-end type for it, but then casting the symbol address for the assignment leads to issues if the ctor is @safe. So I decided to handle it in DtoSymbolAddress(). Unfortunately, this seems not to solve the extern(C++) issues exposed by LDC self-compilation yet.
| { | ||
| // if super is defined in C++, it sets the vtable pointer to the base class | ||
| // so we have to rewrite it, but still return 'this' from super() call: | ||
| // (auto tmp = super(), this.__vptr = __vtbl, tmp) |
There was a problem hiding this comment.
This vtbl assignment cannot be interpreted at compile-time, see https://forum.dlang.org/thread/ghusgzuqpcskhwzmlwbh@forum.dlang.org. Happening here:
pointer cast from cpp.Y to immutable(void*)** is not supported at compile time
There was a problem hiding this comment.
How does CTFE assign vtables?
There was a problem hiding this comment.
Is this actually an issue? Any class with an opaque constructor (ie, is defined in C++) can't possibly CTFE...?
There was a problem hiding this comment.
Yep, if there's an external ctor, CTFE will fail anyway (untested :]). If they're all in D, it used to work before, so it's clearly a regression, and resetting the vptr can probably be safely skipped during CTFE (something like __ctfe || (this.__vptr = __vtbl)).
There was a problem hiding this comment.
Yeah, if that logic is only performed when the base constructor is extern, then it should still be fine.
7 participants
…assigning vtable
set vtbl after exlicit or implicit call to super() in C++ constructors