[PATCH] D18419: Handle section vs global name conflict.

[Peter Collingbourne via llvm-commits]

pcc added inline comments.

================
Comment at: lib/MC/ELFObjectWriter.cpp:390
@@ +389,3 @@
+
+    Renames.insert(
+        std::make_pair(cast<MCSymbolELF>(Alias), cast<MCSymbolELF>(Begin)));
----------------
eugenis wrote:
> pcc wrote:
> > This problem affects the other object formats as well. Can't you do this in the generic object format lowering code with `setVariableValue`?
> In that case we emit two symbols instead of one. Also, the MCContext part is ELF specific, too.
> In that case we emit two symbols instead of one. 

You could set `IsTemporary` on the symbol you look up.

> Also, the MCContext part is ELF specific, too.

Sorry, I don't see how that is the case.

================
Comment at: lib/MC/MCContext.cpp:189-190
@@ -198,5 +188,4 @@
     }
     auto NameEntry = UsedNames.insert(std::make_pair(NewName, true));
-    if (NameEntry.second) {
-      // Ok, we found a name. Have the MCSymbol object itself refer to the copy
-      // of the string that is embedded in the UsedNames entry.
+    if (NameEntry.second || !NameEntry.first->second) {
+      // Ok, we found a name.
----------------
eugenis wrote:
> pcc wrote:
> > You can simplify this to:
> > 
> > ```
> > bool &Used = UsedNames[NewName];
> > if (!Used) {
> >   Used = true;
> > ...
> > ```
> MCSymbol constructor needs a pointer to StringMapEntry.
Yes, sorry.


Repository:
  rL LLVM

http://reviews.llvm.org/D18419