[PATCH] D153722: [LLParser] Friendly error out if a non global value's name exceeds max name size

[Han Yulong via Phabricator via llvm-commits]

wheatfox added inline comments.


================
Comment at: llvm/lib/AsmParser/LLParser.cpp:3447
+
+    if (InstNameSize != NameStrSize + 1) {
+      // The name is too long and was trimmed when stored in SmallString with
----------------
nikic wrote:
> So this is based on the assumption that in the "multiple definition" case the name will always be `%name0`? This seems a bit fragile (I'm not entirely confident it is true, and we can't reach a counter with more than one digit here). Is it possible to check InstNameSize < NameStrSize instead?
according to `createValueName` and `makeUniqueName`, if we have a code of
```
  %var = add i32 0, 1
  %var = alloca i32, align 4
```
then the first `var` will be fine, but the second `var` will be appended a counter `S << ++LastUnique` in `makeUniqueName`, so it will store `var1` as the value's name. Then the parser throws the "multiple definition" error immediately, so this suffix `1` will only exist for once(and also for a short time since the error will be thrown at once).

Moreover. I agree that `InstNameSize < NameStrSize` will be more intuitive for "name size overflowed". If `InstNameSize != NameStrSize + 1` and obviously `InstNameSize != NameStrSize`(considering the first if `Inst->getName() != NameStr`, if the size are the same, then the two strings should be exactly the same), then `InstNameSize < NameStrSize` exactly matches the case. I will update my diff later :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153722