[PATCH] D72977: [mlir][ods] Fix StringRef initialization in builders

[Lei Zhang via Phabricator via llvm-commits]

antiagainst updated this revision to Diff 239311.
antiagainst added a comment.

Address comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72977/new/

https://reviews.llvm.org/D72977

Files:
  mlir/include/mlir/TableGen/Attribute.h
  mlir/test/mlir-tblgen/op-attribute.td
  mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp


Index: mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
===================================================================
--- mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -12,6 +12,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "mlir/Support/STLExtras.h"
+#include "mlir/Support/StringExtras.h"
 #include "mlir/TableGen/Format.h"
 #include "mlir/TableGen/GenInfo.h"
 #include "mlir/TableGen/OpClass.h"
@@ -91,6 +92,17 @@
 // Utility structs and functions
 //===----------------------------------------------------------------------===//
 
+// Replaces all occurrences of `match` in `str` with `substitute`.
+static std::string replaceAllSubstrs(std::string str, const std::string &match,
+                                     const std::string &substitute) {
+  std::string::size_type scanLoc = 0, matchLoc = std::string::npos;
+  while ((matchLoc = str.find(match, scanLoc)) != std::string::npos) {
+    str = str.replace(matchLoc, match.size(), substitute);
+    scanLoc = matchLoc + substitute.size();
+  }
+  return str;
+}
+
 // Returns whether the record has a value of the given name that can be returned
 // via getValueAsString.
 static inline bool hasStringAttribute(const Record &record,
@@ -968,8 +980,18 @@
         // instance.
         FmtContext fctx;
         fctx.withBuilder("(*odsBuilder)");
-        std::string value =
-            tgfmt(attr.getConstBuilderTemplate(), &fctx, namedAttr.name);
+
+        std::string builderTemplate = attr.getConstBuilderTemplate();
+
+        // For StringAttr, its constant builder call will wrap the input in
+        // quotes, which is correct for normal string literals, but incorrect
+        // here given we use function arguments. So we need to strip the
+        // wrapping quotes.
+        if (StringRef(builderTemplate).contains("\"$0\"")) {
+          builderTemplate = replaceAllSubstrs(builderTemplate, "\"$0\"", "$0");
+        }
+
+        std::string value = tgfmt(builderTemplate, &fctx, namedAttr.name);
         body << formatv("  {0}.addAttribute(\"{1}\", {2});\n", builderOpState,
                         namedAttr.name, value);
       } else {
Index: mlir/test/mlir-tblgen/op-attribute.td
===================================================================
--- mlir/test/mlir-tblgen/op-attribute.td
+++ mlir/test/mlir-tblgen/op-attribute.td
@@ -186,6 +186,11 @@
 // DECL-LABEL: DOp declarations
 // DECL: static void build({{.*}}, APInt i32_attr, APFloat f64_attr, StringRef str_attr, bool bool_attr, ::SomeI32Enum enum_attr, APInt dv_i32_attr, APFloat dv_f64_attr, StringRef dv_str_attr = "abc", bool dv_bool_attr = true, ::SomeI32Enum dv_enum_attr = ::SomeI32Enum::case5)
 
+// DEF-LABEL: DOp definitions
+// DEF: odsState.addAttribute("str_attr", (*odsBuilder).getStringAttr(str_attr));
+// DEF: odsState.addAttribute("dv_str_attr", (*odsBuilder).getStringAttr(dv_str_attr));
+
+
 // Test that only default valued attributes at the end of the arguments
 // list get default values in the builder signature
 // ---
Index: mlir/include/mlir/TableGen/Attribute.h
===================================================================
--- mlir/include/mlir/TableGen/Attribute.h
+++ mlir/include/mlir/TableGen/Attribute.h
@@ -64,8 +64,8 @@
 
   // Returns the template that can be used to produce an instance of the
   // attribute.
-  // Syntax: {0} should be replaced with a builder, {1} should be replaced with
-  // the constant value.
+  // Syntax: `$builder` should be replaced with a builder, `$0` should be
+  // replaced with the constant value.
   StringRef getConstBuilderTemplate() const;
 
   // Returns the base-level attribute that this attribute constraint is


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72977.239311.patch
Type: text/x-patch
Size: 3753 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200121/300a1e07/attachment.bin>