[mlir][StorageUniquer] Restore old signature for default implementaion of verifyInvariants.

[bchetioui]

PR #102326 changed the prototype of the default implementation of verify to include emitErrorFn.

This breaks automatic derivation in consumer attributes, such as https://github.com/tensorflow/runtime/blob/60277ba976739502e45ad26585e071568fa44af1/include/tfrt/core_runtime/opdefs/attributes.h#L53.

This PR simply restores the signature to what it was prior to PR #102326.

[bchetioui]

@matthias-springer btw, thank you for leaving notes on your PR for integrators---much appreciated!

[llvmbot]

@llvm/pr-subscribers-mlir

Author: Benjamin Chetioui (bchetioui)

Changes

PR #102326 changed the prototype of the default implementation of verify to include emitErrorFn.

This breaks automatic derivation in consumer attributes, such as https://github.com/tensorflow/runtime/blob/60277ba976739502e45ad26585e071568fa44af1/include/tfrt/core_runtime/opdefs/attributes.h#L53.

This PR simply restores the signature to what it was prior to PR #102326.

---

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

1 Files Affected:

• (modified) mlir/include/mlir/IR/StorageUniquerSupport.h (+1-2)

diff --git a/mlir/include/mlir/IR/StorageUniquerSupport.h b/mlir/include/mlir/IR/StorageUniquerSupport.h
index d6ccbbd8579947..c493fd9c5042c7 100644
--- a/mlir/include/mlir/IR/StorageUniquerSupport.h
+++ b/mlir/include/mlir/IR/StorageUniquerSupport.h
@@ -227,8 +227,7 @@ class StorageUserBase : public BaseT, public Traits<ConcreteT>... {
   /// Default implementation that just returns success.
   template <typename... Args>
   static LogicalResult
-  verifyInvariants(function_ref<InFlightDiagnostic()> emitErrorFn,
-                   Args... args) {
+  verifyInvariants(Args... args) {
     return success();
   }
 

[llvmbot]

@llvm/pr-subscribers-mlir-core

Author: Benjamin Chetioui (bchetioui)

Changes

PR #102326 changed the prototype of the default implementation of verify to include emitErrorFn.

This breaks automatic derivation in consumer attributes, such as https://github.com/tensorflow/runtime/blob/60277ba976739502e45ad26585e071568fa44af1/include/tfrt/core_runtime/opdefs/attributes.h#L53.

This PR simply restores the signature to what it was prior to PR #102326.

---

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

1 Files Affected:

• (modified) mlir/include/mlir/IR/StorageUniquerSupport.h (+1-2)

diff --git a/mlir/include/mlir/IR/StorageUniquerSupport.h b/mlir/include/mlir/IR/StorageUniquerSupport.h
index d6ccbbd8579947..c493fd9c5042c7 100644
--- a/mlir/include/mlir/IR/StorageUniquerSupport.h
+++ b/mlir/include/mlir/IR/StorageUniquerSupport.h
@@ -227,8 +227,7 @@ class StorageUserBase : public BaseT, public Traits<ConcreteT>... {
   /// Default implementation that just returns success.
   template <typename... Args>
   static LogicalResult
-  verifyInvariants(function_ref<InFlightDiagnostic()> emitErrorFn,
-                   Args... args) {
+  verifyInvariants(Args... args) {
     return success();
   }
 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[matthias-springer]

Isn't this function always called with a function_ref<InFlightDiagnostic()> emitErrorFn? (There is no particular reason why I changed this. Just wanted to make the params a bit more explicit.)

[matthias-springer]

LGTM if the CI passes

[bchetioui]

Nope, it seems like this is also called with a llvm::unique_function<InFlightDiagnostic ()>, see the following error I encountered:

llvm-project/mlir/include/mlir/IR/StorageUniquerSupport.h:230:3: note: candidate function template not viable: no known conversion from 'llvm::unique_function<InFlightDiagnostic ()>' to 'function_ref<InFlightDiagnostic ()>' (aka 'function_ref<mlir::InFlightDiagnostic ()>') for 1st argument

[bchetioui]

Thanks for the review, @matthias-springer!