-
-
Notifications
You must be signed in to change notification settings - Fork 608
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix 5309 - Support extern(D) symbol refs
#12839
base: master
Are you sure you want to change the base?
Conversation
|
Thanks for your pull request, @Geod24! Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#12839" |
|
Yeah, |
bcb1607
to
a3d0ad4
Compare
| ParentSymbolAttribute& opAssign(CPPNamespaceDeclaration* cpp); | ||
| ParentSymbolAttribute& opAssign(LinkDeclaration* link); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And so is it ? It should be operator=:
0000000100029884 T __ZN21ParentSymbolAttributeaSEP15LinkDeclaration
000000010002985c T __ZN21ParentSymbolAttributeaSEP23CPPNamespaceDeclaration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, operator overloads were probably not considered for the initial implementation. Probably because the spec states that those cannot be accessed from C++ IIRC.
Currently, `extern(D)` is quite incomplete: An `extern(D)` symbol is mangled as if it was in the module it is declared in, while users usually want to declare a function or type that lives in another module. Borrowing from the syntax that was adopted for `extern(C++, name.space)`, we introduce `extern(D, pkg.mod)`. Note that, unlike `extern(C++)`, no string alternative is needed: the motivation for the string variant was that C++ namespaces could be D keywords, hence some namespaces could not be bound with the identifier variant, a problem which obviously does not apply to `extern(D)` symbols. The need for this functionality is easily demonstrated by druntime's `externDFunc`. `core.internal.traits : externDFunc` is a template that defines an `extern(D)` symbol in another module. This is currently done by using `pragma(mangle)` along with `core.demangle : mangleFunc`. And as it turns out, the only reason for `core.demangle : mangleFunc` to exists is for `externDFunc`. Hence, implementing this functionality will greatly simplify a core piece of druntime: `core.demangle : mangle` (and its derivatives, including `mangleFunc` and `externDFunc`) can be removed and replaced by `XXX.mangleof`, relying on the compiler instead of a library function which have to be kept in sync with the compiler implementation.
|
This is common practice in C. But D tries to get away from that. I fear this sort of thing will have a tendency to break the sanctity(!) of the module system in D. It could be used to insert symbols into modules that aren't in those modules at all. I've found a need to do this when translating C code to D, as C does that all over the place. But the way I get it to work is by declaring those functions Having this work with I recommend instead one of:
|
It does indeed, and it's not really something we should advertise.
This part I'm not sure I follow how that'd be possible ?
When looking at the issue, I was wondering what was the best way to fix this. I think that we are faced with two solutions:
I went with the later because:
|
|
@Geod24 I agree with @WalterBright in that I really don't want this complexity in the language if we can avoid it. Do you still want to add this feature? |
TL;DR: While we should do things in libraries as much as possible, replicating in a library complex compiler logic (in this case, symbol mangling) is bad. With this in mind, I was puzzled as to why
core.demangle : mangleexisted in the first place (also consider that the module is calleddemanglebut holds a mangle function, so it was clearly an afterthought). Issue 5309 held the answer, and hopefully after this is merged we can greatly simplycore.demangle("fun" fact: the demangling code is re-used for the mangler, with added boolean logic here and there).