[PATCH] D41778: Improve diagnostics for instruction mapping

[Simon Dardis via Phabricator via llvm-commits]

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.

Some small nits inlined, otherwise LGTM. Please wait a few working days to let others chime in if they have suggestions before committing.



================
Comment at: test/TableGen/RelTest.td:3
+
+include "llvm/Target/Target.td"
+
----------------
Add a brief description of the purpose of this test. 


================
Comment at: utils/TableGen/CodeGenMapTable.cpp:246
     for (Init *RowField : RowFields->getValues()) {
-      Init *CurInstrVal = CurInstr->getValue(RowField)->getValue();
+      RecordVal *recVal = CurInstr->getValue(RowField);
+      if (recVal == nullptr)
----------------
recVal -> RecVal.


================
Comment at: utils/TableGen/CodeGenMapTable.cpp:248
+      if (recVal == nullptr)
+        PrintFatalError("No value " + RowField->getAsString() + " found in \"" +
+                        CurInstr->getName() + "\" instruction description.");
----------------
I believe you should use the overloaded variant of PrintFatalError that takes a source location of a record. i.e:

        PrintFatalError(CurInstr->getLoc(), "No value " + RowField->getAsString() + " found in \"" +
                        CurInstr->getName() + "\" instruction description.");


This gives the following error message:

   /Users/simon/dev/llvm/llvm/test/TableGen/RelTest.td:34:1: error: No value "BaseName" found in "SimpleInstr" instruction description.
   def SimpleInstr : SimpleRel, INSTR_DEF;
   ^

Which is a better in my opinion as directly points to the record which lacks the named value.


Repository:
  rL LLVM

https://reviews.llvm.org/D41778