[TableGen] Improve error message for semicolon after braced body

[Paul-C-Anagnostopoulos]

Summary

This revision improves the error message resulting from a braced body followed by a semicolon, in defs, classes, and multiclasses.

def foo {

...

};

https://bugs.llvm.org/show_bug.cgi?id=48706

[craig.topper]

Can we write tests for this?

llvm/lib/TableGen/TGParser.cpp

2839

I don't think there should be a comma between two choices.

2856

I think "A class or def body should not end with a semicolon" reads better to me.

3446

Similar here.

[Paul-C-Anagnostopoulos]

I will write tests for these errors.

llvm/lib/TableGen/TGParser.cpp

2839

Here "to start body" only applies to the second choice, so the comma disambiguates the sentence. Let me actually rewrite it so it's clear.

2856

Agreed.

[Paul-C-Anagnostopoulos]

I incorporated Craig's suggestions.

I will add a new test on Tuesday.

[Paul-C-Anagnostopoulos]

What am I waiting for? Here is a test of this new message.

[craig.topper]

llvm/test/TableGen/spurious-semi.td

23

I thought we change the error message to say "should not"?

[Paul-C-Anagnostopoulos]

I fixed the broken test. Thanks for catching this, Craig.

[lattner]

Looks great, thanks Paul!

llvm/lib/TableGen/TGParser.cpp

 ///   BodyList BodyItem*
 ///
 bool TGParser::ParseBody(Record *CurRec) {
   // If this is a null definition, just eat the semi and return.
   if (consume(tgtok::semi))
     return false;
 
   if (!consume(tgtok::l_brace))
-    return TokError("Expected ';' or '{' to start body");
+    return TokError("Expected '{' to start body or ';' for declaration only");
 
   // An object body introduces a new scope for local variables.
   TGLocalVarScope *BodyScope = PushLocalScope();
 
   while (Lex.getCode() != tgtok::r_brace)
     if (ParseBodyItem(CurRec))
       return true;
 
   PopLocalScope(BodyScope);
 
   // Eat the '}'.
   Lex.Lex();
+
+  // If we have a semicolon, print a gentle error.
+  SMLoc SemiLoc = Lex.getLoc();
+  if (consume(tgtok::semi)) {
+    PrintError(SemiLoc, "A class or def body should not end with a semicolon");
+    PrintNote("Semicolon ignored; remove to eliminate this error");    
+  }
+
   return false;
 }
 
 /// Apply the current let bindings to \a CurRec.
 /// \returns true on error, false otherwise.
 bool TGParser::ApplyLetStack(Record *CurRec) {
   for (SmallVectorImpl<LetRecord> &LetInfo : LetStack)
     for (LetRecord &LR : LetInfo)
while (Lex.getCode() != tgtok::r_brace) {
       case tgtok::Let:
         if (ParseObject(CurMultiClass))
           return true;
         break;
       }
     }
     Lex.Lex();  // eat the '}'.
 
+    // If we have a semicolon, print a gentle error.
+    SMLoc SemiLoc = Lex.getLoc();
+    if (consume(tgtok::semi)) {
+      PrintError(SemiLoc, "A multiclass body should not end with a semicolon");
+      PrintNote("Semicolon ignored; remove to eliminate this error");    
+    }
+
     PopLocalScope(MulticlassScope);
   }
 
   CurMultiClass = nullptr;
   return false;
 }
 
 /// ParseDefm - Parse the instantiation of a multiclass.

 bool TGParser::ParseFile() {
   Lex.Lex(); // Prime the lexer.
   if (ParseObjectList()) return true;
 
   // If we have unread input at the end of the file, report it.
   if (Lex.getCode() == tgtok::Eof)
     return false;
 
-  return TokError("Unexpected input at top level");
+  return TokError("Unexpected token at top level");
 }
 
 // Check an assertion: Obtain the condition value and be sure it is true.
 // If not, print a nonfatal error along with the message.
 void TGParser::CheckAssert(SMLoc Loc, Init *Condition, Init *Message) {
   auto *CondValue = dyn_cast_or_null<IntInit>(
                         Condition->convertInitializerTo(IntRecTy::get()));
   if (CondValue) {

llvm/test/TableGen/spurious-semi.td

+// RUN: llvm-tblgen %s | FileCheck %s
+// RUN: not llvm-tblgen -DERROR1 %s 2>&1 | FileCheck --check-prefix=ERROR1 %s
+
+// This file tests the error message that is printed when a body is
+// terminated with a semicolon in addition to the close brace.
+
+// CHECK: class Class0
+// CHECK: def Rec0
+
+class Class0 {
+}
+
+def Rec0 {
+}
+
+multiclass MC0 {
+  def R;
+}
+
+#ifdef ERROR1
+
+// ERROR1: error: A class or def body should not end with a semicolon
+// ERROR1: Semicolon ignored
+// ERROR1: error: A class or def body should not end with a semicolon
+// ERROR1: Semicolon ignored
+// ERROR1: error: A multiclass body should not end with a semicolon
+// ERROR1: Semicolon ignored
+
+class Class1 {
+};
+
+def Rec1 {
+};
+
+multiclass MC1 {
+  def R;
+};
+
+#endif