Skip to content

Conversation

@jurahul
Copy link
Contributor

@jurahul jurahul commented Oct 4, 2024

Print assert message after the "assertion failed" message instead of printing it as a separate note. This makes the assert failure reporting less verbose and also more useful to see the failure message inline with the "assertion failed" message.

Print assert message after the "assertion failed" message instead
of printing it as a separate note. This makes the assert failure
reporting less verbose and also more useful to see the failure
message inline with the "assertion failed" message.
@jurahul jurahul marked this pull request as ready for review October 4, 2024 19:15
@jurahul jurahul requested a review from arsenm October 4, 2024 19:16
@jurahul jurahul requested a review from optimisan October 4, 2024 19:16
@llvmbot
Copy link
Member

llvmbot commented Oct 4, 2024

@llvm/pr-subscribers-tablegen

Author: Rahul Joshi (jurahul)

Changes

Print assert message after the "assertion failed" message instead of printing it as a separate note. This makes the assert failure reporting less verbose and also more useful to see the failure message inline with the "assertion failed" message.


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

2 Files Affected:

  • (modified) llvm/lib/TableGen/Error.cpp (+7-10)
  • (modified) llvm/test/TableGen/assert.td (+23-55)
diff --git a/llvm/lib/TableGen/Error.cpp b/llvm/lib/TableGen/Error.cpp
index 45cdcc8ae26193..6d1d5814223ab7 100644
--- a/llvm/lib/TableGen/Error.cpp
+++ b/llvm/lib/TableGen/Error.cpp
@@ -168,11 +168,10 @@ bool CheckAssert(SMLoc Loc, Init *Condition, Init *Message) {
     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)");
+    auto *MessageInit = dyn_cast<StringInit>(Message);
+    StringRef AssertMsg = MessageInit ? MessageInit->getValue()
+                                      : "(assert message is not a string)";
+    PrintError(Loc, "assertion failed: " + AssertMsg);
     return true;
   }
   return false;
@@ -180,12 +179,10 @@ bool CheckAssert(SMLoc Loc, Init *Condition, Init *Message) {
 
 // Dump a message to stderr.
 void dumpMessage(SMLoc Loc, Init *Message) {
-  auto *MessageInit = dyn_cast<StringInit>(Message);
-  if (!MessageInit) {
-    PrintError(Loc, "dump value is not of type string");
-  } else {
+  if (auto *MessageInit = dyn_cast<StringInit>(Message))
     PrintNote(Loc, MessageInit->getValue());
-  }
+  else
+    PrintError(Loc, "dump value is not of type string");
 }
 
 } // end namespace llvm
diff --git a/llvm/test/TableGen/assert.td b/llvm/test/TableGen/assert.td
index 17c08b43135319..c6c230da1be9ef 100644
--- a/llvm/test/TableGen/assert.td
+++ b/llvm/test/TableGen/assert.td
@@ -4,18 +4,13 @@
 // Test the assert statement at top level.
 // -----------------------------------------------------------------------------
 
-// CHECK: assertion failed
+// CHECK: [[FILE]]:[[@LINE+4]]:8: error: assertion failed: primary name is too long
 // CHECK-NOT: note: primary name is too short
-// CHECK: note: primary name is too long
-
 defvar Name = "Grace Brewster Murray Hopper";
-
 assert !ge(!size(Name), 20), "primary name is too short: " # Name;
 assert !le(!size(Name), 20), "primary name is too long: " # Name;
 
-// CHECK: assertion failed
-// CHECK: note: first name is incorrect
-
+// CHECK: assertion failed: first name is incorrect
 def Rec01 {
   string name = "Fred Smith";
 }
@@ -23,28 +18,21 @@ def Rec01 {
 assert !eq(!substr(Rec01.name, 0, 3), "Jane"),
        !strconcat("first name is incorrect: ", Rec01.name);
 
-// CHECK: assertion failed
-// CHECK: note: record Rec02 is broken
-
+// CHECK: assertion failed: record Rec02 is broken
 def Rec02 {
   bit broken = true;
 }
-
 assert !not(Rec02.broken), "record Rec02 is broken";
 
-// CHECK: assertion failed
-// CHECK: note: cube of 9
-
+// CHECK: assertion failed: cube of 9 should be 729
 class Cube<int n> {
   int result = !mul(n, n, n);
 }
-
 assert !eq(Cube<9>.result, 81), "cube of 9 should be 729";
 
-// CHECK: assertion failed
-// CHECK: note: foreach i cannot be 2
-// CHECK-NOT: note: foreach i cannot be 2
-
+// Verify that the assert fails only once.
+// CHECK: assertion failed: foreach i cannot be 2
+// CHECK-NOT: assertion failed: foreach i cannot be 2
 foreach i = 1...3 in {
   assert !ne(i, 2), "foreach i cannot be 2";
   def bar_ # i;
@@ -54,11 +42,9 @@ foreach i = 1...3 in {
 // Test the assert statement in a record definition.
 // -----------------------------------------------------------------------------
 
-// CHECK: [[FILE]]:[[@LINE+8]]:10: error: assertion failed
+// CHECK: [[FILE]]:[[@LINE+6]]:10: error: assertion failed: primary first name is not "Grack"
 // CHECK-NOT: primary first name is not "Grace"
-// 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+6]]:10: error: assertion failed: 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\"";
@@ -67,8 +53,7 @@ def Rec10 {
   assert !eq(foo, "foo"), "foo field should be \"Foo\"";
 }
 
-// CHECK: [[FILE]]:[[@LINE+5]]:10: error: assertion failed
-// CHECK: note: magic field is incorrect: 42
+// CHECK: [[FILE]]:[[@LINE+4]]:10: error: assertion failed: magic field is incorrect: 42
 // CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record
 def Rec11 {
   int magic = 13;
@@ -76,8 +61,7 @@ def Rec11 {
   let magic = 42;       
 }
 
-// CHECK: [[FILE]]:[[@LINE+6]]:10: error: assertion failed
-// CHECK: note: var field has wrong value
+// CHECK: [[FILE]]:[[@LINE+5]]:10: error: assertion failed: var field has wrong value
 // CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record
 def Rec12 {
   defvar prefix = "foo_";
@@ -85,8 +69,7 @@ def Rec12 {
   assert !eq(var, "foo_snorx"), "var field has wrong value: " # var;
 }
 
-// CHECK: assertion failed
-// CHECK: note: kind field has wrong value
+// CHECK: assertion failed: kind field has wrong value
 class Kind {
   int kind = 7;
 }
@@ -97,8 +80,7 @@ def Rec13 : Kind {
   assert !eq(kind, 7), "kind field has wrong value: " # kind;
 }
 
-// CHECK: assertion failed
-// CHECK: note: double_result should be
+// CHECK: assertion failed: 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);
@@ -122,13 +104,11 @@ class Person<string name, int age> : PersonName<name> {
 
 def Rec20 : Person<"Donald Knuth", 60>;
 
-// CHECK: assertion failed
-// CHECK: note: person name is too long
+// CHECK: assertion failed: 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: assertion failed: person age is invalid
 // CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record
 def Rec22 : Person<"Donald Knuth", 150>;
 
@@ -138,16 +118,14 @@ def Rec30 {
   int Age = Person<"Margaret Heafield Hamilton", 25>.Age;
 }
 
-// CHECK: assertion failed
-// CHECK: note: person name is too long
+// CHECK: assertion failed: 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: assertion failed: 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;
@@ -158,11 +136,8 @@ def Rec32 {
 // Test the assert statement in a multiclass.
 // -----------------------------------------------------------------------------
 
-// CHECK: assertion failed
-// CHECK: note: MC1 id string is too long
-// CHECK: assertion failed
-// CHECK: note: MC1 seq is too high
-
+// CHECK: assertion failed: MC1 id string is too long
+// CHECK: assertion failed: MC1 seq is too high
 multiclass MC1<string id, int seq> {
   assert !le(!size(id), 5), "MC1 id string is too long";
   assert !le(seq, 999999), "MC1 seq is too high";
@@ -177,9 +152,7 @@ defm Rec40 : MC1<"ILISP", 999>;
 defm Rec41 : MC1<"ILISPX", 999>;
 defm Rec42 : MC1<"ILISP", 999999999>;
 
-// CHECK: assertion failed
-// CHECK: note: MC2 phrase must be secret: secrex code
-
+// CHECK: assertion failed: MC2 phrase must be secret: secrex code
 multiclass MC2<string phr> {
   assert !eq(!substr(phr, 0, 6), "secret"), "MC2 phrase must be secret: " # phr;
 
@@ -194,9 +167,7 @@ multiclass MC3<string phr> {
 
 defm Rec43 : MC3<"secrex code">;
 
-// CHECK: assertion failed
-// CHECK: note: MC2 phrase must be secret: xecret code
-
+// CHECK: assertion failed: MC2 phrase must be secret: xecret code
 multiclass MC4<string phr> : MC2<phr> {
   def _def;
 }
@@ -205,11 +176,8 @@ 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
-
+// CHECK: assertion failed: MC5 name must include a space: Ada_Lovelace
+// CHECK: assertion failed: 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;
 

@jurahul jurahul changed the title [TableGen] Print assert message in line with assert failure [TableGen] Print assert message inline with assert failure Oct 4, 2024
@jurahul jurahul merged commit e31e6f2 into llvm:main Oct 4, 2024
13 checks passed
@jurahul jurahul deleted the print_assert_message_with_assert_failure branch October 4, 2024 20:21
@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 4, 2024

LLVM Buildbot has detected a new failure on builder lldb-arm-ubuntu running on linaro-lldb-arm-ubuntu while building llvm at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/18/builds/4950

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
PASS: lldb-api :: lang/cpp/function-qualifiers/TestCppFunctionQualifiers.py (783 of 2809)
PASS: lldb-api :: lang/cpp/function_refs/TestFunctionRefs.py (784 of 2809)
UNSUPPORTED: lldb-api :: lang/cpp/gmodules/alignment/TestPchAlignment.py (785 of 2809)
UNSUPPORTED: lldb-api :: lang/cpp/gmodules/basic/TestWithModuleDebugging.py (786 of 2809)
UNSUPPORTED: lldb-api :: lang/cpp/gmodules/pch-chain/TestPchChain.py (787 of 2809)
PASS: lldb-api :: lang/cpp/global_operators/TestCppGlobalOperators.py (788 of 2809)
PASS: lldb-api :: lang/cpp/global_variables/TestCPPGlobalVariables.py (789 of 2809)
UNSUPPORTED: lldb-api :: lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py (790 of 2809)
UNSUPPORTED: lldb-api :: lang/cpp/gmodules/templates/TestGModules.py (791 of 2809)
PASS: lldb-api :: lang/cpp/incompatible-class-templates/TestCppIncompatibleClassTemplates.py (792 of 2809)
FAIL: lldb-api :: lang/c/shared_lib_stripped_symbols/TestSharedLibStrippedSymbols.py (793 of 2809)
******************** TEST 'lldb-api :: lang/c/shared_lib_stripped_symbols/TestSharedLibStrippedSymbols.py' FAILED ********************
Script:
--
/usr/bin/python3.10 /home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env ARCHIVER=/usr/local/bin/llvm-ar --env OBJCOPY=/usr/bin/llvm-objcopy --env LLVM_LIBS_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin --arch armv8l --build-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/lldb --compiler /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/clang --dsymutil /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/dsymutil --llvm-tools-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin --lldb-obj-root /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/tools/lldb --lldb-libs-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./lib /home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/lang/c/shared_lib_stripped_symbols -p TestSharedLibStrippedSymbols.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 20.0.0git (https://github.com/llvm/llvm-project.git revision e31e6f259ed22acfb2135700cb0482f84d6ea386)
  clang revision e31e6f259ed22acfb2135700cb0482f84d6ea386
  llvm revision e31e6f259ed22acfb2135700cb0482f84d6ea386
Skipping the following test categories: ['libc++', 'dsym', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang-arm) :: test_expr_dsym (TestSharedLibStrippedSymbols.SharedLibStrippedTestCase) (test case does not fall in any category of interest for this run) 
FAIL: LLDB (/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang-arm) :: test_expr_dwarf (TestSharedLibStrippedSymbols.SharedLibStrippedTestCase)
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang-arm) :: test_expr_dwo (TestSharedLibStrippedSymbols.SharedLibStrippedTestCase)
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang-arm) :: test_frame_variable_dsym (TestSharedLibStrippedSymbols.SharedLibStrippedTestCase) (test case does not fall in any category of interest for this run) 
XFAIL: LLDB (/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang-arm) :: test_frame_variable_dwarf (TestSharedLibStrippedSymbols.SharedLibStrippedTestCase)
XFAIL: LLDB (/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang-arm) :: test_frame_variable_dwo (TestSharedLibStrippedSymbols.SharedLibStrippedTestCase)
======================================================================
FAIL: test_expr_dwarf (TestSharedLibStrippedSymbols.SharedLibStrippedTestCase)
   Test that types work when defined in a shared library and forwa/d-declared in the main executable
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1769, in test_method
    return attrvalue(self)
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/lang/c/shared_lib_stripped_symbols/TestSharedLibStrippedSymbols.py", line 24, in test_expr
    self.expect(
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 2370, in expect
    self.runCmd(
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1000, in runCmd
    self.assertTrue(self.res.Succeeded(), msg + output)
AssertionError: False is not true : Variable(s) displayed correctly
Error output:

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.

4 participants