[PATCH] D67774: [Mangle] Check ExternalASTSource before adding prefix to asm label names

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/AST/Mangle.cpp:137
+        (!Source || Source->UseGlobalPrefixInAsmLabelMangle()))
       Out << '\01'; // LLVM IR Marker for __asm("foo")
 
----------------
This is one of those bugs where looking at the code really reveals a lot of problematic behavior.

`AsmLabelAttr` is generally assumed to carry a literal label, i.e. it should not have the global prefix added to it.  We should not be changing that assumption globally just based on whether an `ExternalASTSource` has been set up; that's going to break in any sort of non-uniform situation, e.g. if an LLDB user writes a declaration with an asm label, or if we import a Clang module where a declaration has an asm label.  Either `ExternalASTSource`s should be putting the real (prefixed) symbol name in the `AsmLabelAttr`, or they should be making `AsmLabelAttr`s that are different somehow, e.g. by giving `AsmLabelAttr` a flag (which doesn't need to be spellable in the source language) saying whether the label is literal or subject to global prefixing.

Separately, it seems to me that if the `AsmLabelAttr` is literal but the label happens to include the global prefix, we should strip the prefix from the label and not add a `\01` so that we get a consistent symbol name in LLVM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67774/new/

https://reviews.llvm.org/D67774