rjmccall added inline comments.
================
Comment at: clang/lib/AST/Mangle.cpp:130
+ return;
+ }
+
----------------
vsk wrote:
> rjmccall wrote:
> > 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.
> Thank you for the careful and helpful review! Before updating this patch, I'd
> like to make sure we're on the same page on this point.
>
> What I'm searching for is a way to prevent `mangleName` from adding either a
> `\01` prefix or the global prefix to the label contained within an
> AsmLabelAttr. As the result from `mangleName` is passed to a hash function,
> omitting both prefixes is necessary to fix the lldb expression evaluation
> failure mentioned in the summary. Based on (probably a bad reading of)
> earlier feedback, I thought that marking an attr as "Literal" would be the
> preferred method to suppress all prefixes.
>
> However, it sounds like you believe:
> 1) An attr marked as "Literal" should have a `\01` prefix applied, but no
> global prefix.
> 2) An attr *not* marked as "Literal" should have a global prefix applied (if
> one is available).
>
> If I've characterized your view correctly, then we really need something else
> to denote "do not add any kind of prefix". One option is to use
> `BoolArgument<"IsUnprefixed", /*optional=*/0, /*fake=*/1>`, and set the
> literal/non-literal distinction aside.
>
> OTOH, if what you really meant was that `mangleName` should omit all prefixes
> for non-"Literal" labels (and that "Literal" is the right name/distinction
> for this idea), I can simply invert the condition I've been using.
I'm just asking you to invert the condition. At the user level, a "literal"
label is one that should be taken "literally", as in, it shouldn't have the
global prefix. The `\01` prefix is not part of the user-level semantics of asm
labels; it's just part of the representation of symbols in LLVM IR, and
`mangleName` should add it or not depending on what gets the right behavior
from LLVM. Ultimately, that will include canonicalizing labels so that an asm
label that starts with the global prefix (e.g. `asm("_foo")`) will just be
translated appropriately (to `@foo` rather than `@\01_foo`), but you've
reasonably asked not to take that on in this patch, so all you need to do is
make sure that we're only adding `\01` when the label is literal and there
actually is a global prefix.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67774/new/
https://reviews.llvm.org/D67774
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
