[llvm] [mlir] [TableGen] Do not exit in template argument check (PR #121636)
Markus Böck via llvm-commits
llvm-commits at lists.llvm.org
Sat Jan 4 22:28:37 PST 2025
https://github.com/zero9178 updated https://github.com/llvm/llvm-project/pull/121636
>From 122c9da3a7cded6e9a5dd3a77bf016b381df9755 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20B=C3=B6ck?= <markus.boeck02 at gmail.com>
Date: Sat, 4 Jan 2025 11:59:46 +0100
Subject: [PATCH 1/3] [TableGen] Do not exit on template argument check
The signature of `CheckTemplateArgValues` implements error handling via the `bool` return type, yet always returned false.
The single possible error case instead used `PrintFatalError,` which exits the program afterward.
This behavior is undesirable: It prevents any further errors from being printed and makes TableGen less usable as a library as it crashes the entire process (e.g. `tblgen-lsp-server`).
This PR therefore fixes the issue by using `Error` instead and returning true if an error occurred. All callers already perform proper error handling.
As `llvm-tblgen` exits on error as well, a test was added to the LSP to ensure it exits normally.
---
llvm/lib/TableGen/TGParser.cpp | 9 +++++----
llvm/test/TableGen/template-args.td | 17 ++++++++++++++---
.../test/tblgen-lsp-server/templ-arg-check.test | 15 +++++++++++++++
3 files changed, 34 insertions(+), 7 deletions(-)
create mode 100644 mlir/test/tblgen-lsp-server/templ-arg-check.test
diff --git a/llvm/lib/TableGen/TGParser.cpp b/llvm/lib/TableGen/TGParser.cpp
index e8679439c81de3..d7a8bdefa1c508 100644
--- a/llvm/lib/TableGen/TGParser.cpp
+++ b/llvm/lib/TableGen/TGParser.cpp
@@ -4421,6 +4421,7 @@ bool TGParser::CheckTemplateArgValues(
const Record *ArgsRec) {
ArrayRef<const Init *> TArgs = ArgsRec->getTemplateArgs();
+ bool HasError = false;
for (const ArgumentInit *&Value : Values) {
const Init *ArgName = nullptr;
if (Value->isPositional())
@@ -4439,16 +4440,16 @@ bool TGParser::CheckTemplateArgValues(
"result of template arg value cast has wrong type");
Value = Value->cloneWithValue(CastValue);
} else {
- PrintFatalError(Loc, "Value specified for template argument '" +
- Arg->getNameInitAsString() + "' is of type " +
- ArgValue->getType()->getAsString() +
+ HasError |= Error(Loc, "Value specified for template argument '" +
+ Arg->getNameInitAsString() + "' is of type " +
+ ArgValue->getType()->getAsString() +
"; expected type " + ArgType->getAsString() +
": " + ArgValue->getAsString());
}
}
}
- return false;
+ return HasError;
}
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
diff --git a/llvm/test/TableGen/template-args.td b/llvm/test/TableGen/template-args.td
index f3eb02dd823ef5..40623dcb701e0d 100644
--- a/llvm/test/TableGen/template-args.td
+++ b/llvm/test/TableGen/template-args.td
@@ -9,6 +9,7 @@
// RUN: not llvm-tblgen -DERROR8 %s 2>&1 | FileCheck --check-prefix=ERROR8 %s
// RUN: not llvm-tblgen -DERROR9 %s 2>&1 | FileCheck --check-prefix=ERROR9 %s
// RUN: not llvm-tblgen -DERROR10 %s 2>&1 | FileCheck --check-prefix=ERROR10 %s
+// RUN: not llvm-tblgen -DERROR11 %s 2>&1 | FileCheck --check-prefix=ERROR11 %s
// This file tests that all required arguments are specified and template
// arguments are type-checked and cast if necessary.
@@ -158,13 +159,13 @@ defm MissingComma : TwoArgs<2 "two">;
#ifdef ERROR8
def error8: Class1;
// ERROR8: value not specified for template argument 'Class1:nm'
-// ERROR8: 18:21: note: declared in 'Class1'
+// ERROR8: 19:21: note: declared in 'Class1'
#endif
#ifdef ERROR9
defm error9: MC1;
// ERROR9: value not specified for template argument 'MC1::nm'
-// ERROR9: 99:23: note: declared in 'MC1'
+// ERROR9: 100:23: note: declared in 'MC1'
#endif
#ifdef ERROR10
@@ -172,5 +173,15 @@ def error10 {
int value = Class2<>.Code;
}
// ERROR10: value not specified for template argument 'Class2:cd'
-// ERROR10: 37:22: note: declared in 'Class2'
+// ERROR10: 38:22: note: declared in 'Class2'
+#endif
+
+#ifdef ERROR11
+
+class Foo<int i, int j>;
+
+def error11 : Foo<"", "">;
+// ERROR11: Value specified for template argument 'Foo:i' is of type string; expected type int: ""
+// ERROR11: Value specified for template argument 'Foo:j' is of type string; expected type int: ""
+
#endif
diff --git a/mlir/test/tblgen-lsp-server/templ-arg-check.test b/mlir/test/tblgen-lsp-server/templ-arg-check.test
new file mode 100644
index 00000000000000..cda9b79a1f4633
--- /dev/null
+++ b/mlir/test/tblgen-lsp-server/templ-arg-check.test
@@ -0,0 +1,15 @@
+// RUN: tblgen-lsp-server -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"tablegen","capabilities":{},"trace":"off"}}
+// -----
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{
+ "uri":"test:///foo.td",
+ "languageId":"tablegen",
+ "version":1,
+ "text":"class Foo<int i>;\ndef : Foo<\"\">;"
+}}}
+// CHECK: "method": "textDocument/publishDiagnostics",
+// CHECK: "message": "Value specified for template argument 'Foo:i' is of type string; expected type int: \"\"",
+// -----
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+// -----
+{"jsonrpc":"2.0","method":"exit"}
>From d166e60f3afc5f05fb83af66e8b85cd22e2766c0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20B=C3=B6ck?= <markus.boeck02 at gmail.com>
Date: Sat, 4 Jan 2025 12:22:04 +0100
Subject: [PATCH 2/3] clang-format
---
llvm/lib/TableGen/TGParser.cpp | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/TableGen/TGParser.cpp b/llvm/lib/TableGen/TGParser.cpp
index d7a8bdefa1c508..9d76cc123fe0cf 100644
--- a/llvm/lib/TableGen/TGParser.cpp
+++ b/llvm/lib/TableGen/TGParser.cpp
@@ -4440,11 +4440,11 @@ bool TGParser::CheckTemplateArgValues(
"result of template arg value cast has wrong type");
Value = Value->cloneWithValue(CastValue);
} else {
- HasError |= Error(Loc, "Value specified for template argument '" +
- Arg->getNameInitAsString() + "' is of type " +
- ArgValue->getType()->getAsString() +
- "; expected type " + ArgType->getAsString() +
- ": " + ArgValue->getAsString());
+ HasError |= Error(
+ Loc, "Value specified for template argument '" +
+ Arg->getNameInitAsString() + "' is of type " +
+ ArgValue->getType()->getAsString() + "; expected type " +
+ ArgType->getAsString() + ": " + ArgValue->getAsString());
}
}
}
>From cd933d0696dbc965aa9c864cb6501e7fd32fe77e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20B=C3=B6ck?= <markus.boeck02 at gmail.com>
Date: Sun, 5 Jan 2025 07:28:09 +0100
Subject: [PATCH 3/3] track argument locations
---
llvm/lib/TableGen/TGParser.cpp | 28 +++++++++++++++++-----------
llvm/lib/TableGen/TGParser.h | 4 +++-
llvm/test/TableGen/template-args.td | 4 ++--
3 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/llvm/lib/TableGen/TGParser.cpp b/llvm/lib/TableGen/TGParser.cpp
index 9d76cc123fe0cf..60ae11b7f42612 100644
--- a/llvm/lib/TableGen/TGParser.cpp
+++ b/llvm/lib/TableGen/TGParser.cpp
@@ -776,13 +776,14 @@ ParseSubClassReference(Record *CurRec, bool isDefm) {
return Result;
}
- if (ParseTemplateArgValueList(Result.TemplateArgs, CurRec, Result.Rec)) {
+ SmallVector<SMLoc> ArgLocs;
+ if (ParseTemplateArgValueList(Result.TemplateArgs, ArgLocs, CurRec,
+ Result.Rec)) {
Result.Rec = nullptr; // Error parsing value list.
return Result;
}
- if (CheckTemplateArgValues(Result.TemplateArgs, Result.RefRange.Start,
- Result.Rec)) {
+ if (CheckTemplateArgValues(Result.TemplateArgs, ArgLocs, Result.Rec)) {
Result.Rec = nullptr; // Error checking value list.
return Result;
}
@@ -812,7 +813,8 @@ ParseSubMultiClassReference(MultiClass *CurMC) {
return Result;
}
- if (ParseTemplateArgValueList(Result.TemplateArgs, &CurMC->Rec,
+ SmallVector<SMLoc> ArgLocs;
+ if (ParseTemplateArgValueList(Result.TemplateArgs, ArgLocs, &CurMC->Rec,
&Result.MC->Rec)) {
Result.MC = nullptr; // Error parsing value list.
return Result;
@@ -2722,11 +2724,12 @@ const Init *TGParser::ParseSimpleValue(Record *CurRec, const RecTy *ItemType,
}
SmallVector<const ArgumentInit *, 8> Args;
+ SmallVector<SMLoc> ArgLocs;
Lex.Lex(); // consume the <
- if (ParseTemplateArgValueList(Args, CurRec, Class))
+ if (ParseTemplateArgValueList(Args, ArgLocs, CurRec, Class))
return nullptr; // Error parsing value list.
- if (CheckTemplateArgValues(Args, NameLoc.Start, Class))
+ if (CheckTemplateArgValues(Args, ArgLocs, Class))
return nullptr; // Error checking template argument values.
if (resolveArguments(Class, Args, NameLoc.Start))
@@ -3201,8 +3204,8 @@ void TGParser::ParseValueList(SmallVectorImpl<const Init *> &Result,
// PostionalArgValueList ::= [Value {',' Value}*]
// NamedArgValueList ::= [NameValue '=' Value {',' NameValue '=' Value}*]
bool TGParser::ParseTemplateArgValueList(
- SmallVectorImpl<const ArgumentInit *> &Result, Record *CurRec,
- const Record *ArgsRec) {
+ SmallVectorImpl<const ArgumentInit *> &Result,
+ SmallVectorImpl<SMLoc> &ArgLocs, Record *CurRec, const Record *ArgsRec) {
assert(Result.empty() && "Result vector is not empty");
ArrayRef<const Init *> TArgs = ArgsRec->getTemplateArgs();
@@ -3217,7 +3220,7 @@ bool TGParser::ParseTemplateArgValueList(
return true;
}
- SMLoc ValueLoc = Lex.getLoc();
+ SMLoc ValueLoc = ArgLocs.emplace_back(Lex.getLoc());
// If we are parsing named argument, we don't need to know the argument name
// and argument type will be resolved after we know the name.
const Init *Value = ParseValue(
@@ -4417,12 +4420,15 @@ bool TGParser::ParseFile() {
// If necessary, replace an argument with a cast to the required type.
// The argument count has already been checked.
bool TGParser::CheckTemplateArgValues(
- SmallVectorImpl<const ArgumentInit *> &Values, SMLoc Loc,
+ SmallVectorImpl<const ArgumentInit *> &Values, ArrayRef<SMLoc> ValuesLocs,
const Record *ArgsRec) {
+ assert(Values.size() == ValuesLocs.size() &&
+ "expected as many values as locations");
+
ArrayRef<const Init *> TArgs = ArgsRec->getTemplateArgs();
bool HasError = false;
- for (const ArgumentInit *&Value : Values) {
+ for (auto [Value, Loc] : llvm::zip_equal(Values, ValuesLocs)) {
const Init *ArgName = nullptr;
if (Value->isPositional())
ArgName = TArgs[Value->getIndex()];
diff --git a/llvm/lib/TableGen/TGParser.h b/llvm/lib/TableGen/TGParser.h
index cac1ba827f1138..4509893eefc2c8 100644
--- a/llvm/lib/TableGen/TGParser.h
+++ b/llvm/lib/TableGen/TGParser.h
@@ -296,6 +296,7 @@ class TGParser {
void ParseValueList(SmallVectorImpl<const Init *> &Result, Record *CurRec,
const RecTy *ItemType = nullptr);
bool ParseTemplateArgValueList(SmallVectorImpl<const ArgumentInit *> &Result,
+ SmallVectorImpl<SMLoc> &ArgLocs,
Record *CurRec, const Record *ArgsRec);
void ParseDagArgList(
SmallVectorImpl<std::pair<const Init *, const StringInit *>> &Result,
@@ -321,7 +322,8 @@ class TGParser {
bool ApplyLetStack(Record *CurRec);
bool ApplyLetStack(RecordsEntry &Entry);
bool CheckTemplateArgValues(SmallVectorImpl<const ArgumentInit *> &Values,
- SMLoc Loc, const Record *ArgsRec);
+ ArrayRef<SMLoc> ValuesLocs,
+ const Record *ArgsRec);
};
} // end namespace llvm
diff --git a/llvm/test/TableGen/template-args.td b/llvm/test/TableGen/template-args.td
index 40623dcb701e0d..1644b0a12dc3e1 100644
--- a/llvm/test/TableGen/template-args.td
+++ b/llvm/test/TableGen/template-args.td
@@ -181,7 +181,7 @@ def error10 {
class Foo<int i, int j>;
def error11 : Foo<"", "">;
-// ERROR11: Value specified for template argument 'Foo:i' is of type string; expected type int: ""
-// ERROR11: Value specified for template argument 'Foo:j' is of type string; expected type int: ""
+// ERROR11: [[#@LINE-1]]:19: error: Value specified for template argument 'Foo:i' is of type string; expected type int: ""
+// ERROR11: [[#@LINE-2]]:23: error: Value specified for template argument 'Foo:j' is of type string; expected type int: ""
#endif
More information about the llvm-commits
mailing list