[PATCH] D43564: TableGen: Introduce an abstract variable resolver interface

[Artem Belevich via Phabricator via llvm-commits]

tra added inline comments.


================
Comment at: include/llvm/TableGen/Record.h:1631
+
+  virtual bool keepUnsetBits() const { return false; }
+};
----------------
Please describe the purpose of this function. It's not clear what are those unset bits and why would we want (or not) to keep them.


================
Comment at: include/llvm/TableGen/Record.h:1642
+
+  Init *resolve(Init *VarName) override;
+
----------------
This needs documentation, too. Does it accept nullptr? What does it return? Can it return nullptr? If so, what does it mean? 


================
Comment at: lib/TableGen/Record.cpp:1837-1858
+Init *RecordResolver::resolve(Init *VarName) {
+  Init *Val = Cache.lookup(VarName);
+  if (Val)
+    return Val;
+
+  for (Init *S : Stack) {
+    if (S == VarName)
----------------
Do we ever cache objects that get deallocated? 
If so, using pointers for cache is problematic VarName may have been deallocated and re-allocated to represent something else since it was cached. Perhaps you want to use fully qualified var name as the key instead.


Repository:
  rL LLVM

https://reviews.llvm.org/D43564