Skip to content

Commit 2f877c2

Browse files
authored
[MLIR] Check that the prop-dict dictionnary does not have extra unknown entries (#138668)
At the moment we would just ignore them, which can be surprising and is error prone (a typo for a unit attribute flag for example).
1 parent 5be080e commit 2f877c2

File tree

2 files changed

+26
-3
lines changed

2 files changed

+26
-3
lines changed

mlir/test/IR/invalid-custom-print-parse.mlir

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,10 @@ test.custom_dimension_list_attr dimension_list = [2x3]
1919

2020
// expected-error @below {{expected attribute value}}
2121
test.optional_custom_attr foo
22+
23+
// -----
24+
25+
// expected-error @below {{unknown key '"foo"' when parsing properties dictionary}}
26+
test.op_with_enum_prop_attr_form <{value = 0 : i32, foo}>
27+
28+

mlir/tools/mlir-tblgen/OpFormatGen.cpp

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,6 +1300,11 @@ if (!dict) {
13001300
emitError() << "expected DictionaryAttr to set properties";
13011301
return ::mlir::failure();
13021302
}
1303+
// keep track of used keys in the input dictionary to be able to error out
1304+
// if there are some unknown ones.
1305+
DenseSet<StringAttr> usedKeys;
1306+
MLIRContext *ctx = dict.getContext();
1307+
(void)ctx;
13031308
)decl";
13041309

13051310
// {0}: fromAttribute call
@@ -1310,7 +1315,9 @@ auto setFromAttr = [] (auto &propStorage, ::mlir::Attribute propAttr,
13101315
::llvm::function_ref<::mlir::InFlightDiagnostic()> emitError) -> ::mlir::LogicalResult {{
13111316
{0};
13121317
};
1313-
auto attr = dict.get("{1}");
1318+
auto {1}AttrName = StringAttr::get(ctx, "{1}");
1319+
usedKeys.insert({1}AttrName);
1320+
auto attr = dict.get({1}AttrName);
13141321
if (!attr && {2}) {{
13151322
emitError() << "expected key entry for {1} in DictionaryAttr to set "
13161323
"Properties.";
@@ -1356,7 +1363,9 @@ if (attr && ::mlir::failed(setFromAttr(prop.{1}, attr, emitError)))
13561363
bool isRequired = !attr.isOptional() && !attr.hasDefaultValue();
13571364
body << formatv(R"decl(
13581365
auto &propStorage = prop.{0};
1359-
auto attr = dict.get("{0}");
1366+
auto {0}AttrName = StringAttr::get(ctx, "{0}");
1367+
auto attr = dict.get({0}AttrName);
1368+
usedKeys.insert(StringAttr::get(ctx, "{1}"));
13601369
if (attr || /*isRequired=*/{1}) {{
13611370
if (!attr) {{
13621371
emitError() << "expected key entry for {0} in DictionaryAttr to set "
@@ -1374,7 +1383,14 @@ if (attr || /*isRequired=*/{1}) {{
13741383
)decl",
13751384
namedAttr.name, isRequired);
13761385
}
1377-
body << "return ::mlir::success();\n";
1386+
body << R"decl(
1387+
for (NamedAttribute attr : dict) {
1388+
if (!usedKeys.contains(attr.getName()))
1389+
return emitError() << "unknown key '" << attr.getName() <<
1390+
"' when parsing properties dictionary";
1391+
}
1392+
return ::mlir::success();
1393+
)decl";
13781394
}
13791395

13801396
void OperationFormat::genParser(Operator &op, OpClass &opClass) {

0 commit comments

Comments
 (0)