TableGen: Resolve all template args simultaneously in AddSubClass

[nhaehnle]

Summary

Use the new resolver interface more explicitly, and avoid traversing
all the initializers multiple times.

Add a test case for a pattern that was broken by an earlier version
of this change.

An additional change is that we now remove *all* template arguments
after resolving them.

Change-Id: I86c828c8cc84c18b052dfe0f64c0d5cbf3c4e13c

llvm/trunk/include/llvm/TableGen/Record.h

public:
 
   void addSuperClass(Record *R, SMRange Range) {
     assert(!isSubClassOf(R) && "Already subclassing record!");
     SuperClasses.push_back(std::make_pair(R, Range));
   }
 
   /// If there are any field references that refer to fields
   /// that have been filled in, we can propagate the values now.
-  void resolveReferences() { resolveReferencesTo(nullptr); }
+  void resolveReferences();
+
+  /// Apply the resolver to the name of the record as well as to the
+  /// initializers of all fields of the record except SkipVal.
+  ///
+  /// The resolver should not resolve any of the fields itself, to avoid
+  /// recursion / infinite loops.
+  void resolveReferences(Resolver &R, const RecordVal *SkipVal = nullptr);
 
   /// If anything in this record refers to RV, replace the
   /// reference to RV with the RHS of RV.  If RV is null, we resolve all
   /// possible references.
   void resolveReferencesTo(const RecordVal *RV);
 
   RecordKeeper &getRecords() const {
     return TrackedRecords;

llvm/trunk/lib/TableGen/Record.cpp

void Record::setName(Init *NewName) {
   //
   // Nonetheless, it may be that some of this Record's values
   // reference the record name.  Indeed, the reason for having the
   // record name be an Init is to provide this flexibility.  The extra
   // resolve steps after completely instantiating defs takes care of
   // this.  See TGParser::ParseDef and TGParser::ParseDefm.
 }
 
-void Record::resolveReferencesTo(const RecordVal *RV) {
-  RecordResolver RecResolver(*this);
-  RecordValResolver RecValResolver(*this, RV);
-  Resolver *R;
-  if (RV)
-    R = &RecValResolver;
-  else
-    R = &RecResolver;
-
+void Record::resolveReferences(Resolver &R, const RecordVal *SkipVal) {
   for (RecordVal &Value : Values) {
-    if (RV == &Value) // Skip resolve the same field as the given one
+    if (SkipVal == &Value) // Skip resolve the same field as the given one
       continue;
-    if (Init *V = Value.getValue())
-      if (Value.setValue(V->resolveReferences(*R)))
+    if (Init *V = Value.getValue()) {
+      Init *VR = V->resolveReferences(R);
+      if (Value.setValue(VR))
         PrintFatalError(getLoc(), "Invalid value is found when setting '" +
                                       Value.getNameInitAsString() +
-                        "' after resolving references" +
-                        (RV ? " against '" + RV->getNameInitAsString() +
-                              "' of (" + RV->getValue()->getAsUnquotedString() +
-                              ")"
-                            : "") + "\n");
+                                      "' after resolving references: " +
+                                      VR->getAsUnquotedString() + "\n");
+    }
   }
   Init *OldName = getNameInit();
-  Init *NewName = Name->resolveReferences(*R);
+  Init *NewName = Name->resolveReferences(R);
   if (NewName != OldName) {
     // Re-register with RecordKeeper.
     setName(NewName);
   }
 }
 
+void Record::resolveReferences() {
+  RecordResolver R(*this);
+  resolveReferences(R);
+}
+
+void Record::resolveReferencesTo(const RecordVal *RV) {
+  RecordValResolver R(*this, RV);
+  resolveReferences(R, RV);
+}
+
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 LLVM_DUMP_METHOD void Record::dump() const { errs() << *this; }
 #endif
 
 raw_ostream &llvm::operator<<(raw_ostream &OS, const Record &R) {
   OS << R.getNameInitAsString();
 
   ArrayRef<Init *> TArgs = R.getTemplateArgs();

llvm/trunk/lib/TableGen/TGParser.cpp

bool TGParser::AddSubClass(Record *CurRec, SubClassReference &SubClass) {
 
   // Ensure that an appropriate number of template arguments are specified.
   if (TArgs.size() < SubClass.TemplateArgs.size())
     return Error(SubClass.RefRange.Start,
                  "More template args specified than expected");
 
   // Loop over all of the template arguments, setting them to the specified
   // value or leaving them as the default if necessary.
+  MapResolver R(CurRec);
+
   for (unsigned i = 0, e = TArgs.size(); i != e; ++i) {
     if (i < SubClass.TemplateArgs.size()) {
       // If a value is specified for this template arg, set it now.
       if (SetValue(CurRec, SubClass.RefRange.Start, TArgs[i],
                    None, SubClass.TemplateArgs[i]))
         return true;
-
-      // Resolve it next.
-      CurRec->resolveReferencesTo(CurRec->getValue(TArgs[i]));
-
-      // Now remove it.
-      CurRec->removeValue(TArgs[i]);
-
     } else if (!CurRec->getValue(TArgs[i])->getValue()->isComplete()) {
       return Error(SubClass.RefRange.Start,
                    "Value not specified for template argument #" +
                    Twine(i) + " (" + TArgs[i]->getAsUnquotedString() +
                    ") of subclass '" + SC->getNameInitAsString() + "'!");
     }
+
+    R.set(TArgs[i], CurRec->getValue(TArgs[i])->getValue());
+
+    CurRec->removeValue(TArgs[i]);
   }
 
+  CurRec->resolveReferences(R);
+
   // Since everything went well, we can now set the "superclass" list for the
   // current record.
   ArrayRef<std::pair<Record *, SMRange>> SCs = SC->getSuperClasses();
   for (const auto &SCPair : SCs) {
     if (CurRec->isSubClassOf(SCPair.first))
       return Error(SubClass.RefRange.Start,
                    "Already subclass of '" + SCPair.first->getName() + "'!\n");
     CurRec->addSuperClass(SCPair.first, SCPair.second);

llvm/trunk/test/TableGen/template-arg-dependency.td

+// RUN: llvm-tblgen %s | FileCheck %s
+// XFAIL: vg_leak
+
+// CHECK: --- Defs ---
+
+// CHECK: def A0 {
+// CHECK:   int ret = 3;
+// CHECK: }
+
+class A<int a,
+        int b = !add(a, 1),
+        int sum = !add(a, b)> {
+  int ret = sum;
+}
+
+def A0 : A<1>;