[PATCH] D143487: [IR] Allow destruction of symbol table entries regardless of DiscardValueNames

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 21 13:21:29 PDT 2023

efriedma added inline comments.

Comment at: llvm/lib/IR/Value.cpp:323
   SmallString<256> NameData;
   StringRef NameRef = NewName.toStringRef(NameData);
   assert(NameRef.find_first_of(0) == StringRef::npos &&
yrouban wrote:
> efriedma wrote:
> > Can we just make this `StringRef NameRef = NeedNewName ? NewName.toStringRef(NameData) : "";`,instead of messing with the rest of the function?
> I agree that we have to avoid //NewName.toStringRef(NameData)//. I have another patch that refactors //Value::setNameImpl()// further in this way - D143550. Please take a look and choose one patch that I should finalize.
Yes, D143550 does that... but it also rearranges the whole function in a bunch of other ways that I'm not sure make sense.  I'd prefer the minimal change for the bugfix, then considering the full refactoring separately.

  rG LLVM Github Monorepo



More information about the llvm-commits mailing list