Skip to content

Commit

Permalink
[mlir] Sanitize identifiers with leading symbol. (llvm#94795)
Browse files Browse the repository at this point in the history
Presently, if name starts with a symbol it's converted to hex which may
cause the result to be invalid by starting with a digit.

Address this and add a small test.

Co-authored-by: Will Dietz <w@wdtz.org>
  • Loading branch information
dtzSiFive and dtzWill authored Jun 11, 2024
1 parent 48aebd4 commit 46e41c8
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 3 deletions.
10 changes: 7 additions & 3 deletions mlir/lib/IR/AsmPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -999,9 +999,13 @@ static StringRef sanitizeIdentifier(StringRef name, SmallString<16> &buffer,
bool allowTrailingDigit = true) {
assert(!name.empty() && "Shouldn't have an empty name here");

auto validChar = [&](char ch) {
return llvm::isAlnum(ch) || allowedPunctChars.contains(ch);
};

auto copyNameToBuffer = [&] {
for (char ch : name) {
if (llvm::isAlnum(ch) || allowedPunctChars.contains(ch))
if (validChar(ch))
buffer.push_back(ch);
else if (ch == ' ')
buffer.push_back('_');
Expand All @@ -1013,7 +1017,7 @@ static StringRef sanitizeIdentifier(StringRef name, SmallString<16> &buffer,
// Check to see if this name is valid. If it starts with a digit, then it
// could conflict with the autogenerated numeric ID's, so add an underscore
// prefix to avoid problems.
if (isdigit(name[0])) {
if (isdigit(name[0]) || (!validChar(name[0]) && name[0] != ' ')) {
buffer.push_back('_');
copyNameToBuffer();
return buffer;
Expand All @@ -1029,7 +1033,7 @@ static StringRef sanitizeIdentifier(StringRef name, SmallString<16> &buffer,

// Check to see that the name consists of only valid identifier characters.
for (char ch : name) {
if (!llvm::isAlnum(ch) && !allowedPunctChars.contains(ch)) {
if (!validChar(ch)) {
copyNameToBuffer();
return buffer;
}
Expand Down
3 changes: 3 additions & 0 deletions mlir/test/IR/print-attr-type-aliases.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
// CHECK-DAG: #_0_test_alias = "alias_test:prefixed_digit"
"test.op"() {alias_test = "alias_test:prefixed_digit"} : () -> ()

// CHECK-DAG: #_25test = "alias_test:prefixed_symbol"
"test.op"() {alias_test = "alias_test:prefixed_symbol"} : () -> ()

// CHECK-DAG: #test_alias_conflict0_ = "alias_test:sanitize_conflict_a"
// CHECK-DAG: #test_alias_conflict0_1 = "alias_test:sanitize_conflict_b"
"test.op"() {alias_test = ["alias_test:sanitize_conflict_a", "alias_test:sanitize_conflict_b"]} : () -> ()
Expand Down
1 change: 1 addition & 0 deletions mlir/test/lib/Dialect/Test/TestDialectInterfaces.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ struct TestOpAsmInterface : public OpAsmDialectInterface {
.Case("alias_test:dot_in_name", StringRef("test.alias"))
.Case("alias_test:trailing_digit", StringRef("test_alias0"))
.Case("alias_test:prefixed_digit", StringRef("0_test_alias"))
.Case("alias_test:prefixed_symbol", StringRef("%test"))
.Case("alias_test:sanitize_conflict_a",
StringRef("test_alias_conflict0"))
.Case("alias_test:sanitize_conflict_b",
Expand Down

0 comments on commit 46e41c8

Please sign in to comment.