[PATCH] D67945: LowerTypeTests: Rename local functions to avoid collisions with identically named functions in ThinLTO modules.

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:1890-1896
+        if (F && F->hasLocalLinkage()) {
+          // Locally defined function that happens to have the same name as a
+          // function defined in a ThinLTO module. Rename it to move it out of
+          // the way of the external reference that we're about to create.
+          F->setName(F->getName() + ".1");
+          F = nullptr;
+        }
----------------
pcc wrote:
> dexonsmith wrote:
> > This looks like it could collide with an existing function.  Clang handles this in a few places by looping until it finds an available name.
> I was under the impression that this was already handled by setName (see http://llvm-cs.pcc.me.uk/lib/IR/Value.cpp#282 -> http://llvm-cs.pcc.me.uk/lib/IR/ValueSymbolTable.cpp#112 -> http://llvm-cs.pcc.me.uk/lib/IR/ValueSymbolTable.cpp#61), although it may be worth adding a test for this case.
Ah, you're right.  A test for that might be nice, but a comment might also be enough given that it's likely tested elsewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67945