[Mlir-commits] [mlir] [mlir] Adjust split marker flag logic to fix MSVC build. (PR #85614)

[Adrian Kuegel]

https://github.com/akuegel created https://github.com/llvm/llvm-project/pull/85614

We should not try to capture the flag variable inside the callback passed when creating the flag variable. As a fix, we use a separate boolean variable to track whether the command line flag was specified.

>From 92bb4aea56d9fee02c6c98f248d767d77cef25cc Mon Sep 17 00:00:00 2001
From: Adrian Kuegel <akuegel at google.com>
Date: Mon, 18 Mar 2024 08:45:46 +0000
Subject: [PATCH] [mlir] Adjust split marker flag logic to fix MSVC build.

We should not try to capture the flag variable inside the callback
passed when creating the flag variable. As a fix, we use a separate
boolean variable to track whether the command line flag was specified.
---
 .../include/mlir/Tools/mlir-opt/MlirOptMain.h | 14 +++++++++++--
 mlir/lib/Tools/mlir-opt/MlirOptMain.cpp       | 11 ++++------
 .../mlir-translate/MlirTranslateMain.cpp      | 21 ++++++++++---------
 3 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h b/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
index 8adc80908de116..8cc191b11015ca 100644
--- a/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
+++ b/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
@@ -142,10 +142,17 @@ class MlirOptMainConfig {
   MlirOptMainConfig &
   splitInputFile(std::string splitMarker = kDefaultSplitMarker) {
     splitInputFileFlag = std::move(splitMarker);
+    splitInputFileSpecified = true;
     return *this;
   }
-  bool shouldSplitInputFile() const { return splitInputFileFlag.empty(); }
-  StringRef inputSplitMarker() const { return splitInputFileFlag; }
+  bool shouldSplitInputFile() const {
+    return splitInputFileSpecified || !splitInputFileFlag.empty();
+  }
+  std::string inputSplitMarker() const {
+    return splitInputFileSpecified && splitInputFileFlag.empty()
+               ? kDefaultSplitMarker
+               : splitInputFileFlag;
+  }
 
   /// Set whether to merge the output chunks into one file using the given
   /// marker.
@@ -227,6 +234,9 @@ class MlirOptMainConfig {
   /// Show the registered dialects before trying to load the input file.
   bool showDialectsFlag = false;
 
+  /// Whether the split-input-file flag was specified.
+  bool splitInputFileSpecified = false;
+
   /// Split the input file based on the given marker into chunks and process
   /// each chunk independently. Input is not split if empty.
   std::string splitInputFileFlag = "";
diff --git a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
index 44c5e9826f3b79..f1fb5bfb667601 100644
--- a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
+++ b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
@@ -128,16 +128,13 @@ struct MlirOptMainConfigCLOptions : public MlirOptMainConfig {
         cl::desc("Print the list of registered dialects and exit"),
         cl::location(showDialectsFlag), cl::init(false));
 
-    static cl::opt<std::string, /*ExternalStorage=*/true> splitInputFile{
+    static cl::opt<std::string, /*ExternalStorage=*/true> splitInputFile(
         "split-input-file", llvm::cl::ValueOptional,
-        cl::callback([&](const std::string &str) {
-          // Implicit value: use default marker if flag was used without value.
-          if (str.empty())
-            splitInputFile.setValue(kDefaultSplitMarker);
-        }),
+        cl::callback(
+            [&](const std::string &str) { splitInputFileSpecified = true; }),
         cl::desc("Split the input file into chunks using the given or "
                  "default marker and process each chunk independently"),
-        cl::location(splitInputFileFlag), cl::init("")};
+        cl::location(splitInputFileFlag), cl::init(""));
 
     static cl::opt<std::string, /*ExternalStorage=*/true> outputSplitMarker(
         "output-split-marker",
diff --git a/mlir/lib/Tools/mlir-translate/MlirTranslateMain.cpp b/mlir/lib/Tools/mlir-translate/MlirTranslateMain.cpp
index bd9928950ecc76..81c9f56e0e1f36 100644
--- a/mlir/lib/Tools/mlir-translate/MlirTranslateMain.cpp
+++ b/mlir/lib/Tools/mlir-translate/MlirTranslateMain.cpp
@@ -62,16 +62,14 @@ LogicalResult mlir::mlirTranslateMain(int argc, char **argv,
       llvm::cl::desc("Allow operation with no registered dialects (discouraged: testing only!)"),
       llvm::cl::init(false));
 
-  static llvm::cl::opt<std::string> inputSplitMarker{
+  static bool inputSplitMarkerSpecified = false;
+  static llvm::cl::opt<std::string> inputSplitMarker(
       "split-input-file", llvm::cl::ValueOptional,
-      llvm::cl::callback([&](const std::string &str) {
-        // Implicit value: use default marker if flag was used without value.
-        if (str.empty())
-          inputSplitMarker.setValue(kDefaultSplitMarker);
-      }),
+      llvm::cl::callback(
+          [&](const std::string &str) { inputSplitMarkerSpecified = true; }),
       llvm::cl::desc("Split the input file into chunks using the given or "
                      "default marker and process each chunk independently"),
-      llvm::cl::init("")};
+      llvm::cl::init(""));
 
   static llvm::cl::opt<bool> verifyDiagnostics(
       "verify-diagnostics",
@@ -185,9 +183,12 @@ LogicalResult mlir::mlirTranslateMain(int argc, char **argv,
     return result;
   };
 
-  if (failed(splitAndProcessBuffer(std::move(input), processBuffer,
-                                   output->os(), inputSplitMarker,
-                                   outputSplitMarker)))
+  if (failed(splitAndProcessBuffer(
+          std::move(input), processBuffer, output->os(),
+          inputSplitMarkerSpecified && inputSplitMarker.getValue().empty()
+              ? kDefaultSplitMarker
+              : inputSplitMarker.getValue(),
+          outputSplitMarker)))
     return failure();
 
   output->keep();