[mlir] Fix printing of EmitC attrs/types with escape characters

[simon-camp]

Summary

Attributes and types were not escaped when printing.

[simon-camp]

Updating D109143: [mlir] Fix printing of EmitC attrs/types with escape characters

[jpienaar]

Could we have a test showing them all the way to C/C++? And is there only subset of characters supported to make the result still legal C/C++ but that's only checked in emitter?

[simon-camp]

I will add tests for the emitter now that it has landed, as well as a verifier for the opaque type to check whether it's a valid C identifier.

[jpienaar]

In D109143#2979856, @simon-camp wrote:

I will add tests for the emitter now that it has landed, as well as a verifier for the opaque type to check whether it's a valid C identifier.

SG, just to be clear, I'm waiting for those and then we can review them all together

[simon-camp]

Add tests for the CPP target.

[simon-camp]

Sorry for the delay, I got distracted by other work :)

I additonally changed the type tests to use the template_args attribute because calling a function with a type is not valid C code (i.e. f(int) is not a valid function call).

Regarding the opaque type verifier, I'd suggest to do it in a follow up.

[jpienaar]

Looks good thanks

mlir/test/Dialect/EmitC/attrs.mlir

7

Could you add a comment here explaining this test a bit more?

mlir/lib/Dialect/EmitC/IR/EmitC.cpp

 //===- EmitC.cpp - EmitC Dialect ------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "mlir/Dialect/EmitC/IR/EmitC.h"
 #include "mlir/IR/Builders.h"
 #include "mlir/IR/DialectImplementation.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/TypeSwitch.h"
 
 using namespace mlir;
 using namespace mlir::emitc;
 
 #include "mlir/Dialect/EmitC/IR/EmitCDialect.cpp.inc"
 
 //===----------------------------------------------------------------------===//

 }
 
 void EmitCDialect::printAttribute(Attribute attr, DialectAsmPrinter &os) const {
   if (failed(generatedAttributePrinter(attr, os)))
     llvm_unreachable("unexpected 'EmitC' attribute kind");
 }
 
 void emitc::OpaqueAttr::print(DialectAsmPrinter &printer) const {
-  printer << "opaque<\"" << getValue() << "\">";
+  printer << "opaque<\"";
+  llvm::printEscapedString(getValue(), printer.getStream());
+  printer << "\">";
 }
 
 //===----------------------------------------------------------------------===//
 // EmitC Types
 //===----------------------------------------------------------------------===//
 
 #define GET_TYPEDEF_CLASSES
 #include "mlir/Dialect/EmitC/IR/EmitCTypes.cpp.inc"

 }
 
 void EmitCDialect::printType(Type type, DialectAsmPrinter &os) const {
   if (failed(generatedTypePrinter(type, os)))
     llvm_unreachable("unexpected 'EmitC' type kind");
 }
 
 void emitc::OpaqueType::print(DialectAsmPrinter &printer) const {
-  printer << "opaque<\"" << getValue() << "\">";
+  printer << "opaque<\"";
+  llvm::printEscapedString(getValue(), printer.getStream());
+  printer << "\">";
 }

mlir/test/Dialect/EmitC/attrs.mlir

+// RUN: mlir-opt -verify-diagnostics %s | FileCheck %s
+// check parser
+// RUN: mlir-opt -verify-diagnostics %s | mlir-opt -verify-diagnostics | FileCheck %s
+
+// CHECK-LABEL: func @opaque_attrs() {
+func @opaque_attrs() {
+  // CHECK-NEXT: #emitc.opaque<"attr">
+  emitc.call "f"() {args = [#emitc.opaque<"attr">]} : () -> ()
+  // CHECK-NEXT: #emitc.opaque<"\22quoted_attr\22">
+  emitc.call "f"() {args = [#emitc.opaque<"\"quoted_attr\"">]} : () -> ()
+  return
+}

mlir/test/Dialect/EmitC/types.mlir

 // RUN: mlir-opt -verify-diagnostics %s | FileCheck %s
 // check parser
 // RUN: mlir-opt -verify-diagnostics %s | mlir-opt -verify-diagnostics | FileCheck %s
 
 // CHECK-LABEL: func @opaque_types() {
 func @opaque_types() {
   // CHECK-NEXT: !emitc.opaque<"int">
-  emitc.call "f"() {args = [!emitc<"opaque<\"int\">">]} : () -> ()
+  emitc.call "f"() {template_args = [!emitc<"opaque<\"int\">">]} : () -> ()
   // CHECK-NEXT: !emitc.opaque<"byte">
-  emitc.call "f"() {args = [!emitc<"opaque<\"byte\">">]} : () -> ()
+  emitc.call "f"() {template_args = [!emitc<"opaque<\"byte\">">]} : () -> ()
   // CHECK-NEXT: !emitc.opaque<"unsigned">
-  emitc.call "f"() {args = [!emitc<"opaque<\"unsigned\">">]} : () -> ()
+  emitc.call "f"() {template_args = [!emitc<"opaque<\"unsigned\">">]} : () -> ()
   // CHECK-NEXT: !emitc.opaque<"status_t">
-  emitc.call "f"() {args = [!emitc<"opaque<\"status_t\">">]} : () -> ()
+  emitc.call "f"() {template_args = [!emitc<"opaque<\"status_t\">">]} : () -> ()
   // CHECK-NEXT: !emitc.opaque<"std::vector<std::string>">
-  emitc.call "f"() {args = [!emitc.opaque<"std::vector<std::string>">]} : () -> ()
+  emitc.call "f"() {template_args = [!emitc.opaque<"std::vector<std::string>">]} : () -> ()
+
   return
 }

mlir/test/Target/Cpp/attrs.mlir

+// RUN: mlir-translate -mlir-to-cpp %s | FileCheck %s
+
+// CHECK-LABEL: void opaque_attrs() {
+func @opaque_attrs() {
+  // CHECK-NEXT: f(OPAQUE_ENUM_VALUE);
+  emitc.call "f"() {args = [#emitc.opaque<"OPAQUE_ENUM_VALUE">]} : () -> ()
+  // CHECK-NEXT: f("some string");
+  emitc.call "f"() {args = [#emitc.opaque<"\"some string\"">]} : () -> ()
+  return
+}

mlir/test/Target/Cpp/types.mlir

+// RUN: mlir-translate -mlir-to-cpp %s | FileCheck %s
+
+// CHECK-LABEL: void opaque_template_args() {
+func @opaque_template_args() {
+  // CHECK-NEXT: f<int>();
+  emitc.call "f"() {template_args = [!emitc<"opaque<\"int\">">]} : () -> ()
+  // CHECK-NEXT: f<byte>();
+  emitc.call "f"() {template_args = [!emitc<"opaque<\"byte\">">]} : () -> ()
+  // CHECK-NEXT: f<unsigned>();
+  emitc.call "f"() {template_args = [!emitc<"opaque<\"unsigned\">">]} : () -> ()
+  // CHECK-NEXT: f<status_t>();
+  emitc.call "f"() {template_args = [!emitc<"opaque<\"status_t\">">]} : () -> ()
+  // CHECK-NEXT: f<std::vector<std::string>>();
+  emitc.call "f"() {template_args = [!emitc.opaque<"std::vector<std::string>">]} : () -> ()
+
+  return
+}