TableGen: More helpful error messages

[nhaehnle]

Summary

Change-Id: Ic78afd0cd765fdb4cf1b7ecfb6bba22653ce6d29

[tra]

One nit. LGTM otherwise.

lib/TableGen/Record.cpp

1839

You are constructing a std::string only to use it to build another Twine. Why not just use Twine here?

[nhaehnle]

lib/TableGen/Record.cpp

1839

Because of Twine lifetime limitations. Twine works by chaining together temporary node objects to represent the string concatenations. So changing Type to type Twine can't work, because the temporary node objects would be destroyed again immediately, before they are passed to PrintFatalError.

(The compiler actually prevents attempts to do this, since Twine::operator= is deleted.)

I think it would work to "inline" Type in the argument to PrintFatalError, guarding it with a ternary ?: operator, but that seemed uglier to me.

[tra]

lib/TableGen/Record.cpp

1839

Thank you for the explanation. Reading ADT/Twine.h was enlightening. Now std::string makes sense to me.

[nhaehnle]

r327118

lib/TableGen/Record.cpp

 }
 
 void Record::resolveReferences(Resolver &R, const RecordVal *SkipVal) {
   for (RecordVal &Value : Values) {
     if (SkipVal == &Value) // Skip resolve the same field as the given one
       continue;
     if (Init *V = Value.getValue()) {
       Init *VR = V->resolveReferences(R);
-      if (Value.setValue(VR))
-        PrintFatalError(getLoc(), "Invalid value is found when setting '" +
+      if (Value.setValue(VR)) {
+        std::string Type;
+        if (TypedInit *VRT = dyn_cast<TypedInit>(VR))
+          Type =
+              (Twine("of type '") + VRT->getType()->getAsString() + "' ").str();
+        PrintFatalError(getLoc(), Twine("Invalid value ") + Type +
+                                      "is found when setting '" +
                                       Value.getNameInitAsString() +
+                                      " of type '" +
+                                      Value.getType()->getAsString() +
                                       "' after resolving references: " +
                                       VR->getAsUnquotedString() + "\n");
       }
     }
+  }
   Init *OldName = getNameInit();
   Init *NewName = Name->resolveReferences(R);
   if (NewName != OldName) {
     // Re-register with RecordKeeper.
     setName(NewName);
   }
 }
 

 int64_t Record::getValueAsInt(StringRef FieldName) const {
   const RecordVal *R = getValue(FieldName);
   if (!R || !R->getValue())
     PrintFatalError(getLoc(), "Record `" + getName() +
       "' does not have a field named `" + FieldName + "'!\n");
 
   if (IntInit *II = dyn_cast<IntInit>(R->getValue()))
     return II->getValue();
-  PrintFatalError(getLoc(), "Record `" + getName() + "', field `" +
-    FieldName + "' does not have an int initializer!");
+  PrintFatalError(getLoc(), Twine("Record `") + getName() + "', field `" +
+                                FieldName +
+                                "' does not have an int initializer: " +
+                                R->getValue()->getAsString());
 }
 
 std::vector<int64_t>
 Record::getValueAsListOfInts(StringRef FieldName) const {
   ListInit *List = getValueAsListInit(FieldName);
   std::vector<int64_t> Ints;
   for (Init *I : List->getValues()) {
     if (IntInit *II = dyn_cast<IntInit>(I))
       Ints.push_back(II->getValue());
     else
-      PrintFatalError(getLoc(), "Record `" + getName() + "', field `" +
-        FieldName + "' does not have a list of ints initializer!");
+      PrintFatalError(getLoc(),
+                      Twine("Record `") + getName() + "', field `" + FieldName +
+                          "' does not have a list of ints initializer: " +
+                          I->getAsString());
   }
   return Ints;
 }
 
 std::vector<StringRef>
 Record::getValueAsListOfStrings(StringRef FieldName) const {
   ListInit *List = getValueAsListInit(FieldName);
   std::vector<StringRef> Strings;
   for (Init *I : List->getValues()) {
     if (StringInit *SI = dyn_cast<StringInit>(I))
       Strings.push_back(SI->getValue());
     else
-      PrintFatalError(getLoc(), "Record `" + getName() + "', field `" +
-        FieldName + "' does not have a list of strings initializer!");
+      PrintFatalError(getLoc(),
+                      Twine("Record `") + getName() + "', field `" + FieldName +
+                          "' does not have a list of strings initializer: " +
+                          I->getAsString());
   }
   return Strings;
 }
 
 Record *Record::getValueAsDef(StringRef FieldName) const {
   const RecordVal *R = getValue(FieldName);
   if (!R || !R->getValue())
     PrintFatalError(getLoc(), "Record `" + getName() +