vsk added inline comments.

================
Comment at: clang/lib/AST/Mangle.cpp:130
+      return;
+    }
+
----------------
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.


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

Reply via email to