[PATCH] D102707: Fix non-global-value-max-name-size not considered by LLParser

[serge via Phabricator via llvm-commits]

serge-sans-paille added inline comments.


================
Comment at: llvm/include/llvm/IR/ValueSymbolTable.h:74
   /// Lookup a named Value.
-  Value *lookup(StringRef Name) const { return vmap.lookup(Name); }
+  virtual Value *lookup(StringRef Name) const;
 
----------------
@mehdi_amini are we fine with making a few methods here virtual? The approach proposed by @hasyimibhar is elegant, but I'm worried about the performance impact.

An alternative would be to have something like (pseudo code)

```
class LocalValueSymbolTable {
    ValueSymbolTable table;
    public:
      using ValueSymbolTable::size;  // nothing to change for that one
      Value *lookup(StringRef Name) const { // cap then forward to table }
    private:
      ValueName *createValueName(StringRef Name, Value *V); { // cap then forward to table }
```
      
      


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102707