Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TableGen] Print record location when record asserts fail #111029

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

jurahul
Copy link
Contributor

@jurahul jurahul commented Oct 3, 2024

When record assertions fail, print an error message with the record's location, so it's easier to see where the record that caused the assert to fail was instantiated. This is useful when the assert condition in a class depends on a template parameter, so we need to know the context of the definition to determine why the assert failed.

Also enhanced the assert.td test to check for these context messages, and also add checks for some assert failures that were missing in the test.

When record assertions fail, print an error message with the
record's location, so its easier to see where the record that caused
the assert to fail was instantiated. This is useful when the assert
condition in a class depends on a template parameter, so we need to
know the context of the definition to determine why the assert failed.

Also enhanced the assert.td test to check for these context messages,
and also add checks for some assert failures that were missing in the
test.
@jurahul jurahul marked this pull request as ready for review October 3, 2024 20:08
@jurahul jurahul requested a review from arsenm October 3, 2024 20:08
@llvmbot
Copy link
Member

llvmbot commented Oct 3, 2024

@llvm/pr-subscribers-tablegen

Author: Rahul Joshi (jurahul)

Changes

When record assertions fail, print an error message with the record's location, so it's easier to see where the record that caused the assert to fail was instantiated. This is useful when the assert condition in a class depends on a template parameter, so we need to know the context of the definition to determine why the assert failed.

Also enhanced the assert.td test to check for these context messages, and also add checks for some assert failures that were missing in the test.


Full diff: https://github.com/llvm/llvm-project/pull/111029.diff

4 Files Affected:

  • (modified) llvm/include/llvm/TableGen/Error.h (+2-1)
  • (modified) llvm/lib/TableGen/Error.cpp (+7-3)
  • (modified) llvm/lib/TableGen/Record.cpp (+9-1)
  • (modified) llvm/test/TableGen/assert.td (+28-15)
diff --git a/llvm/include/llvm/TableGen/Error.h b/llvm/include/llvm/TableGen/Error.h
index a51ed48dba0e86..512249b0160c24 100644
--- a/llvm/include/llvm/TableGen/Error.h
+++ b/llvm/include/llvm/TableGen/Error.h
@@ -48,7 +48,8 @@ void PrintError(const RecordVal *RecVal, const Twine &Msg);
 [[noreturn]] void PrintFatalError(const RecordVal *RecVal, const Twine &Msg);
 [[noreturn]] void PrintFatalError(function_ref<void(raw_ostream &OS)> PrintMsg);
 
-void CheckAssert(SMLoc Loc, Init *Condition, Init *Message);
+// Returns true if the assert failed.
+bool CheckAssert(SMLoc Loc, Init *Condition, Init *Message);
 void dumpMessage(SMLoc Loc, Init *Message);
 
 extern SourceMgr SrcMgr;
diff --git a/llvm/lib/TableGen/Error.cpp b/llvm/lib/TableGen/Error.cpp
index e21f369833be63..45cdcc8ae26193 100644
--- a/llvm/lib/TableGen/Error.cpp
+++ b/llvm/lib/TableGen/Error.cpp
@@ -160,18 +160,22 @@ void PrintFatalError(const RecordVal *RecVal, const Twine &Msg) {
 
 // Check an assertion: Obtain the condition value and be sure it is true.
 // If not, print a nonfatal error along with the message.
-void CheckAssert(SMLoc Loc, Init *Condition, Init *Message) {
+bool CheckAssert(SMLoc Loc, Init *Condition, Init *Message) {
   auto *CondValue = dyn_cast_or_null<IntInit>(Condition->convertInitializerTo(
       IntRecTy::get(Condition->getRecordKeeper())));
-  if (!CondValue)
+  if (!CondValue) {
     PrintError(Loc, "assert condition must of type bit, bits, or int.");
-  else if (!CondValue->getValue()) {
+    return true;
+  }
+  if (!CondValue->getValue()) {
     PrintError(Loc, "assertion failed");
     if (auto *MessageInit = dyn_cast<StringInit>(Message))
       PrintNote(MessageInit->getValue());
     else
       PrintNote("(assert message is not a string)");
+    return true;
   }
+  return false;
 }
 
 // Dump a message to stderr.
diff --git a/llvm/lib/TableGen/Record.cpp b/llvm/lib/TableGen/Record.cpp
index bebfb4b8c381c1..9307987df6a87d 100644
--- a/llvm/lib/TableGen/Record.cpp
+++ b/llvm/lib/TableGen/Record.cpp
@@ -3182,11 +3182,19 @@ void Record::checkRecordAssertions() {
   RecordResolver R(*this);
   R.setFinal(true);
 
+  bool AnyFailed = false;
   for (const auto &Assertion : getAssertions()) {
     Init *Condition = Assertion.Condition->resolveReferences(R);
     Init *Message = Assertion.Message->resolveReferences(R);
-    CheckAssert(Assertion.Loc, Condition, Message);
+    AnyFailed |= CheckAssert(Assertion.Loc, Condition, Message);
   }
+
+  if (!AnyFailed)
+    return;
+
+  // If any of the record assertions failed, print some context that will
+  // help see where the record that caused these assert failures is defined.
+  PrintError(this, "assertion failed in this record");
 }
 
 void Record::emitRecordDumps() {
diff --git a/llvm/test/TableGen/assert.td b/llvm/test/TableGen/assert.td
index d616adcb5d0d12..17c08b43135319 100644
--- a/llvm/test/TableGen/assert.td
+++ b/llvm/test/TableGen/assert.td
@@ -1,6 +1,8 @@
-// RUN: not llvm-tblgen %s 2>&1 | FileCheck %s
+// RUN: not llvm-tblgen %s 2>&1 | FileCheck %s -DFILE=%s
 
+// -----------------------------------------------------------------------------
 // Test the assert statement at top level.
+// -----------------------------------------------------------------------------
 
 // CHECK: assertion failed
 // CHECK-NOT: note: primary name is too short
@@ -48,14 +50,16 @@ foreach i = 1...3 in {
   def bar_ # i;
 }
 
+// -----------------------------------------------------------------------------
 // Test the assert statement in a record definition.
+// -----------------------------------------------------------------------------
 
-// CHECK: assertion failed
+// CHECK: [[FILE]]:[[@LINE+8]]:10: error: assertion failed
 // CHECK-NOT: primary first name is not "Grace"
-// CHECK: primary first name is not "Grack"
-// CHECK: assertion failed
-// CHECK: foo field should be
-
+// CHECK: note: primary first name is not "Grack"
+// CHECK: [[FILE]]:[[@LINE+7]]:10: error: assertion failed
+// CHECK: note: foo field should be
+// CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record
 def Rec10 {
   assert !eq(!substr(Name, 0, 5), "Grace"), "primary first name is not \"Grace\"";
   assert !eq(!substr(Name, 0, 5), "Grack"), "primary first name is not \"Grack\"";
@@ -63,18 +67,18 @@ def Rec10 {
   assert !eq(foo, "foo"), "foo field should be \"Foo\"";
 }
 
-// CHECK: assertion failed
+// CHECK: [[FILE]]:[[@LINE+5]]:10: error: assertion failed
 // CHECK: note: magic field is incorrect: 42
-
+// CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record
 def Rec11 {
   int magic = 13;
   assert !eq(magic, 13), "magic field is incorrect: " # magic;
   let magic = 42;       
 }
 
-// CHECK: assertion failed
+// CHECK: [[FILE]]:[[@LINE+6]]:10: error: assertion failed
 // CHECK: note: var field has wrong value
-
+// CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record
 def Rec12 {
   defvar prefix = "foo_";
   string var = prefix # "snork";
@@ -83,11 +87,11 @@ def Rec12 {
 
 // CHECK: assertion failed
 // CHECK: note: kind field has wrong value
-
 class Kind {
   int kind = 7;
 }
 
+// CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record
 def Rec13 : Kind {
   let kind = 8;
   assert !eq(kind, 7), "kind field has wrong value: " # kind;
@@ -95,13 +99,15 @@ def Rec13 : Kind {
 
 // CHECK: assertion failed
 // CHECK: note: double_result should be
-
+// CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record
 def Rec14 : Cube<3> {
   int double_result = !mul(result, 2);
   assert !eq(double_result, 53), "double_result should be 54";
 }
 
+// -----------------------------------------------------------------------------
 // Test the assert statement in a class definition.
+// -----------------------------------------------------------------------------
 
 class PersonName<string name> {
   assert !le(!size(name), 32), "person name is too long: " # name;
@@ -118,32 +124,39 @@ def Rec20 : Person<"Donald Knuth", 60>;
 
 // CHECK: assertion failed
 // CHECK: note: person name is too long
-
+// CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record
 def Rec21 : Person<"Donald Uh Oh This Name Is Too Long Knuth", 50>;
 
 // CHECK: assertion failed
 // CHECK: note: person age is invalid
-
+// CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record
 def Rec22 : Person<"Donald Knuth", 150>;
 
 // Test the assert statement in an anonymous class invocation.
-
 def Rec30 {
   string Name = Person<"Margaret Heafield Hamilton", 25>.Name;
   int Age = Person<"Margaret Heafield Hamilton", 25>.Age;
 }
 
+// CHECK: assertion failed
+// CHECK: note: person name is too long
+// CHECK: [[FILE]]:[[@LINE+2]]:17: error: assertion failed in this record
 def Rec31 {
   string Name = Person<"Margaret Heafield And More Middle Names Hamilton", 25>.Name;
   int Age = Person<"Margaret Heafield Hamilton", 25>.Age;
 }
 
+// CHECK: assertion failed
+// CHECK: note: person age is invalid: 0
+// CHECK: [[FILE]]:[[@LINE+3]]:13: error: assertion failed in this record
 def Rec32 {
   string Name = Person<"Margaret Heafield Hamilton", 25>.Name;
   int Age = Person<"Margaret Heafield Hamilton", 0>.Age;
 }
 
+// -----------------------------------------------------------------------------
 // Test the assert statement in a multiclass.
+// -----------------------------------------------------------------------------
 
 // CHECK: assertion failed
 // CHECK: note: MC1 id string is too long

@jurahul jurahul merged commit 04540fa into llvm:main Oct 4, 2024
12 checks passed
@jurahul jurahul deleted the tablegen_assert_context branch October 4, 2024 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants