Skip to content

Conversation

@zwuis
Copy link
Contributor

@zwuis zwuis commented Jul 1, 2025

There is no need to distinguish between null optional and null function_ref in this case.

@llvmbot
Copy link
Member

llvmbot commented Jul 1, 2025

@llvm/pr-subscribers-mlir

Author: Yanzuo Liu (zwuis)

Changes

There is no need to distinguish between null optional and null function_ref in this case.


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

1 Files Affected:

  • (modified) mlir/lib/Dialect/EmitC/IR/EmitC.cpp (+6-8)
diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index 27298e892e599..ba1d010d29062 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -174,10 +174,9 @@ static LogicalResult verifyInitializationAttribute(Operation *op,
 /// In the format string, all `{}` are replaced by Placeholders, except if the
 /// `{` is escaped by `{{` - then it doesn't start a placeholder.
 template <class ArgType>
-FailureOr<SmallVector<ReplacementItem>>
-parseFormatString(StringRef toParse, ArgType fmtArgs,
-                  std::optional<llvm::function_ref<mlir::InFlightDiagnostic()>>
-                      emitError = {}) {
+FailureOr<SmallVector<ReplacementItem>> parseFormatString(
+    StringRef toParse, ArgType fmtArgs,
+    llvm::function_ref<mlir::InFlightDiagnostic()> emitError = {}) {
   SmallVector<ReplacementItem> items;
 
   // If there are not operands, the format string is not interpreted.
@@ -200,8 +199,7 @@ parseFormatString(StringRef toParse, ArgType fmtArgs,
       continue;
     }
     if (toParse.size() < 2) {
-      return (*emitError)()
-             << "expected '}' after unescaped '{' at end of string";
+      return emitError() << "expected '}' after unescaped '{' at end of string";
     }
     // toParse contains at least two characters and starts with `{`.
     char nextChar = toParse[1];
@@ -217,8 +215,8 @@ parseFormatString(StringRef toParse, ArgType fmtArgs,
       continue;
     }
 
-    if (emitError.has_value()) {
-      return (*emitError)() << "expected '}' after unescaped '{'";
+    if (emitError) {
+      return emitError() << "expected '}' after unescaped '{'";
     }
     return failure();
   }

@llvmbot
Copy link
Member

llvmbot commented Jul 1, 2025

@llvm/pr-subscribers-mlir-emitc

Author: Yanzuo Liu (zwuis)

Changes

There is no need to distinguish between null optional and null function_ref in this case.


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

1 Files Affected:

  • (modified) mlir/lib/Dialect/EmitC/IR/EmitC.cpp (+6-8)
diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index 27298e892e599..ba1d010d29062 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -174,10 +174,9 @@ static LogicalResult verifyInitializationAttribute(Operation *op,
 /// In the format string, all `{}` are replaced by Placeholders, except if the
 /// `{` is escaped by `{{` - then it doesn't start a placeholder.
 template <class ArgType>
-FailureOr<SmallVector<ReplacementItem>>
-parseFormatString(StringRef toParse, ArgType fmtArgs,
-                  std::optional<llvm::function_ref<mlir::InFlightDiagnostic()>>
-                      emitError = {}) {
+FailureOr<SmallVector<ReplacementItem>> parseFormatString(
+    StringRef toParse, ArgType fmtArgs,
+    llvm::function_ref<mlir::InFlightDiagnostic()> emitError = {}) {
   SmallVector<ReplacementItem> items;
 
   // If there are not operands, the format string is not interpreted.
@@ -200,8 +199,7 @@ parseFormatString(StringRef toParse, ArgType fmtArgs,
       continue;
     }
     if (toParse.size() < 2) {
-      return (*emitError)()
-             << "expected '}' after unescaped '{' at end of string";
+      return emitError() << "expected '}' after unescaped '{' at end of string";
     }
     // toParse contains at least two characters and starts with `{`.
     char nextChar = toParse[1];
@@ -217,8 +215,8 @@ parseFormatString(StringRef toParse, ArgType fmtArgs,
       continue;
     }
 
-    if (emitError.has_value()) {
-      return (*emitError)() << "expected '}' after unescaped '{'";
+    if (emitError) {
+      return emitError() << "expected '}' after unescaped '{'";
     }
     return failure();
   }

@marbre marbre requested a review from jpienaar July 1, 2025 09:12
Copy link
Member

@MacDue MacDue left a comment

Choose a reason for hiding this comment

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

LGTM, function_ref is nullable.

@zwuis zwuis merged commit 9cd2413 into llvm:main Jul 25, 2025
10 checks passed
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
…al<llvm::function_ref<>>` (llvm#146478)

There is no need to distinguish between null `optional` and null
`function_ref` in this case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants