Enhance TableGen error message capabilities

[Paul-C-Anagnostopoulos]

Summary

This is the first step in enhancing TableGen's error message capabilities. I added the source location to the RecordVal class, so that now source locations are associated with both the Record and RecordVal classes. This allows more precise error messages in backends. I added two PrintFatalError methods that accept a Record or a RecordVal.

As a test of this, I improved the error messages in the SearchableTableEmitter backend. Every message now includes at least the record definition location and quite often the field definition location. I also made its messages more consistent.

[lattner]

This patch looks generally good to me (but please fix the clang format issues if you can), but this is detailed enough that it would be best to find another reviewer to give you a full tech review, as I haven't been in this code for (too) many years :)

[Paul-C-Anagnostopoulos]

Thanks! I will definitely fix the formatting issues.

@nhaehnle: Can you review this? Or recommend another reviewer?

[Paul-C-Anagnostopoulos]

I reformatted the code according to the Lint suggestions.

[madhur13490]

llvm/lib/TableGen/Record.cpp

2071

I think function can be very well written with early-return pattern i.e. check for the condition, if fails, return false. This way you can avoid nested "if" blocks.

2082

Please capitalize induction variable.

[Paul-C-Anagnostopoulos]

@madhur13490 I simply copied the preceding setValue() function. Let me commit this the way it is and then I will revisit both functions.

I will capitalize the induction variable.

[madhur13490]

In D87393#2271038, @Paul-C-Anagnostopoulos wrote:

@madhur13490 I simply copied the preceding setValue() function. Let me commit this the way it is and then I will revisit both functions.

I will capitalize the induction variable.

Sure, thanks!

[Paul-C-Anagnostopoulos]

I capitalized the I and E variables in the two SetValue() functions.

I will revisit them in a future pass to reorganize for early returns.

[Paul-C-Anagnostopoulos]

Just a bump to request a full review.

[madhur13490]

A lot of clang-format errors still exists. Looks okay otherwise.

[Paul-C-Anagnostopoulos]

I will fix the Clang errors that won't cause incompatibilities and that aren't pointless. Thanks!

llvm/include/llvm/TableGen/Error.h

 #ifndef LLVM_TABLEGEN_ERROR_H
 #define LLVM_TABLEGEN_ERROR_H
 
+#include "llvm/TableGen/Record.h"
 #include "llvm/Support/SourceMgr.h"
 
 namespace llvm {

 LLVM_ATTRIBUTE_NORETURN void PrintFatalError(const Twine &Msg);
 LLVM_ATTRIBUTE_NORETURN void PrintFatalError(ArrayRef<SMLoc> ErrorLoc,
                                              const Twine &Msg);
+LLVM_ATTRIBUTE_NORETURN void PrintFatalError(const Record *Rec,
+                                             const Twine &Msg);
+LLVM_ATTRIBUTE_NORETURN void PrintFatalError(const RecordVal *RecVal,
+                                             const Twine &Msg);
 
 extern SourceMgr SrcMgr;
 extern unsigned ErrorsPrinted;

llvm/include/llvm/TableGen/Record.h

   friend class Record;
 
   Init *Name;
+  SMLoc Loc; // Source location of definition of name.
   PointerIntPair<RecTy *, 1, bool> TyAndPrefix;
   Init *Value;
 
 public:
   RecordVal(Init *N, RecTy *T, bool P);
+  RecordVal(Init *N, SMLoc Loc, RecTy *T, bool P);
 
   StringRef getName() const;
   Init *getNameInit() const { return Name; }

     return getNameInit()->getAsUnquotedString();
   }
 
+  const SMLoc &getLoc() const { return Loc; }
   bool getPrefix() const { return TyAndPrefix.getInt(); }
   RecTy *getType() const { return TyAndPrefix.getPointer(); }
   Init *getValue() const { return Value; }
 
   bool setValue(Init *V);
+  bool setValue(Init *V, SMLoc NewLoc);
 
   void dump() const;
   void print(raw_ostream &OS, bool PrintSem = true) const;

llvm/lib/TableGen/Error.cpp

 //===----------------------------------------------------------------------===//
 
 #include "llvm/TableGen/Error.h"
+#include "llvm/TableGen/Record.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"

   std::exit(1);
 }
 
+// This method takes a Record and uses the source location
+// stored in it.
+void PrintFatalError(const Record *Rec, const Twine &Msg) {
+  PrintError(Rec->getLoc(), Msg);
+  // The following call runs the file cleanup handlers.
+  sys::RunInterruptHandlers();
+  std::exit(1);
+}
+
+// This method takes a RecordVal and uses the source location
+// stored in it.
+void PrintFatalError(const RecordVal *RecVal, const Twine &Msg) {
+  PrintError(RecVal->getLoc(), Msg);
+  // The following call runs the file cleanup handlers.
+  sys::RunInterruptHandlers();
+  std::exit(1);
+}
+
 } // end namespace llvm

llvm/lib/TableGen/Record.cpp

   assert(Value && "Cannot create unset value for current type!");
 }
 
+// This constructor accepts the same arguments as the above, but also
+// a source location.
+RecordVal::RecordVal(Init *N, SMLoc Loc, RecTy *T, bool P)
+  : Name(N), Loc(Loc), TyAndPrefix(T, P) {
+  setValue(UnsetInit::get());
+  assert(Value && "Cannot create unset value for current type!");
+}
+
 StringRef RecordVal::getName() const {
   return cast<StringInit>(getNameInit())->getValue();
 }

   return false;
 }
 
+// This version of setValue takes an source location and resets the
+// location in the RecordVal.
+bool RecordVal::setValue(Init *V, SMLoc NewLoc) {
+  Loc = NewLoc;
+  if (V) {
+    Value = V->getCastTo(getType());
+    if (Value) {
+      assert(!isa<TypedInit>(Value) ||
+             cast<TypedInit>(Value)->getType()->typeIsA(getType()));
+      if (BitsRecTy *BTy = dyn_cast<BitsRecTy>(getType())) {
+        if (!isa<BitsInit>(Value)) {
+          SmallVector<Init *, 64> Bits;
+          Bits.reserve(BTy->getNumBits());
+          for (unsigned i = 0, e = BTy->getNumBits(); i < e; ++i)
+            Bits.push_back(Value->getBit(i));
+          Value = BitsInit::get(Bits);
+        }
+      }
+    }
+    return Value == nullptr;
+  }
+  Value = nullptr;
+  return false;
+}
+
+#include "llvm/TableGen/Record.h"
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 LLVM_DUMP_METHOD void RecordVal::dump() const { errs() << *this; }
 #endif

     return CI->getValue();
 
   PrintFatalError(getLoc(), "Record `" + getName() + "', field `" +
-    FieldName + "' does not have a string initializer!");
+    FieldName + "' exists but does not have a string value");
 }
 
 BitsInit *Record::getValueAsBitsInit(StringRef FieldName) const {

   if (BitsInit *BI = dyn_cast<BitsInit>(R->getValue()))
     return BI;
   PrintFatalError(getLoc(), "Record `" + getName() + "', field `" +
-    FieldName + "' does not have a BitsInit initializer!");
+    FieldName + "' exists but does not have a bits value");
 }
 
 ListInit *Record::getValueAsListInit(StringRef FieldName) const {

   if (ListInit *LI = dyn_cast<ListInit>(R->getValue()))
     return LI;
   PrintFatalError(getLoc(), "Record `" + getName() + "', field `" +
-    FieldName + "' does not have a list initializer!");
+    FieldName + "' exists but does not have a list value");
 }
 
 std::vector<Record*>

     return II->getValue();
   PrintFatalError(getLoc(), Twine("Record `") + getName() + "', field `" +
                                 FieldName +
-                                "' does not have an int initializer: " +
+                                "' exists but does not have an int value: " +
                                 R->getValue()->getAsString());
 }
 

     else
       PrintFatalError(getLoc(),
                       Twine("Record `") + getName() + "', field `" + FieldName +
-                          "' does not have a list of ints initializer: " +
+                          "' exists but does not have a list of ints value: " +
                           I->getAsString());
   }
   return Ints;

     else
       PrintFatalError(getLoc(),
                       Twine("Record `") + getName() + "', field `" + FieldName +
-                          "' does not have a list of strings initializer: " +
+                          "' exists but does not have a list of strings value: " +
                           I->getAsString());
   }
   return Strings;

llvm/lib/TableGen/TGParser.cpp

     V = BitsInit::get(NewBits);
   }
 
-  if (RV->setValue(V)) {
+  if (RV->setValue(V, Loc)) {
     std::string InitType;
     if (BitsInit *BI = dyn_cast<BitsInit>(V))
       InitType = (Twine("' of type bit initializer with length ") +
                   Twine(BI->getNumBits())).str();
     else if (TypedInit *TI = dyn_cast<TypedInit>(V))
       InitType = (Twine("' of type '") + TI->getType()->getAsString()).str();
-    return Error(Loc, "Value '" + ValName->getAsUnquotedString() +
+    return Error(Loc, "Field '" + ValName->getAsUnquotedString() +
                           "' of type '" + RV->getType()->getAsString() +
-                          "' is incompatible with initializer '" +
+                          "' is incompatible with value '" +
                           V->getAsString() + InitType + "'");
   }
   return false;

     return nullptr;
   }
 
-  case tgtok::XForEach: { // Value ::= !foreach '(' Id ',' Value ',' Value ')'
+  case tgtok::XForEach: {
+    // Value ::= !foreach '(' Id ',' Value ',' Value ')'
     SMLoc OpLoc = Lex.getLoc();
     Lex.Lex(); // eat the operation
     if (Lex.getCode() != tgtok::l_paren) {

       return nullptr;
     }
 
-    // We need to create a temporary record to provide a scope for the iteration
-    // variable while parsing top-level foreach's.
+    // We need to create a temporary record to provide a scope for the
+    // iteration variable.
     std::unique_ptr<Record> ParseRecTmp;
     Record *ParseRec = CurRec;
     if (!ParseRec) {

     return ParseOperationCond(CurRec, ItemType);
 
   case tgtok::XFoldl: {
-    // Value ::= !foldl '(' Id ',' Id ',' Value ',' Value ',' Value ')'
+    // Value ::= !foldl '(' Value ',' Value ',' Id ',' Id ',' Expr ')'
     Lex.Lex(); // eat the operation
     if (!consume(tgtok::l_paren)) {
       TokError("expected '(' after !foldl");

     }
     Lex.Lex(); // eat the ','
 
-    // We need to create a temporary record to provide a scope for the iteration
-    // variable while parsing top-level foreach's.
+    // We need to create a temporary record to provide a scope for the
+    // two variables.
     std::unique_ptr<Record> ParseRecTmp;
     Record *ParseRec = CurRec;
     if (!ParseRec) {

   }
 
   // Add the value.
-  if (AddValue(CurRec, IdLoc, RecordVal(DeclName, Type, HasField)))
+  if (AddValue(CurRec, IdLoc, RecordVal(DeclName, IdLoc, Type, HasField)))
     return nullptr;
 
   // If a value is present, parse it.

llvm/test/TableGen/cond-type.td

 class B<int dummy> : A<dummy> {}
 class C<int dummy> : A<dummy> {}
 
-// CHECK: Value 'x' of type 'C' is incompatible with initializer '{{.*}}' of type 'A'
+// CHECK: Field 'x' of type 'C' is incompatible with value '{{.*}}' of type 'A'
 class X<int cc, B b, C c> {
   C x = !cond(cc: b, 1 : c);
 }

llvm/test/TableGen/generic-tables.td

 }
 
 def DFoo : DEntry<"foo", 1>;
-// ERROR1: [[@LINE+1]]:1: error: Record 'DBar' in table 'DTable' is missing field 'Val1'
+// ERROR1: [[@LINE+1]]:1: error: Record 'DBar' for table 'DTable' is missing field 'Val1'
 def DBar : DEntry<"bar", ?>;
 
 def DTable : GenericTable {

llvm/test/TableGen/getsetop.td

   // assignment whose LHS demands an instance of Base, so we expect a
   // static (parse-time) type-checking error.
 
-  // ERROR1: error: Value 'noCast' of type 'Base' is incompatible with initializer '!getop(orig)' of type '{}'
+  // ERROR1: error: Field 'noCast' of type 'Base' is incompatible with value '!getop(orig)' of type '{}'
   Base noCast = !getop(orig);
 #endif
 

llvm/test/TableGen/if-type.td

 class B<int dummy> : A<dummy> {}
 class C<int dummy> : A<dummy> {}
 
-// CHECK: Value 'x' of type 'C' is incompatible with initializer '{{.*}}' of type 'A'
+// CHECK: Field 'x' of type 'C' is incompatible with value '{{.*}}' of type 'A'
 class X<int cc, B b, C c> {
   C x = !if(cc, b, c);
 }

llvm/test/TableGen/self-reference-typeerror.td

 // This kind of self-reference is discourage, but if you *really* want it, you
 // can force it with !cast.
 //
-// CHECK: Value 'A:x' of type 'A' is incompatible with initializer
+// CHECK: Field 'A:x' of type 'A' is incompatible with value
 def A0 : A<A0>;

llvm/utils/TableGen/SearchableTableEmitter.cpp

 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Format.h"

 
 struct SearchIndex {
   std::string Name;
+  SMLoc Loc;  // Source location of PrimaryKey or Key field definition.
   SmallVector<GenericField, 1> Fields;
   bool EarlyOut = false;
 };
 
 struct GenericTable {
   std::string Name;
+  ArrayRef<SMLoc> Locs; // Source locations from the Record instance.
   std::string PreprocessorGuard;
   std::string CppTypeName;
   SmallVector<GenericField, 2> Fields;

     TypeInArgument,
   };
 
-  std::string primaryRepresentation(const GenericField &Field, Init *I) {
+  std::string primaryRepresentation(SMLoc Loc, const GenericField &Field, Init *I) {
     if (StringInit *SI = dyn_cast<StringInit>(I))
       return SI->getAsString();
     else if (BitsInit *BI = dyn_cast<BitsInit>(I))

     else if (Field.Enum) {
       auto *Entry = Field.Enum->EntryMap[cast<DefInit>(I)->getDef()];
       if (!Entry)
-        PrintFatalError(Twine("Entry for field '") + Field.Name + "' is null");
+        PrintFatalError(Loc, Twine("Entry for field '") + Field.Name +
+                               "' is null");
       return std::string(Entry->first);
     }
-    PrintFatalError(Twine("invalid field type for field '") + Field.Name +
-                    "', expected: string, bits, bit or code");
+    PrintFatalError(Loc, Twine("invalid field type for field '") + 
+                           Field.Name + "'; expected: bit, bits, string, or code");
   }
 
   bool isIntrinsic(Init *I) {

     return isa<BitsInit>(I) || isa<CodeInit>(I) || isIntrinsic(I);
   }
 
-  std::string searchableFieldType(const GenericField &Field, TypeContext Ctx) {
+  std::string searchableFieldType(const GenericTable &Table,
+                                  const SearchIndex &Index,
+                                  const GenericField &Field,
+                                  TypeContext Ctx) {
     if (isa<StringRecTy>(Field.RecType)) {
       if (Ctx == TypeInStaticStruct)
         return "const char *";

         return "uint32_t";
       if (NumBits <= 64)
         return "uint64_t";
-      PrintFatalError(Twine("bitfield '") + Field.Name +
-                      "' too large to search");
+      PrintFatalError(Index.Loc, Twine("In table '") + Table.Name + 
+                                  "' lookup method '" + Index.Name +
+                                  "', key field '" + Field.Name +
+                                  "' of type bits is too large");
     } else if (Field.Enum || Field.IsIntrinsic || Field.IsInstruction)
       return "unsigned";
-    PrintFatalError(Twine("Field '") + Field.Name + "' has unknown type '" +
-                    Field.RecType->getAsString() + "' to search by");
+    PrintFatalError(Index.Loc, Twine("In table '") + Table.Name + 
+                                  "' lookup method '" + Index.Name +
+                                  "', key field '" + Field.Name +
+                                  "' has invalid type: " +
+                                  Field.RecType->getAsString());
   }
 
   void emitGenericTable(const GenericTable &Table, raw_ostream &OS);

 
   bool parseFieldType(GenericField &Field, Init *II);
   std::unique_ptr<SearchIndex>
-  parseSearchIndex(GenericTable &Table, StringRef Name,
+  parseSearchIndex(GenericTable &Table, const RecordVal *RecVal, StringRef Name,
                    const std::vector<StringRef> &Key, bool EarlyOut);
   void collectEnumEntries(GenericEnum &Enum, StringRef NameField,
                           StringRef ValueField,

       if (LHSv > RHSv)
         return false;
     } else {
-      std::string LHSs = primaryRepresentation(Field, LHSI);
-      std::string RHSs = primaryRepresentation(Field, RHSI);
+      std::string LHSs = primaryRepresentation(Index.Loc, Field, LHSI);
+      std::string RHSs = primaryRepresentation(Index.Loc, Field, RHSI);
 
       if (isa<StringRecTy>(Field.RecType)) {
         LHSs = StringRef(LHSs).upper();

   } else {
     OS << "  struct IndexType {\n";
     for (const auto &Field : Index.Fields) {
-      OS << "    " << searchableFieldType(Field, TypeInStaticStruct) << " "
+      OS << "    " << searchableFieldType(Table, Index, Field,
+                                          TypeInStaticStruct) << " "
          << Field.Name << ";\n";
     }
     OS << "    unsigned _index;\n";

         NeedComma = true;
 
         std::string Repr =
-            primaryRepresentation(Field, Entry.first->getValueInit(Field.Name));
+            primaryRepresentation(Index.Loc, Field,
+                                  Entry.first->getValueInit(Field.Name));
         if (isa<StringRecTy>(Field.RecType))
           Repr = StringRef(Repr).upper();
         OS << Repr;

   if (Index.EarlyOut) {
     const GenericField &Field = Index.Fields[0];
     std::string FirstRepr =
-        primaryRepresentation(Field, IndexRows[0]->getValueInit(Field.Name));
-    std::string LastRepr = primaryRepresentation(
+        primaryRepresentation(Index.Loc, Field,
+                              IndexRows[0]->getValueInit(Field.Name));
+    std::string LastRepr = primaryRepresentation(Index.Loc,
         Field, IndexRows.back()->getValueInit(Field.Name));
     OS << "  if ((" << Field.Name << " < " << FirstRepr << ") ||\n";
     OS << "      (" << Field.Name << " > " << LastRepr << "))\n";

 
   OS << "  struct KeyType {\n";
   for (const auto &Field : Index.Fields) {
-    OS << "    " << searchableFieldType(Field, TypeInTempStruct) << " "
+    OS << "    " << searchableFieldType(Table, Index, Field,
+                                        TypeInTempStruct) << " "
        << Field.Name << ";\n";
   }
   OS << "  };\n";

     if (isa<StringRecTy>(Field.RecType)) {
       OS << ".upper()";
       if (IsPrimary)
-        PrintFatalError(Twine("Use a secondary index for case-insensitive "
-                              "comparison of field '") +
-                        Field.Name + "' in table '" + Table.Name + "'");
+        PrintFatalError(Index.Loc, 
+                        Twine("In table '") + Table.Name +
+                          "', use a secondary lookup method for "
+                          "case-insensitive comparison of field '" +
+                          Field.Name + "'");
     }
   }
   OS << " };\n";

       OS << ", ";
     NeedComma = true;
 
-    OS << searchableFieldType(Field, TypeInArgument) << " " << Field.Name;
+    OS << searchableFieldType(Table, Index, Field, TypeInArgument)
+       << " " << Field.Name;
   }
   OS << ")";
 }

         OS << ", ";
       NeedComma = true;
 
-      OS << primaryRepresentation(Field, Entry->getValueInit(Field.Name));
+      OS << primaryRepresentation(Table.Locs[0], Field,
+                                  Entry->getValueInit(Field.Name));
     }
 
     OS << " }, // " << i << "\n";

 }
 
 std::unique_ptr<SearchIndex>
-SearchableTableEmitter::parseSearchIndex(GenericTable &Table, StringRef Name,
+SearchableTableEmitter::parseSearchIndex(GenericTable &Table,
+                                         const RecordVal *KeyRecVal,
+                                         StringRef Name,
                                          const std::vector<StringRef> &Key,
                                          bool EarlyOut) {
   auto Index = std::make_unique<SearchIndex>();
   Index->Name = std::string(Name);
+  Index->Loc = KeyRecVal->getLoc();
   Index->EarlyOut = EarlyOut;
 
   for (const auto &FieldName : Key) {
     const GenericField *Field = Table.getFieldByName(FieldName);
     if (!Field)
-      PrintFatalError(Twine("Search index '") + Name +
-                      "' refers to non-existing field '" + FieldName +
-                      "' in table '" + Table.Name + "'");
+      PrintFatalError(KeyRecVal,
+                      Twine("In table '") + Table.Name +
+                        "', 'PrimaryKey' or 'Key' refers to nonexistent field '" +
+                        FieldName + "'");
+                      
     Index->Fields.push_back(*Field);
   }
 
   if (EarlyOut && isa<StringRecTy>(Index->Fields[0].RecType)) {
-    PrintFatalError(
-        "Early-out is not supported for string types (in search index '" +
-        Twine(Name) + "'");
+    PrintFatalError(KeyRecVal,
+        Twine("In lookup method '") + Name + "', early-out is not " +
+          "supported for a first key field of type string");
   }
 
   return Index;

 void SearchableTableEmitter::collectTableEntries(
     GenericTable &Table, const std::vector<Record *> &Items) {
   if (Items.empty())
-    PrintWarning(Twine("Table '") + Table.Name + "' has no items");
+    PrintFatalError(Table.Locs, Twine("Table '") + Table.Name + 
+                                  "' has no entries");
 
   for (auto EntryRec : Items) {
     for (auto &Field : Table.Fields) {
       auto TI = dyn_cast<TypedInit>(EntryRec->getValueInit(Field.Name));
       if (!TI || !TI->isComplete()) {
-        PrintFatalError(EntryRec->getLoc(),
+        PrintFatalError(EntryRec, 
                         Twine("Record '") + EntryRec->getName() +
-                            "' in table '" + Table.Name +
+                            "' for table '" + Table.Name +
                             "' is missing field '" + Field.Name + "'");
       }
       if (!Field.RecType) {

       } else {
         RecTy *Ty = resolveTypes(Field.RecType, TI->getType());
         if (!Ty)
-          PrintFatalError(Twine("Field '") + Field.Name + "' of table '" +
-                          Table.Name + "' has incompatible type: " +
-                          Field.RecType->getAsString() + " vs. " +
-                          TI->getType()->getAsString());
+          PrintFatalError(EntryRec->getValue(Field.Name), 
+                          Twine("Field '") + Field.Name + "' of table '" +
+                          Table.Name + "' entry has incompatible type: " +
+                          TI->getType()->getAsString() + " vs. " +
+                          Field.RecType->getAsString());
         Field.RecType = Ty;
       }
     }
 
-    Table.Entries.push_back(EntryRec);
+    Table.Entries.push_back(EntryRec);  // Add record to table's record list.
   }
 
   Record *IntrinsicClass = Records.getClass("Intrinsic");

     StringRef FilterClass = EnumRec->getValueAsString("FilterClass");
     Enum->Class = Records.getClass(FilterClass);
     if (!Enum->Class)
-      PrintFatalError(EnumRec->getLoc(), Twine("Enum FilterClass '") +
-                                             FilterClass + "' does not exist");
+      PrintFatalError(EnumRec->getValue("FilterClass"), 
+                      Twine("Enum FilterClass '") +
+                        FilterClass + "' does not exist");
 
     collectEnumEntries(*Enum, NameField, ValueField,
                        Records.getAllDerivedDefinitions(FilterClass));

   for (auto TableRec : Records.getAllDerivedDefinitions("GenericTable")) {
     auto Table = std::make_unique<GenericTable>();
     Table->Name = std::string(TableRec->getName());
+    Table->Locs = TableRec->getLoc();
     Table->PreprocessorGuard = std::string(TableRec->getName());
     Table->CppTypeName = std::string(TableRec->getValueAsString("CppTypeName"));
 
     std::vector<StringRef> Fields = TableRec->getValueAsListOfStrings("Fields");
     for (const auto &FieldName : Fields) {
-      Table->Fields.emplace_back(FieldName);
+      Table->Fields.emplace_back(FieldName);  // Construct a GenericField.
 
       if (auto TypeOfVal = TableRec->getValue(("TypeOf_" + FieldName).str())) {
         if (!parseFieldType(Table->Fields.back(), TypeOfVal->getValue())) {
-          PrintFatalError(TableRec->getLoc(),
+          PrintFatalError(TypeOfVal, 
                           Twine("Table '") + Table->Name +
-                              "' has bad 'TypeOf_" + FieldName +
+                              "' has invalid 'TypeOf_" + FieldName +
                               "': " + TypeOfVal->getValue()->getAsString());
         }
       }
     }
 
-    collectTableEntries(*Table, Records.getAllDerivedDefinitions(
-                                    TableRec->getValueAsString("FilterClass")));
+    StringRef FilterClass = TableRec->getValueAsString("FilterClass");
+    if (!Records.getClass(FilterClass))
+      PrintFatalError(TableRec->getValue("FilterClass"), 
+                      Twine("Table FilterClass '") +
+                        FilterClass + "' does not exist");
+
+    collectTableEntries(*Table, Records.getAllDerivedDefinitions(FilterClass));
 
     if (!TableRec->isValueUnset("PrimaryKey")) {
       Table->PrimaryKey =
-          parseSearchIndex(*Table, TableRec->getValueAsString("PrimaryKeyName"),
+          parseSearchIndex(*Table, TableRec->getValue("PrimaryKey"),
+                           TableRec->getValueAsString("PrimaryKeyName"),
                            TableRec->getValueAsListOfStrings("PrimaryKey"),
                            TableRec->getValueAsBit("PrimaryKeyEarlyOut"));
 

     Record *TableRec = IndexRec->getValueAsDef("Table");
     auto It = TableMap.find(TableRec);
     if (It == TableMap.end())
-      PrintFatalError(IndexRec->getLoc(),
+      PrintFatalError(IndexRec->getValue("Table"), 
                       Twine("SearchIndex '") + IndexRec->getName() +
-                          "' refers to non-existing table '" +
+                          "' refers to nonexistent table '" +
                           TableRec->getName());
 
     GenericTable &Table = *It->second;
     Table.Indices.push_back(parseSearchIndex(
-        Table, IndexRec->getName(), IndexRec->getValueAsListOfStrings("Key"),
+        Table, IndexRec->getValue("Key"),
+        IndexRec->getName(), IndexRec->getValueAsListOfStrings("Key"),
         IndexRec->getValueAsBit("EarlyOut")));
   }
 

 
     auto Table = std::make_unique<GenericTable>();
     Table->Name = (Twine(Class->getName()) + "sList").str();
+    Table->Locs = Class->getLoc();
     Table->PreprocessorGuard = Class->getName().upper();
     Table->CppTypeName = std::string(Class->getName());
 

          Class->getValueAsListOfStrings("SearchableFields")) {
       std::string Name =
           (Twine("lookup") + Table->CppTypeName + "By" + Field).str();
-      Table->Indices.push_back(parseSearchIndex(*Table, Name, {Field}, false));
+      Table->Indices.push_back(parseSearchIndex(*Table, Class->getValue(Field),
+                                                Name, {Field}, false));
     }
 
     Tables.emplace_back(std::move(Table));