[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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143487/new/
https://reviews.llvm.org/D143487
More information about the llvm-commits
mailing list