[TableGen] Fix two bugs in 'defm' when a complex 'assert' is involved

[Paul-C-Anagnostopoulos]

Summary

This patch fixes two bugs that arise when a 'defm' inherits from a multiclass and also from a class with assertions. I discovered them while adding assertions to the Clang TableGen files.

I'm not sure whether this warrants a Phabricator review, but it certainly can't hurt.

[jansvoboda11]

LGTM.

llvm/test/TableGen/assert.td

197

I'd expect TableGen to report only the first assertion failure and abort immediately. What's the logic behind the current behavior?

[Paul-C-Anagnostopoulos]

llvm/test/TableGen/assert.td

197

Assertion failures are not fatal, so that other errors in the .td files can be reported at the same time.

[jansvoboda11]

llvm/test/TableGen/assert.td

197

What if the assertion failure is the only error in the .td file? Will TableGen exit with a non-zero code?

[Paul-C-Anagnostopoulos]

llvm/test/TableGen/assert.td

197

Yes. Both errors and fatal errors increment the error count, which is used to determine the exit code.

llvm/include/llvm/TableGen/Record.h

public:
   explicit Record(StringRef N, ArrayRef<SMLoc> locs, RecordKeeper &records,
                   bool Class = false)
       : Record(StringInit::get(N), locs, records, false, Class) {}
 
   // When copy-constructing a Record, we must still guarantee a globally unique
   // ID number. Don't copy CorrespondingDefInit either, since it's owned by the
   // original record. All other fields can be copied normally.
   Record(const Record &O)
     : Name(O.Name), Locs(O.Locs), TemplateArgs(O.TemplateArgs),
-      Values(O.Values), SuperClasses(O.SuperClasses),
+      Values(O.Values), Assertions(O.Assertions), SuperClasses(O.SuperClasses),
       TrackedRecords(O.TrackedRecords), ID(LastID++),
       IsAnonymous(O.IsAnonymous), IsClass(O.IsClass) { }
 
   static unsigned getNewUID() { return LastID++; }
 
   unsigned getID() const { return ID; }
 
   StringRef getName() const { return cast<StringInit>(Name)->getValue(); }

llvm/lib/TableGen/TGParser.cpp

bool TGParser::AddSubClass(Record *CurRec, SubClassReference &SubClass) {
   CurRec->addSuperClass(SC, SubClass.RefRange);
   return false;
 }
 
 bool TGParser::AddSubClass(RecordsEntry &Entry, SubClassReference &SubClass) {
   if (Entry.Rec)
     return AddSubClass(Entry.Rec.get(), SubClass);
 
+  if (Entry.Assertion)
+    return false;
+
   for (auto &E : Entry.Loop->Entries) {
     if (AddSubClass(E, SubClass))
       return true;
   }
 
   return false;
 }
 

llvm/test/TableGen/assert.td

 // CHECK: assertion failed
 // CHECK: note: MC2 phrase must be secret: xecret code
 
 multiclass MC4<string phr> : MC2<phr> {
   def _def;
 }
 
 defm Rec44 : MC4<"xecret code">;
+
+// Test a defm in a multiclass that inherits from a class with asserts.
+
+// CHECK: assertion failed
+// CHECK: note: MC5 name must include a space: Ada_Lovelace
+// CHECK: assertion failed
+// CHECK: note: person age is invalid: 666
+
+multiclass MC5<string phr, string name, int age> {
+  assert !ne(!find(name, " "), -1), "MC5 name must include a space: " # name;
+
+  defm _mc5 : MC2<phr>, Person<name, age>;
+}
+
+defm Rec45 : MC5<"secret password", "Ada_Lovelace", 666>;