[Mlir-commits] [mlir] c50617d - Simplify diagnostic error management for MLIR properties API (NFC) (#67409)

llvmlistbot at llvm.org llvmlistbot at llvm.org
Tue Sep 26 11:44:41 PDT 2023


Author: Mehdi Amini
Date: 2023-09-26T11:44:37-07:00
New Revision: c50617dae3d3e1d324d31c3071965076a1856b4d

URL: https://github.com/llvm/llvm-project/commit/c50617dae3d3e1d324d31c3071965076a1856b4d
DIFF: https://github.com/llvm/llvm-project/commit/c50617dae3d3e1d324d31c3071965076a1856b4d.diff

LOG: Simplify diagnostic error management for MLIR properties API (NFC) (#67409)

This is a follow-up to 8c2bff1ab929 which lazy-initialized the
diagnostic and removed the need to dynamically abandon() an
InFlightDiagnostic. This further simplifies the code to not needed to
return a reference to an InFlightDiagnostic and instead eagerly emit
errors.

Also use `emitError` as name instead of `getDiag` which seems more
explicit and in-line with the common usage.

Added: 
    

Modified: 
    mlir/include/mlir/IR/ExtensibleDialect.h
    mlir/include/mlir/IR/ODSSupport.h
    mlir/include/mlir/IR/OpDefinition.h
    mlir/include/mlir/IR/Operation.h
    mlir/include/mlir/IR/OperationSupport.h
    mlir/lib/AsmParser/Parser.cpp
    mlir/lib/CAPI/IR/IR.cpp
    mlir/lib/IR/MLIRContext.cpp
    mlir/lib/IR/ODSSupport.cpp
    mlir/lib/IR/Operation.cpp
    mlir/lib/IR/OperationSupport.cpp
    mlir/lib/TableGen/CodeGenHelpers.cpp
    mlir/test/lib/Dialect/Test/TestDialect.cpp
    mlir/test/lib/Dialect/Test/TestDialect.h
    mlir/test/mlir-tblgen/constraint-unique.td
    mlir/test/mlir-tblgen/interfaces-as-constraints.td
    mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
    mlir/unittests/IR/OpPropertiesTest.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/IR/ExtensibleDialect.h b/mlir/include/mlir/IR/ExtensibleDialect.h
index baff6011ff84651..5ae1e9ab537af9d 100644
--- a/mlir/include/mlir/IR/ExtensibleDialect.h
+++ b/mlir/include/mlir/IR/ExtensibleDialect.h
@@ -476,7 +476,7 @@ class DynamicOpDefinition : public OperationName::Impl {
   void populateInherentAttrs(Operation *op, NamedAttrList &attrs) final {}
   LogicalResult
   verifyInherentAttrs(OperationName opName, NamedAttrList &attributes,
-                      function_ref<InFlightDiagnostic()> getDiag) final {
+                      function_ref<InFlightDiagnostic()> emitError) final {
     return success();
   }
   int getOpPropertyByteSize() final { return 0; }
@@ -489,8 +489,8 @@ class DynamicOpDefinition : public OperationName::Impl {
   LogicalResult
   setPropertiesFromAttr(OperationName opName, OpaqueProperties properties,
                         Attribute attr,
-                        function_ref<InFlightDiagnostic &()> getDiag) final {
-    getDiag() << "extensible Dialects don't support properties";
+                        function_ref<InFlightDiagnostic()> emitError) final {
+    emitError() << "extensible Dialects don't support properties";
     return failure();
   }
   Attribute getPropertiesAsAttr(Operation *op) final { return {}; }

diff  --git a/mlir/include/mlir/IR/ODSSupport.h b/mlir/include/mlir/IR/ODSSupport.h
index 748bf52a55c557a..70e3f986431e266 100644
--- a/mlir/include/mlir/IR/ODSSupport.h
+++ b/mlir/include/mlir/IR/ODSSupport.h
@@ -28,7 +28,7 @@ namespace mlir {
 /// error message is also emitted.
 LogicalResult
 convertFromAttribute(int64_t &storage, Attribute attr,
-                     function_ref<InFlightDiagnostic &()> getDiag);
+                     function_ref<InFlightDiagnostic()> emitError);
 
 /// Convert the provided int64_t to an IntegerAttr attribute.
 Attribute convertToAttribute(MLIRContext *ctx, int64_t storage);
@@ -39,7 +39,7 @@ Attribute convertToAttribute(MLIRContext *ctx, int64_t storage);
 /// the optional diagnostic is provided an error message is also emitted.
 LogicalResult
 convertFromAttribute(MutableArrayRef<int64_t> storage, Attribute attr,
-                     function_ref<InFlightDiagnostic &()> getDiag);
+                     function_ref<InFlightDiagnostic()> emitError);
 
 /// Convert a DenseI32ArrayAttr to the provided storage. It is expected that the
 /// storage has the same size as the array. An error is returned if the
@@ -47,7 +47,7 @@ convertFromAttribute(MutableArrayRef<int64_t> storage, Attribute attr,
 /// the optional diagnostic is provided an error message is also emitted.
 LogicalResult
 convertFromAttribute(MutableArrayRef<int32_t> storage, Attribute attr,
-                     function_ref<InFlightDiagnostic &()> getDiag);
+                     function_ref<InFlightDiagnostic()> emitError);
 
 /// Convert the provided ArrayRef<int64_t> to a DenseI64ArrayAttr attribute.
 Attribute convertToAttribute(MLIRContext *ctx, ArrayRef<int64_t> storage);

diff  --git a/mlir/include/mlir/IR/OpDefinition.h b/mlir/include/mlir/IR/OpDefinition.h
index 82d0e93a8ee2fa9..8ab37c1d51d6b6c 100644
--- a/mlir/include/mlir/IR/OpDefinition.h
+++ b/mlir/include/mlir/IR/OpDefinition.h
@@ -1741,8 +1741,8 @@ class Op : public OpState, public Traits<ConcreteType>... {
   template <typename PropertiesTy>
   static LogicalResult
   setPropertiesFromAttr(PropertiesTy &prop, Attribute attr,
-                        function_ref<InFlightDiagnostic &()> getDiag) {
-    return setPropertiesFromAttribute(prop, attr, getDiag);
+                        function_ref<InFlightDiagnostic()> emitError) {
+    return setPropertiesFromAttribute(prop, attr, emitError);
   }
   /// Convert the provided properties to an attribute. This default
   /// implementation forwards to a free function `getPropertiesAsAttribute` that

diff  --git a/mlir/include/mlir/IR/Operation.h b/mlir/include/mlir/IR/Operation.h
index 35e9d31a6323173..3ba0daeca0258a0 100644
--- a/mlir/include/mlir/IR/Operation.h
+++ b/mlir/include/mlir/IR/Operation.h
@@ -901,7 +901,7 @@ class alignas(8) Operation final
   /// generic format. An optional diagnostic can be passed in for richer errors.
   LogicalResult
   setPropertiesFromAttribute(Attribute attr,
-                             function_ref<InFlightDiagnostic &()> getDiag);
+                             function_ref<InFlightDiagnostic()> emitError);
 
   /// Copy properties from an existing other properties object. The two objects
   /// must be the same type.

diff  --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index 9e9f89b95e6fd50..bb3d1643e1687ab 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -129,7 +129,7 @@ class OperationName {
     virtual void populateInherentAttrs(Operation *op, NamedAttrList &attrs) = 0;
     virtual LogicalResult
     verifyInherentAttrs(OperationName opName, NamedAttrList &attributes,
-                        function_ref<InFlightDiagnostic()> getDiag) = 0;
+                        function_ref<InFlightDiagnostic()> emitError) = 0;
     virtual int getOpPropertyByteSize() = 0;
     virtual void initProperties(OperationName opName, OpaqueProperties storage,
                                 OpaqueProperties init) = 0;
@@ -138,7 +138,7 @@ class OperationName {
                                            OpaqueProperties properties) = 0;
     virtual LogicalResult
     setPropertiesFromAttr(OperationName, OpaqueProperties, Attribute,
-                          function_ref<InFlightDiagnostic &()> getDiag) = 0;
+                          function_ref<InFlightDiagnostic()> emitError) = 0;
     virtual Attribute getPropertiesAsAttr(Operation *) = 0;
     virtual void copyProperties(OpaqueProperties, OpaqueProperties) = 0;
     virtual bool compareProperties(OpaqueProperties, OpaqueProperties) = 0;
@@ -210,7 +210,7 @@ class OperationName {
     void populateInherentAttrs(Operation *op, NamedAttrList &attrs) final;
     LogicalResult
     verifyInherentAttrs(OperationName opName, NamedAttrList &attributes,
-                        function_ref<InFlightDiagnostic()> getDiag) final;
+                        function_ref<InFlightDiagnostic()> emitError) final;
     int getOpPropertyByteSize() final;
     void initProperties(OperationName opName, OpaqueProperties storage,
                         OpaqueProperties init) final;
@@ -219,7 +219,7 @@ class OperationName {
                                    OpaqueProperties properties) final;
     LogicalResult
     setPropertiesFromAttr(OperationName, OpaqueProperties, Attribute,
-                          function_ref<InFlightDiagnostic &()> getDiag) final;
+                          function_ref<InFlightDiagnostic()> emitError) final;
     Attribute getPropertiesAsAttr(Operation *) final;
     void copyProperties(OpaqueProperties, OpaqueProperties) final;
     bool compareProperties(OpaqueProperties, OpaqueProperties) final;
@@ -408,8 +408,8 @@ class OperationName {
   /// attributes when parsed from the older generic syntax pre-Properties.
   LogicalResult
   verifyInherentAttrs(NamedAttrList &attributes,
-                      function_ref<InFlightDiagnostic()> getDiag) const {
-    return getImpl()->verifyInherentAttrs(*this, attributes, getDiag);
+                      function_ref<InFlightDiagnostic()> emitError) const {
+    return getImpl()->verifyInherentAttrs(*this, attributes, emitError);
   }
   /// This hooks return the number of bytes to allocate for the op properties.
   int getOpPropertyByteSize() const {
@@ -439,8 +439,9 @@ class OperationName {
   /// Define the op properties from the provided Attribute.
   LogicalResult setOpPropertiesFromAttribute(
       OperationName opName, OpaqueProperties properties, Attribute attr,
-      function_ref<InFlightDiagnostic &()> getDiag) const {
-    return getImpl()->setPropertiesFromAttr(opName, properties, attr, getDiag);
+      function_ref<InFlightDiagnostic()> emitError) const {
+    return getImpl()->setPropertiesFromAttr(opName, properties, attr,
+                                            emitError);
   }
 
   void copyOpProperties(OpaqueProperties lhs, OpaqueProperties rhs) const {
@@ -596,9 +597,9 @@ class RegisteredOperationName : public OperationName {
     }
     LogicalResult
     verifyInherentAttrs(OperationName opName, NamedAttrList &attributes,
-                        function_ref<InFlightDiagnostic()> getDiag) final {
+                        function_ref<InFlightDiagnostic()> emitError) final {
       if constexpr (hasProperties)
-        return ConcreteOp::verifyInherentAttrs(opName, attributes, getDiag);
+        return ConcreteOp::verifyInherentAttrs(opName, attributes, emitError);
       return success();
     }
     // Detect if the concrete operation defined properties.
@@ -636,12 +637,12 @@ class RegisteredOperationName : public OperationName {
     LogicalResult
     setPropertiesFromAttr(OperationName opName, OpaqueProperties properties,
                           Attribute attr,
-                          function_ref<InFlightDiagnostic &()> getDiag) final {
+                          function_ref<InFlightDiagnostic()> emitError) final {
       if constexpr (hasProperties) {
         auto p = properties.as<Properties *>();
-        return ConcreteOp::setPropertiesFromAttr(*p, attr, getDiag);
+        return ConcreteOp::setPropertiesFromAttr(*p, attr, emitError);
       }
-      getDiag() << "this operation does not support properties";
+      emitError() << "this operation does not support properties";
       return failure();
     }
     Attribute getPropertiesAsAttr(Operation *op) final {
@@ -1010,7 +1011,7 @@ struct OperationState {
   // optionally emit diagnostics on error through the provided diagnostic.
   LogicalResult
   setProperties(Operation *op,
-                function_ref<InFlightDiagnostic &()> getDiag) const;
+                function_ref<InFlightDiagnostic()> emitError) const;
 
   void addOperands(ValueRange newOperands);
 

diff  --git a/mlir/lib/AsmParser/Parser.cpp b/mlir/lib/AsmParser/Parser.cpp
index 84f44dba806df01..8a9b8593110efb2 100644
--- a/mlir/lib/AsmParser/Parser.cpp
+++ b/mlir/lib/AsmParser/Parser.cpp
@@ -1446,16 +1446,11 @@ Operation *OperationParser::parseGenericOperation() {
   // Try setting the properties for the operation, using a diagnostic to print
   // errors.
   if (properties) {
-    std::unique_ptr<InFlightDiagnostic> diagnostic;
-    auto getDiag = [&]() -> InFlightDiagnostic & {
-      if (!diagnostic) {
-        diagnostic = std::make_unique<InFlightDiagnostic>(
-            mlir::emitError(srcLocation, "invalid properties ")
-            << properties << " for op " << name << ": ");
-      }
-      return *diagnostic;
+    auto emitError = [&]() {
+      return mlir::emitError(srcLocation, "invalid properties ")
+             << properties << " for op " << name << ": ";
     };
-    if (failed(op->setPropertiesFromAttribute(properties, getDiag)))
+    if (failed(op->setPropertiesFromAttribute(properties, emitError)))
       return nullptr;
   }
 
@@ -2009,17 +2004,12 @@ OperationParser::parseCustomOperation(ArrayRef<ResultRecord> resultIDs) {
 
   // Try setting the properties for the operation.
   if (properties) {
-    std::unique_ptr<InFlightDiagnostic> diagnostic;
-    auto getDiag = [&]() -> InFlightDiagnostic & {
-      if (!diagnostic) {
-        diagnostic = std::make_unique<InFlightDiagnostic>(
-            mlir::emitError(srcLocation, "invalid properties ")
-            << properties << " for op " << op->getName().getStringRef()
-            << ": ");
-      }
-      return *diagnostic;
+    auto emitError = [&]() {
+      return mlir::emitError(srcLocation, "invalid properties ")
+             << properties << " for op " << op->getName().getStringRef()
+             << ": ";
     };
-    if (failed(op->setPropertiesFromAttribute(properties, getDiag)))
+    if (failed(op->setPropertiesFromAttribute(properties, emitError)))
       return nullptr;
   }
   return op;

diff  --git a/mlir/lib/CAPI/IR/IR.cpp b/mlir/lib/CAPI/IR/IR.cpp
index 04b386b8268e8d4..65b2b7466fb7c39 100644
--- a/mlir/lib/CAPI/IR/IR.cpp
+++ b/mlir/lib/CAPI/IR/IR.cpp
@@ -416,18 +416,13 @@ static LogicalResult inferOperationTypes(OperationState &state) {
     auto prop = std::make_unique<char[]>(info->getOpPropertyByteSize());
     properties = OpaqueProperties(prop.get());
     if (properties) {
-      std::unique_ptr<InFlightDiagnostic> diagnostic;
-      auto getDiag = [&]() -> InFlightDiagnostic & {
-        if (!diagnostic) {
-          diagnostic = std::make_unique<InFlightDiagnostic>(
-              emitError(state.location)
-              << " failed properties conversion while building "
-              << state.name.getStringRef() << " with `" << attributes << "`: ");
-        }
-        return *diagnostic;
+      auto emitError = [&]() {
+        return mlir::emitError(state.location)
+               << " failed properties conversion while building "
+               << state.name.getStringRef() << " with `" << attributes << "`: ";
       };
       if (failed(info->setOpPropertiesFromAttribute(state.name, properties,
-                                                    attributes, getDiag)))
+                                                    attributes, emitError)))
         return failure();
     }
     if (succeeded(inferInterface->inferReturnTypes(

diff  --git a/mlir/lib/IR/MLIRContext.cpp b/mlir/lib/IR/MLIRContext.cpp
index ed1029959639587..e5a397c9d65c1ab 100644
--- a/mlir/lib/IR/MLIRContext.cpp
+++ b/mlir/lib/IR/MLIRContext.cpp
@@ -834,7 +834,7 @@ void OperationName::UnregisteredOpModel::populateInherentAttrs(
     Operation *op, NamedAttrList &attrs) {}
 LogicalResult OperationName::UnregisteredOpModel::verifyInherentAttrs(
     OperationName opName, NamedAttrList &attributes,
-    function_ref<InFlightDiagnostic()> getDiag) {
+    function_ref<InFlightDiagnostic()> emitError) {
   return success();
 }
 int OperationName::UnregisteredOpModel::getOpPropertyByteSize() {
@@ -852,7 +852,7 @@ void OperationName::UnregisteredOpModel::populateDefaultProperties(
     OperationName opName, OpaqueProperties properties) {}
 LogicalResult OperationName::UnregisteredOpModel::setPropertiesFromAttr(
     OperationName opName, OpaqueProperties properties, Attribute attr,
-    function_ref<InFlightDiagnostic &()> getDiag) {
+    function_ref<InFlightDiagnostic()> emitError) {
   *properties.as<Attribute *>() = attr;
   return success();
 }

diff  --git a/mlir/lib/IR/ODSSupport.cpp b/mlir/lib/IR/ODSSupport.cpp
index 0601430c4461651..6e968d62e61c7f4 100644
--- a/mlir/lib/IR/ODSSupport.cpp
+++ b/mlir/lib/IR/ODSSupport.cpp
@@ -20,10 +20,10 @@ using namespace mlir;
 
 LogicalResult
 mlir::convertFromAttribute(int64_t &storage, Attribute attr,
-                           function_ref<InFlightDiagnostic &()> getDiag) {
+                           function_ref<InFlightDiagnostic()> emitError) {
   auto valueAttr = dyn_cast<IntegerAttr>(attr);
   if (!valueAttr) {
-    getDiag() << "expected IntegerAttr for key `value`";
+    emitError() << "expected IntegerAttr for key `value`";
     return failure();
   }
   storage = valueAttr.getValue().getSExtValue();
@@ -36,16 +36,16 @@ Attribute mlir::convertToAttribute(MLIRContext *ctx, int64_t storage) {
 template <typename DenseArrayTy, typename T>
 LogicalResult
 convertDenseArrayFromAttr(MutableArrayRef<T> storage, Attribute attr,
-                          function_ref<InFlightDiagnostic &()> getDiag,
+                          function_ref<InFlightDiagnostic()> emitError,
                           StringRef denseArrayTyStr) {
   auto valueAttr = dyn_cast<DenseArrayTy>(attr);
   if (!valueAttr) {
-    getDiag() << "expected " << denseArrayTyStr << " for key `value`";
+    emitError() << "expected " << denseArrayTyStr << " for key `value`";
     return failure();
   }
   if (valueAttr.size() != static_cast<int64_t>(storage.size())) {
-    getDiag() << "size mismatch in attribute conversion: " << valueAttr.size()
-              << " vs " << storage.size();
+    emitError() << "size mismatch in attribute conversion: " << valueAttr.size()
+                << " vs " << storage.size();
     return failure();
   }
   llvm::copy(valueAttr.asArrayRef(), storage.begin());
@@ -53,14 +53,14 @@ convertDenseArrayFromAttr(MutableArrayRef<T> storage, Attribute attr,
 }
 LogicalResult
 mlir::convertFromAttribute(MutableArrayRef<int64_t> storage, Attribute attr,
-                           function_ref<InFlightDiagnostic &()> getDiag) {
-  return convertDenseArrayFromAttr<DenseI64ArrayAttr>(storage, attr, getDiag,
+                           function_ref<InFlightDiagnostic()> emitError) {
+  return convertDenseArrayFromAttr<DenseI64ArrayAttr>(storage, attr, emitError,
                                                       "DenseI64ArrayAttr");
 }
 LogicalResult
 mlir::convertFromAttribute(MutableArrayRef<int32_t> storage, Attribute attr,
-                           function_ref<InFlightDiagnostic &()> getDiag) {
-  return convertDenseArrayFromAttr<DenseI32ArrayAttr>(storage, attr, getDiag,
+                           function_ref<InFlightDiagnostic()> emitError) {
+  return convertDenseArrayFromAttr<DenseI32ArrayAttr>(storage, attr, emitError,
                                                       "DenseI32ArrayAttr");
 }
 

diff  --git a/mlir/lib/IR/Operation.cpp b/mlir/lib/IR/Operation.cpp
index aa577aa089c6860..7507e0cd0c99b31 100644
--- a/mlir/lib/IR/Operation.cpp
+++ b/mlir/lib/IR/Operation.cpp
@@ -352,14 +352,14 @@ Attribute Operation::getPropertiesAsAttribute() {
   return info->getOpPropertiesAsAttribute(this);
 }
 LogicalResult Operation::setPropertiesFromAttribute(
-    Attribute attr, function_ref<InFlightDiagnostic &()> getDiag) {
+    Attribute attr, function_ref<InFlightDiagnostic()> emitError) {
   std::optional<RegisteredOperationName> info = getRegisteredInfo();
   if (LLVM_UNLIKELY(!info)) {
     *getPropertiesStorage().as<Attribute *>() = attr;
     return success();
   }
   return info->setOpPropertiesFromAttribute(
-      this->getName(), this->getPropertiesStorage(), attr, getDiag);
+      this->getName(), this->getPropertiesStorage(), attr, emitError);
 }
 
 void Operation::copyProperties(OpaqueProperties rhs) {

diff  --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp
index b0c50f3d6e29855..5aac72fcb062c39 100644
--- a/mlir/lib/IR/OperationSupport.cpp
+++ b/mlir/lib/IR/OperationSupport.cpp
@@ -199,10 +199,10 @@ OperationState::~OperationState() {
 }
 
 LogicalResult OperationState::setProperties(
-    Operation *op, function_ref<InFlightDiagnostic &()> getDiag) const {
+    Operation *op, function_ref<InFlightDiagnostic()> emitError) const {
   if (LLVM_UNLIKELY(propertiesAttr)) {
     assert(!properties);
-    return op->setPropertiesFromAttribute(propertiesAttr, getDiag);
+    return op->setPropertiesFromAttribute(propertiesAttr, emitError);
   }
   if (properties)
     propertiesSetter(op->getPropertiesStorage(), properties);

diff  --git a/mlir/lib/TableGen/CodeGenHelpers.cpp b/mlir/lib/TableGen/CodeGenHelpers.cpp
index afd02b1a64ac942..371da8272029a00 100644
--- a/mlir/lib/TableGen/CodeGenHelpers.cpp
+++ b/mlir/lib/TableGen/CodeGenHelpers.cpp
@@ -133,9 +133,9 @@ static ::mlir::LogicalResult {0}(
 /// functions are stripped anyways.
 static const char *const attrConstraintCode = R"(
 static ::mlir::LogicalResult {0}(
-    ::mlir::Attribute attr, ::llvm::StringRef attrName, llvm::function_ref<::mlir::InFlightDiagnostic()> getDiag) {{
+    ::mlir::Attribute attr, ::llvm::StringRef attrName, llvm::function_ref<::mlir::InFlightDiagnostic()> emitError) {{
   if (attr && !({1}))
-    return getDiag() << "attribute '" << attrName
+    return emitError() << "attribute '" << attrName
         << "' failed to satisfy constraint: {2}";
   return ::mlir::success();
 }

diff  --git a/mlir/test/lib/Dialect/Test/TestDialect.cpp b/mlir/test/lib/Dialect/Test/TestDialect.cpp
index 00c251936655d71..3f1bb667a0aecb5 100644
--- a/mlir/test/lib/Dialect/Test/TestDialect.cpp
+++ b/mlir/test/lib/Dialect/Test/TestDialect.cpp
@@ -55,10 +55,10 @@ Attribute MyPropStruct::asAttribute(MLIRContext *ctx) const {
 }
 LogicalResult
 MyPropStruct::setFromAttr(MyPropStruct &prop, Attribute attr,
-                          function_ref<InFlightDiagnostic &()> getDiag) {
+                          function_ref<InFlightDiagnostic()> emitError) {
   StringAttr strAttr = dyn_cast<StringAttr>(attr);
   if (!strAttr) {
-    getDiag() << "Expect StringAttr but got " << attr;
+    emitError() << "Expect StringAttr but got " << attr;
     return failure();
   }
   prop.content = strAttr.getValue();
@@ -108,7 +108,7 @@ static void writeToMlirBytecode(::mlir::DialectBytecodeWriter &writer,
 
 static LogicalResult
 setPropertiesFromAttribute(PropertiesWithCustomPrint &prop, Attribute attr,
-                           function_ref<InFlightDiagnostic &()> getDiag);
+                           function_ref<InFlightDiagnostic()> emitError);
 static DictionaryAttr
 getPropertiesAsAttribute(MLIRContext *ctx,
                          const PropertiesWithCustomPrint &prop);
@@ -119,7 +119,7 @@ static ParseResult customParseProperties(OpAsmParser &parser,
                                          PropertiesWithCustomPrint &prop);
 static LogicalResult
 setPropertiesFromAttribute(VersionedProperties &prop, Attribute attr,
-                           function_ref<InFlightDiagnostic &()> getDiag);
+                           function_ref<InFlightDiagnostic()> emitError);
 static DictionaryAttr getPropertiesAsAttribute(MLIRContext *ctx,
                                                const VersionedProperties &prop);
 static llvm::hash_code computeHash(const VersionedProperties &prop);
@@ -1138,20 +1138,20 @@ OpFoldResult ManualCppOpWithFold::fold(ArrayRef<Attribute> attributes) {
 
 static LogicalResult
 setPropertiesFromAttribute(PropertiesWithCustomPrint &prop, Attribute attr,
-                           function_ref<InFlightDiagnostic &()> getDiag) {
+                           function_ref<InFlightDiagnostic()> emitError) {
   DictionaryAttr dict = dyn_cast<DictionaryAttr>(attr);
   if (!dict) {
-    getDiag() << "expected DictionaryAttr to set TestProperties";
+    emitError() << "expected DictionaryAttr to set TestProperties";
     return failure();
   }
   auto label = dict.getAs<mlir::StringAttr>("label");
   if (!label) {
-    getDiag() << "expected StringAttr for key `label`";
+    emitError() << "expected StringAttr for key `label`";
     return failure();
   }
   auto valueAttr = dict.getAs<IntegerAttr>("value");
   if (!valueAttr) {
-    getDiag() << "expected IntegerAttr for key `value`";
+    emitError() << "expected IntegerAttr for key `value`";
     return failure();
   }
 
@@ -1187,20 +1187,20 @@ static ParseResult customParseProperties(OpAsmParser &parser,
 }
 static LogicalResult
 setPropertiesFromAttribute(VersionedProperties &prop, Attribute attr,
-                           function_ref<InFlightDiagnostic &()> getDiag) {
+                           function_ref<InFlightDiagnostic()> emitError) {
   DictionaryAttr dict = dyn_cast<DictionaryAttr>(attr);
   if (!dict) {
-    getDiag() << "expected DictionaryAttr to set VersionedProperties";
+    emitError() << "expected DictionaryAttr to set VersionedProperties";
     return failure();
   }
   auto value1Attr = dict.getAs<IntegerAttr>("value1");
   if (!value1Attr) {
-    getDiag() << "expected IntegerAttr for key `value1`";
+    emitError() << "expected IntegerAttr for key `value1`";
     return failure();
   }
   auto value2Attr = dict.getAs<IntegerAttr>("value2");
   if (!value2Attr) {
-    getDiag() << "expected IntegerAttr for key `value2`";
+    emitError() << "expected IntegerAttr for key `value2`";
     return failure();
   }
 

diff  --git a/mlir/test/lib/Dialect/Test/TestDialect.h b/mlir/test/lib/Dialect/Test/TestDialect.h
index 0ae0d47615776dd..4ba28c47ed1c33e 100644
--- a/mlir/test/lib/Dialect/Test/TestDialect.h
+++ b/mlir/test/lib/Dialect/Test/TestDialect.h
@@ -95,7 +95,7 @@ class MyPropStruct {
   mlir::Attribute asAttribute(mlir::MLIRContext *ctx) const;
   static mlir::LogicalResult
   setFromAttr(MyPropStruct &prop, mlir::Attribute attr,
-              llvm::function_ref<mlir::InFlightDiagnostic &()> getDiag);
+              llvm::function_ref<mlir::InFlightDiagnostic()> emitError);
   llvm::hash_code hash() const;
   bool operator==(const MyPropStruct &rhs) const {
     return content == rhs.content;

diff  --git a/mlir/test/mlir-tblgen/constraint-unique.td b/mlir/test/mlir-tblgen/constraint-unique.td
index 752bd201fae6de6..cd41efa63be4377 100644
--- a/mlir/test/mlir-tblgen/constraint-unique.td
+++ b/mlir/test/mlir-tblgen/constraint-unique.td
@@ -71,7 +71,7 @@ def OpC : NS_Op<"op_c"> {
 /// Test that an attribute contraint was generated.
 // CHECK:    static ::mlir::LogicalResult [[$A_ATTR_CONSTRAINT:__mlir_ods_local_attr_constraint.*]](
 // CHECK:      if (attr && !((attrPred(attr, *op))))
-// CHECK-NEXT:   return getDiag() << "attribute '" << attrName
+// CHECK-NEXT:   return emitError() << "attribute '" << attrName
 // CHECK-NEXT:       << "' failed to satisfy constraint: an attribute";
 
 /// Test that duplicate attribute constraint was not generated.
@@ -81,7 +81,7 @@ def OpC : NS_Op<"op_c"> {
 // CHECK:    static ::mlir::LogicalResult [[$O_ATTR_CONSTRAINT:__mlir_ods_local_attr_constraint.*]](
 // CHECK:    static ::mlir::LogicalResult [[$O_ATTR_CONSTRAINT:__mlir_ods_local_attr_constraint.*]](
 // CHECK:      if (attr && !((attrPred(attr, *op))))
-// CHECK-NEXT:   return getDiag() << "attribute '" << attrName
+// CHECK-NEXT:   return emitError() << "attribute '" << attrName
 // CHECK-NEXT:       << "' failed to satisfy constraint: another attribute";
 
 /// Test that a successor contraint was generated.

diff  --git a/mlir/test/mlir-tblgen/interfaces-as-constraints.td b/mlir/test/mlir-tblgen/interfaces-as-constraints.td
index efce1c3927044cd..109bf078caed188 100644
--- a/mlir/test/mlir-tblgen/interfaces-as-constraints.td
+++ b/mlir/test/mlir-tblgen/interfaces-as-constraints.td
@@ -35,12 +35,12 @@ def OpUsingAllOfThose : Op<Test_Dialect, "OpUsingAllOfThose"> {
 
 // CHECK: static ::mlir::LogicalResult {{__mlir_ods_local_attr_constraint.*}}(
 // CHECK:   if (attr && !((::llvm::isa<TopLevelAttrInterface>(attr))))
-// CHECK-NEXT:     return getDiag() << "attribute '" << attrName
+// CHECK-NEXT:     return emitError() << "attribute '" << attrName
 // CHECK-NEXT:        << "' failed to satisfy constraint: TopLevelAttrInterface instance";
 
 // CHECK: static ::mlir::LogicalResult {{__mlir_ods_local_attr_constraint.*}}(
 // CHECK:   if (attr && !((::llvm::isa<test::AttrInterfaceInNamespace>(attr))))
-// CHECK-NEXT:    return getDiag() << "attribute '" << attrName
+// CHECK-NEXT:    return emitError() << "attribute '" << attrName
 // CHECK-NEXT:        << "' failed to satisfy constraint: AttrInterfaceInNamespace instance";
 
 // CHECK: TopLevelAttrInterface OpUsingAllOfThose::getAttr1()

diff  --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index ff73e600819cb91..985d82d0f0eb9d7 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -52,7 +52,7 @@ static const char *const builderOpState = "odsState";
 static const char *const propertyStorage = "propStorage";
 static const char *const propertyValue = "propValue";
 static const char *const propertyAttr = "propAttr";
-static const char *const propertyDiag = "getDiag";
+static const char *const propertyDiag = "emitError";
 
 /// The names of the implicit attributes that contain variadic operand and
 /// result segment sizes.
@@ -1213,8 +1213,8 @@ void OpEmitter::genPropertiesSupport() {
               MethodParameter("Properties &", "prop"),
               MethodParameter("::mlir::Attribute", "attr"),
               MethodParameter(
-                  "::llvm::function_ref<::mlir::InFlightDiagnostic &()>",
-                  "getDiag"))
+                  "::llvm::function_ref<::mlir::InFlightDiagnostic()>",
+                  "emitError"))
           ->body();
   auto &getPropMethod =
       opClass
@@ -1256,7 +1256,7 @@ void OpEmitter::genPropertiesSupport() {
               MethodParameter("::mlir::NamedAttrList &", "attrs"),
               MethodParameter(
                   "llvm::function_ref<::mlir::InFlightDiagnostic()>",
-                  "getDiag"))
+                  "emitError"))
           ->body();
 
   opClass.declare<UsingDeclaration>("Properties", "FoldAdaptor::Properties");
@@ -1266,7 +1266,7 @@ void OpEmitter::genPropertiesSupport() {
   setPropMethod << R"decl(
   ::mlir::DictionaryAttr dict = ::llvm::dyn_cast<::mlir::DictionaryAttr>(attr);
   if (!dict) {
-    getDiag() << "expected DictionaryAttr to set properties";
+    emitError() << "expected DictionaryAttr to set properties";
     return ::mlir::failure();
   }
     )decl";
@@ -1274,16 +1274,16 @@ void OpEmitter::genPropertiesSupport() {
   const char *propFromAttrFmt = R"decl(;
     {{
       auto setFromAttr = [] (auto &propStorage, ::mlir::Attribute propAttr,
-               ::llvm::function_ref<::mlir::InFlightDiagnostic &()> getDiag) {{
+               ::llvm::function_ref<::mlir::InFlightDiagnostic()> emitError) {{
         {0};
       };
       {2};
       if (!attr) {{
-        getDiag() << "expected key entry for {1} in DictionaryAttr to set "
+        emitError() << "expected key entry for {1} in DictionaryAttr to set "
                    "Properties.";
         return ::mlir::failure();
       }
-      if (::mlir::failed(setFromAttr(prop.{1}, attr, getDiag)))
+      if (::mlir::failed(setFromAttr(prop.{1}, attr, emitError)))
         return ::mlir::failure();
     }
 )decl";
@@ -1338,7 +1338,7 @@ void OpEmitter::genPropertiesSupport() {
     {2}
     if (attr || /*isRequired=*/{1}) {{
       if (!attr) {{
-        getDiag() << "expected key entry for {0} in DictionaryAttr to set "
+        emitError() << "expected key entry for {0} in DictionaryAttr to set "
                    "Properties.";
         return ::mlir::failure();
       }
@@ -1346,7 +1346,7 @@ void OpEmitter::genPropertiesSupport() {
       if (convertedAttr) {{
         propStorage = convertedAttr;
       } else {{
-        getDiag() << "Invalid attribute `{0}` in property conversion: " << attr;
+        emitError() << "Invalid attribute `{0}` in property conversion: " << attr;
         return ::mlir::failure();
       }
     }
@@ -1535,7 +1535,7 @@ void OpEmitter::genPropertiesSupport() {
           << formatv(R"(
     {{
       ::mlir::Attribute attr = attrs.get({0}AttrName(opName));
-      if (attr && ::mlir::failed({1}(attr, "{2}", getDiag)))
+      if (attr && ::mlir::failed({1}(attr, "{2}", emitError)))
         return ::mlir::failure();
     }
 )",

diff  --git a/mlir/unittests/IR/OpPropertiesTest.cpp b/mlir/unittests/IR/OpPropertiesTest.cpp
index e2d0599f1bd9548..5c3e150f6b94c28 100644
--- a/mlir/unittests/IR/OpPropertiesTest.cpp
+++ b/mlir/unittests/IR/OpPropertiesTest.cpp
@@ -38,33 +38,33 @@ bool operator==(const TestProperties &lhs, TestProperties &rhs) {
 /// parsing with the generic format.
 static LogicalResult
 setPropertiesFromAttribute(TestProperties &prop, Attribute attr,
-                           function_ref<InFlightDiagnostic &()> getDiag) {
+                           function_ref<InFlightDiagnostic()> emitError) {
   DictionaryAttr dict = dyn_cast<DictionaryAttr>(attr);
   if (!dict) {
-    getDiag() << "expected DictionaryAttr to set TestProperties";
+    emitError() << "expected DictionaryAttr to set TestProperties";
     return failure();
   }
   auto aAttr = dict.getAs<IntegerAttr>("a");
   if (!aAttr) {
-    getDiag() << "expected IntegerAttr for key `a`";
+    emitError() << "expected IntegerAttr for key `a`";
     return failure();
   }
   auto bAttr = dict.getAs<FloatAttr>("b");
   if (!bAttr ||
       &bAttr.getValue().getSemantics() != &llvm::APFloatBase::IEEEsingle()) {
-    getDiag() << "expected FloatAttr for key `b`";
+    emitError() << "expected FloatAttr for key `b`";
     return failure();
   }
 
   auto arrayAttr = dict.getAs<DenseI64ArrayAttr>("array");
   if (!arrayAttr) {
-    getDiag() << "expected DenseI64ArrayAttr for key `array`";
+    emitError() << "expected DenseI64ArrayAttr for key `array`";
     return failure();
   }
 
   auto label = dict.getAs<mlir::StringAttr>("label");
   if (!label) {
-    getDiag() << "expected StringAttr for key `label`";
+    emitError() << "expected StringAttr for key `label`";
     return failure();
   }
 
@@ -127,7 +127,7 @@ class OpWithProperties : public Op<OpWithProperties> {
                                     NamedAttrList &attrs) {}
   static LogicalResult
   verifyInherentAttrs(OperationName opName, NamedAttrList &attrs,
-                      function_ref<InFlightDiagnostic()> getDiag) {
+                      function_ref<InFlightDiagnostic()> emitError) {
     return success();
   }
 };
@@ -257,15 +257,10 @@ TEST(OpPropertiesTest, FailedProperties) {
   attrs.push_back(b.getNamedAttr("a", b.getStringAttr("foo")));
   state.propertiesAttr = attrs.getDictionary(&context);
   {
-    std::unique_ptr<InFlightDiagnostic> diagnostic;
-    auto getDiag = [&]() -> InFlightDiagnostic & {
-      if (!diagnostic) {
-        diagnostic = std::make_unique<InFlightDiagnostic>(
-            op->emitError("setting properties failed: "));
-      }
-      return *diagnostic;
+    auto emitError = [&]() {
+      return op->emitError("setting properties failed: ");
     };
-    auto result = state.setProperties(op, getDiag);
+    auto result = state.setProperties(op, emitError);
     EXPECT_TRUE(result.failed());
   }
   EXPECT_STREQ("setting properties failed: expected IntegerAttr for key `a`",


        


More information about the Mlir-commits mailing list