-
-
Notifications
You must be signed in to change notification settings - Fork 421
issue 19949 - C++ Mangling: add support for abi-tags from the Itanium ABI #2639
Conversation
|
Thanks for your pull request, @SSoulaimane! 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 + druntime#2639" |
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.
Name needs to be in D Style
d50c3a9 to
0a7c41f
Compare
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.
Put all the documentation in a version (CoreDdoc) block along with a copy of the struct declaration. I think it's enough with the constructor (without a body) in the copy. Otherwise the documentation will only be available if generated on a Posix system. That's currently a problem with the above selector attribute. Have a look here, it's empty: https://dlang.org/phobos/core_attribute.html.
|
How about something like this: version (Posix)
version = UdaGNUAbiTag;
version (D_ObjectiveC)
version = UdaSelector;
version (CoreDdoc)
{
version = UdaGNUAbiTag;
version = UdaSelector;
} |
Hmm, that might work. I was actually only referring to the documentation you added but if you want to fix the |
src/core/attribute.d
Outdated
| * ) | ||
| * | ||
| * Note: Unlike other attributes, if applied to a namespace, | ||
| * this attribute it is not inherited by the members. |
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.
Tags on an inline namespace are not represented in the mangled name of the namespace, but they are subject to the above tag propagation
https://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangle.abi-tag
The attribute can also be applied to an inline namespace, but does not affect the mangled name of the namespace; in this case it is only used for -Wabi-tag warnings and automatic tagging of functions and variables. Tagging inline namespaces is generally preferable to tagging individual declarations, but the latter is sometimes necessary, such as when only certain members of a class need to be tagged.
https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Attributes.html
So it should be inherited by the member. But that's a C++ side thing, we should represent this with @gnuAbiTag("XXX") { /* decls */ } not by trying to mimic the C++ code.
I would just remove this sentence.
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.
I don't understand what you mean.
That sentence is about the current DMD behavior of inheriting UDAs from a namespace.
for example:
@("abc")
extern (C++, "namespace")
{
void fun();
}fun has the attribute "abc".
while it is not the same in the following case:
@("abc")
extern (C++)
class C
{
void fun();
}fun doesn't have the attribute "abc".
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.
This special UDA is not supposed to be inherited from a namespace.
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 and that makes sense. But what your sentence reads to me is that in this case:
@gnuAbiTag("Test")
extern (C++, "Test") {
void fun();
}fun would not have the funAbiTag("Test") attribute.
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 that's what I meant, It is handled specially by the compiler that's why it's documented.
Doc version added
|
There were several issues raised with this. EDIT: Ah, it's actually the Druntime part, the issues were raised on the DMD PR. |
|
Superseded by #2990 |
Special UDA for mangling
extern(C++)symbols.Required by dlang/dmd#9995 to fix issue 19949.