rjmccall added inline comments.
================
Comment at: clang/include/clang/Basic/Attr.td:734
+ // TODO: Make it possible to specify this in source.
+ BoolArgument<"LiteralLabel">
+ ];
----------------
`LiteralLabel` is an unfortunate name for this property because
`getLiteralLabel` sounds like it ought to return a string. How about
`IsLiteralLabel`?
Also there's a "fake" flag to make it clear that this argument isn't
(currently) written in source, so this should be
`BoolArgument<"IsLiteralLabel", /*optional*/ 0, /*fake*/ 1>`.
Comment suggestion:
IsLiteralLabel specifies whether the label is literal (i.e. suppresses the
global C symbol prefix) or not.
================
Comment at: clang/include/clang/Basic/Attr.td:740
+[{
+bool equivalent(AsmLabelAttr *Other) const {
+ return getLabel() == Other->getLabel() && getLiteralLabel() ==
Other->getLiteralLabel();
----------------
`isEquivalent`, please.
================
Comment at: clang/include/clang/Basic/AttrDocs.td:2579
+ }];
+}
+
----------------
Oh, thank you for adding documentation. I think the prefixing thing probably
ought to be explained here; here's a suggestion for some wording:
This attribute can be used on a function or variable to specify its symbol
name.
On some targets, all C symbols are prefixed by default with a single
character, typically ``_``. This was done historically to distinguish them
from symbols used by other languages. (This prefix is also added to the
standard Itanium C++ ABI prefix on "mangled" symbol names, so that e.g. on such
targets the true symbol name for a C++ variable declared as ``int cppvar;``
would be ``__Z6cppvar``; note the two underscores.) This prefix is *not* added
to the symbol names specified by the ``asm`` attribute; programmers wishing to
match a C symbol name must compensate for this.
For example, consider the following C code:
<example>
Clang's implementation of this attribute is compatible with GCC's,
`documented here <https://gcc.gnu.org/onlinedocs/gcc/Asm-Labels.html>`_.
While it is possible to use this attribute to name a special symbol used
internally by the compiler, such as an LLVM intrinsic, this is neither
recommended nor supported and may cause the compiler to crash or miscompile.
Users who wish to gain access to intrinsic behavior are strongly encouraged to
request new builtin functions.
================
Comment at: clang/lib/AST/Mangle.cpp:130
+ return;
+ }
+
----------------
This is actually backwards, right? A literal label is one that doesn't get the
global prefix and therefore potentially needs the `\01` prefix to suppress it.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67774/new/
https://reviews.llvm.org/D67774
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
