Skip to content

[MLIR][ODS] Use fully qualified ::mlir:: namespace in OpFormatGen property parsing (NFC) #152498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

amirBish
Copy link
Contributor

@amirBish amirBish commented Aug 7, 2025

Use the ::mlir:: namespace to ensure proper namespace resolution in generated code.

- Use the mlir namespace which ensures proper namespace resolution in generated code
and prevents potential compilation issues with unqualified type names.
@amirBish amirBish requested review from ftynse and joker-eph August 7, 2025 13:44
@amirBish amirBish self-assigned this Aug 7, 2025
@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels Aug 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 7, 2025

@llvm/pr-subscribers-mlir

Author: Amir Bishara (amirBish)

Changes
  • Use the mlir namespace which ensures proper namespace resolution in generated code and prevents potential compilation issues with unqualified type names.

Full diff: https://github.com/llvm/llvm-project/pull/152498.diff

1 Files Affected:

  • (modified) mlir/tools/mlir-tblgen/OpFormatGen.cpp (+5-5)
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index 11edf2523f1aa..54fc7155bea73 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -1302,8 +1302,8 @@ if (!dict) {
 }
 // keep track of used keys in the input dictionary to be able to error out
 // if there are some unknown ones.
-DenseSet<StringAttr> usedKeys;
-MLIRContext *ctx = dict.getContext();
+::mlir::DenseSet<::mlir::StringAttr> usedKeys;
+::mlir::MLIRContext *ctx = dict.getContext();
 (void)ctx;
 )decl";
 
@@ -1315,7 +1315,7 @@ auto setFromAttr = [] (auto &propStorage, ::mlir::Attribute propAttr,
          ::llvm::function_ref<::mlir::InFlightDiagnostic()> emitError) -> ::mlir::LogicalResult {{
   {0};
 };
-auto {1}AttrName = StringAttr::get(ctx, "{1}");
+auto {1}AttrName = ::mlir::StringAttr::get(ctx, "{1}");
 usedKeys.insert({1}AttrName);
 auto attr = dict.get({1}AttrName);
 if (!attr && {2}) {{
@@ -1363,7 +1363,7 @@ if (attr && ::mlir::failed(setFromAttr(prop.{1}, attr, emitError)))
     bool isRequired = !attr.isOptional() && !attr.hasDefaultValue();
     body << formatv(R"decl(
 auto &propStorage = prop.{0};
-auto {0}AttrName = StringAttr::get(ctx, "{0}");
+auto {0}AttrName = ::mlir::StringAttr::get(ctx, "{0}");
 auto attr = dict.get({0}AttrName);
 usedKeys.insert({0}AttrName);
 if (attr || /*isRequired=*/{1}) {{
@@ -1384,7 +1384,7 @@ if (attr || /*isRequired=*/{1}) {{
                     namedAttr.name, isRequired);
   }
   body << R"decl(
-for (NamedAttribute attr : dict) {
+for (::mlir::NamedAttribute attr : dict) {
   if (!usedKeys.contains(attr.getName()))
     return emitError() << "unknown key '" << attr.getName() <<
         "' when parsing properties dictionary";

@llvmbot
Copy link
Member

llvmbot commented Aug 7, 2025

@llvm/pr-subscribers-mlir-core

Author: Amir Bishara (amirBish)

Changes
  • Use the mlir namespace which ensures proper namespace resolution in generated code and prevents potential compilation issues with unqualified type names.

Full diff: https://github.com/llvm/llvm-project/pull/152498.diff

1 Files Affected:

  • (modified) mlir/tools/mlir-tblgen/OpFormatGen.cpp (+5-5)
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index 11edf2523f1aa..54fc7155bea73 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -1302,8 +1302,8 @@ if (!dict) {
 }
 // keep track of used keys in the input dictionary to be able to error out
 // if there are some unknown ones.
-DenseSet<StringAttr> usedKeys;
-MLIRContext *ctx = dict.getContext();
+::mlir::DenseSet<::mlir::StringAttr> usedKeys;
+::mlir::MLIRContext *ctx = dict.getContext();
 (void)ctx;
 )decl";
 
@@ -1315,7 +1315,7 @@ auto setFromAttr = [] (auto &propStorage, ::mlir::Attribute propAttr,
          ::llvm::function_ref<::mlir::InFlightDiagnostic()> emitError) -> ::mlir::LogicalResult {{
   {0};
 };
-auto {1}AttrName = StringAttr::get(ctx, "{1}");
+auto {1}AttrName = ::mlir::StringAttr::get(ctx, "{1}");
 usedKeys.insert({1}AttrName);
 auto attr = dict.get({1}AttrName);
 if (!attr && {2}) {{
@@ -1363,7 +1363,7 @@ if (attr && ::mlir::failed(setFromAttr(prop.{1}, attr, emitError)))
     bool isRequired = !attr.isOptional() && !attr.hasDefaultValue();
     body << formatv(R"decl(
 auto &propStorage = prop.{0};
-auto {0}AttrName = StringAttr::get(ctx, "{0}");
+auto {0}AttrName = ::mlir::StringAttr::get(ctx, "{0}");
 auto attr = dict.get({0}AttrName);
 usedKeys.insert({0}AttrName);
 if (attr || /*isRequired=*/{1}) {{
@@ -1384,7 +1384,7 @@ if (attr || /*isRequired=*/{1}) {{
                     namedAttr.name, isRequired);
   }
   body << R"decl(
-for (NamedAttribute attr : dict) {
+for (::mlir::NamedAttribute attr : dict) {
   if (!usedKeys.contains(attr.getName()))
     return emitError() << "unknown key '" << attr.getName() <<
         "' when parsing properties dictionary";

Copy link
Collaborator

@joker-eph joker-eph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not show up in any test? Can we test this?

@joker-eph joker-eph changed the title [mlir][tblgen] Use fully mlir namespace in OpFormatGen property parsing [MLIR][ODS] Use fully qualified ::mlir:: namespace in OpFormatGen property parsing Aug 7, 2025
@joker-eph joker-eph changed the title [MLIR][ODS] Use fully qualified ::mlir:: namespace in OpFormatGen property parsing [MLIR][ODS] Use fully qualified ::mlir:: namespace in OpFormatGen property parsing (NFC) Aug 7, 2025
@amirBish
Copy link
Contributor Author

amirBish commented Aug 8, 2025

@joker-eph Oh it seems that this change is already in merged a month ago by your commit (5f91b69), closing this PR :)

@amirBish amirBish closed this Aug 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlir:core MLIR Core Infrastructure mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants