diff --git a/mlir/include/mlir/IR/OpImplementation.h b/mlir/include/mlir/IR/OpImplementation.h --- a/mlir/include/mlir/IR/OpImplementation.h +++ b/mlir/include/mlir/IR/OpImplementation.h @@ -524,13 +524,18 @@ return success(); } - /// Parse a named dictionary into 'result' if it is present. - virtual ParseResult parseOptionalAttrDict(NamedAttrList &result) = 0; + /// Parse a named dictionary into 'result' if it is present. Fails parsing if + /// any of the attributes in the dictionary is in `disallowed`. + virtual ParseResult + parseOptionalAttrDict(NamedAttrList &result, + ArrayRef disallowed = {}) = 0; /// Parse a named dictionary into 'result' if the `attributes` keyword is - /// present. + /// present. Fails parsing if any of the attributes in the dictionary is in + /// `disallowed`. virtual ParseResult - parseOptionalAttrDictWithKeyword(NamedAttrList &result) = 0; + parseOptionalAttrDictWithKeyword(NamedAttrList &result, + ArrayRef disallowed = {}) = 0; /// Parse an affine map instance into 'map'. virtual ParseResult parseAffineMap(AffineMap &map) = 0; diff --git a/mlir/lib/IR/FunctionImplementation.cpp b/mlir/lib/IR/FunctionImplementation.cpp --- a/mlir/lib/IR/FunctionImplementation.cpp +++ b/mlir/lib/IR/FunctionImplementation.cpp @@ -196,7 +196,10 @@ << (errorMessage.empty() ? "" : ": ") << errorMessage; // If function attributes are present, parse them. - if (parser.parseOptionalAttrDictWithKeyword(result.attributes)) + if (parser.parseOptionalAttrDictWithKeyword( + result.attributes, + {SymbolTable::getSymbolAttrName(), + SymbolTable::getVisibilityAttrName(), getTypeAttrName()})) return failure(); // Add the attributes to the function arguments. diff --git a/mlir/lib/Parser/AttributeParser.cpp b/mlir/lib/Parser/AttributeParser.cpp --- a/mlir/lib/Parser/AttributeParser.cpp +++ b/mlir/lib/Parser/AttributeParser.cpp @@ -243,11 +243,14 @@ /// | `{` attribute-entry (`,` attribute-entry)* `}` /// attribute-entry ::= (bare-id | string-literal) `=` attribute-value /// -ParseResult Parser::parseAttributeDict(NamedAttrList &attributes) { +ParseResult Parser::parseAttributeDict(NamedAttrList &attributes, + ArrayRef disallowed) { if (parseToken(Token::l_brace, "expected '{' in attribute dictionary")) return failure(); llvm::SmallDenseSet seenKeys; + const llvm::SmallDenseSet disallowedKeys(disallowed.begin(), + disallowed.end()); auto parseElt = [&]() -> ParseResult { // The name of an attribute can either be a bare identifier, or a string. Optional nameId; @@ -258,9 +261,13 @@ nameId = builder.getIdentifier(getTokenSpelling()); else return emitError("expected attribute name"); + + if (disallowedKeys.contains(nameId->strref())) + return emitError("attribute '") + << *nameId << "' is not allowed in attribute dictionary"; if (!seenKeys.insert(*nameId).second) return emitError("duplicate key '") - << *nameId << "' in dictionary attribute"; + << *nameId << "' in attribute dictionary"; consumeToken(); // Lazy load a dialect in the context if there is a possible namespace. diff --git a/mlir/lib/Parser/Parser.h b/mlir/lib/Parser/Parser.h --- a/mlir/lib/Parser/Parser.h +++ b/mlir/lib/Parser/Parser.h @@ -207,8 +207,10 @@ return failure(); } - /// Parse an attribute dictionary. - ParseResult parseAttributeDict(NamedAttrList &attributes); + /// Parse an attribute dictionary. Fails parsing if any of the attributes in + /// the dictionary is in `disallowed`. + ParseResult parseAttributeDict(NamedAttrList &attributes, + ArrayRef disallowed = {}); /// Parse an extended attribute. Attribute parseExtendedAttr(Type type); diff --git a/mlir/lib/Parser/Parser.cpp b/mlir/lib/Parser/Parser.cpp --- a/mlir/lib/Parser/Parser.cpp +++ b/mlir/lib/Parser/Parser.cpp @@ -1169,19 +1169,24 @@ return parseOptionalAttributeAndAddToList(result, type, attrName, attrs); } - /// Parse a named dictionary into 'result' if it is present. - ParseResult parseOptionalAttrDict(NamedAttrList &result) override { + /// Parse a named dictionary into 'result' if it is present. Fails parsing if + /// any of the attributes in the dictionary is in `disallowed`. + ParseResult parseOptionalAttrDict(NamedAttrList &result, + ArrayRef disallowed) override { if (parser.getToken().isNot(Token::l_brace)) return success(); - return parser.parseAttributeDict(result); + return parser.parseAttributeDict(result, disallowed); } /// Parse a named dictionary into 'result' if the `attributes` keyword is - /// present. - ParseResult parseOptionalAttrDictWithKeyword(NamedAttrList &result) override { + /// present. Fails parsing if any of the attributes in the dictionary is in + /// `disallowed`. + ParseResult + parseOptionalAttrDictWithKeyword(NamedAttrList &result, + ArrayRef disallowed) override { if (failed(parseOptionalKeyword("attributes"))) return success(); - return parser.parseAttributeDict(result); + return parser.parseAttributeDict(result, disallowed); } /// Parse an affine map instance into 'map'. diff --git a/mlir/test/Dialect/Tosa/inlining.mlir b/mlir/test/Dialect/Tosa/inlining.mlir --- a/mlir/test/Dialect/Tosa/inlining.mlir +++ b/mlir/test/Dialect/Tosa/inlining.mlir @@ -19,11 +19,11 @@ }) : (tensor, tensor, tensor) -> tensor return %0 : tensor } -func @add(%arg0: tensor, %arg1: tensor) -> tensor attributes {sym_visibility = "private"} { +func private @add(%arg0: tensor, %arg1: tensor) -> tensor { %0 = "tosa.add"(%arg0, %arg1) : (tensor, tensor) -> tensor return %0 : tensor } -func @sub(%arg0: tensor, %arg1: tensor) -> tensor attributes {sym_visibility = "private"} { +func private @sub(%arg0: tensor, %arg1: tensor) -> tensor { %0 = "tosa.sub"(%arg0, %arg1) : (tensor, tensor) -> tensor return %0 : tensor } @@ -45,12 +45,12 @@ }) : (tensor, tensor, tensor, tensor<10xi32>) -> (tensor, tensor, tensor, tensor<10xi32>) return %1#3 : tensor<10xi32> } -func @while_body_50(%arg0: tensor, %arg1: tensor, %arg2: tensor, %arg3: tensor<10xi32>) -> (tensor, tensor, tensor, tensor<10xi32>) attributes {sym_visibility = "private"} { +func private @while_body_50(%arg0: tensor, %arg1: tensor, %arg2: tensor, %arg3: tensor<10xi32>) -> (tensor, tensor, tensor, tensor<10xi32>) { %1 = "tosa.add"(%arg0, %arg1) : (tensor, tensor) -> tensor %2 = "tosa.add"(%arg3, %1) : (tensor<10xi32>, tensor) -> tensor<10xi32> return %1, %arg1, %arg2, %2: tensor, tensor, tensor, tensor<10xi32> } -func @while_cond_40(%arg0: tensor, %arg1: tensor, %arg2: tensor, %arg3: tensor<10xi32>) -> tensor attributes {sym_visibility = "private"} { +func private @while_cond_40(%arg0: tensor, %arg1: tensor, %arg2: tensor, %arg3: tensor<10xi32>) -> tensor { %0 = "tosa.greater_equal"(%arg0, %arg1) : (tensor, tensor) -> tensor %1 = "tosa.logical_not"(%0) : (tensor) -> tensor return %1 : tensor diff --git a/mlir/test/IR/core-ops.mlir b/mlir/test/IR/core-ops.mlir --- a/mlir/test/IR/core-ops.mlir +++ b/mlir/test/IR/core-ops.mlir @@ -942,6 +942,3 @@ return } - -// CHECK-LABEL: func private @legacy_visibility_syntax -func @legacy_visibility_syntax() attributes { sym_visibility = "private" } diff --git a/mlir/test/IR/invalid-func-op.mlir b/mlir/test/IR/invalid-func-op.mlir --- a/mlir/test/IR/invalid-func-op.mlir +++ b/mlir/test/IR/invalid-func-op.mlir @@ -78,3 +78,9 @@ // expected-error@+1 {{symbol declaration cannot have public visibility}} func @invalid_public_declaration() + +// ----- + +// expected-error@+1 {{attribute 'sym_visibility' is not allowed in attribute dictionary}} +func @legacy_visibility_syntax() attributes { sym_visibility = "private" } + diff --git a/mlir/test/IR/invalid.mlir b/mlir/test/IR/invalid.mlir --- a/mlir/test/IR/invalid.mlir +++ b/mlir/test/IR/invalid.mlir @@ -1561,7 +1561,7 @@ // ----- func @duplicate_dictionary_attr_key() { - // expected-error @+1 {{duplicate key 'a' in dictionary attribute}} + // expected-error @+1 {{duplicate key 'a' in attribute dictionary}} "foo.op"() {a, a} : () -> () }