[PATCH] D77675: [mlir][Diagnostic] Don't store Operation arguments as a DiagnosticArgument

[River Riddle via Phabricator via llvm-commits]

rriddle created this revision.
rriddle added a reviewer: stellaraccident.
Herald added subscribers: llvm-commits, grosul1, Joonsoo, liufengdb, lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst, shauheen, burmako, jpienaar, mehdi_amini.
Herald added a project: LLVM.
stellaraccident accepted this revision.
stellaraccident added a comment.
This revision is now accepted and ready to land.

Thanks!


Diagnostics may be cached in the parallel diagnostic handler to preserve proper ordering. Storing the Operation as a DiagnosticArgument is problematic as the operation may be erased or changed before it finally gets printed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77675

Files:
  mlir/include/mlir/IR/Diagnostics.h
  mlir/lib/IR/Diagnostics.cpp


Index: mlir/lib/IR/Diagnostics.cpp
===================================================================
--- mlir/lib/IR/Diagnostics.cpp
+++ mlir/lib/IR/Diagnostics.cpp
@@ -36,12 +36,12 @@
 // DiagnosticArgument
 //===----------------------------------------------------------------------===//
 
-// Construct from an Attribute.
+/// Construct from an Attribute.
 DiagnosticArgument::DiagnosticArgument(Attribute attr)
     : kind(DiagnosticArgumentKind::Attribute),
       opaqueVal(reinterpret_cast<intptr_t>(attr.getAsOpaquePointer())) {}
 
-// Construct from a Type.
+/// Construct from a Type.
 DiagnosticArgument::DiagnosticArgument(Type val)
     : kind(DiagnosticArgumentKind::Type),
       opaqueVal(reinterpret_cast<intptr_t>(val.getAsOpaquePointer())) {}
@@ -71,9 +71,6 @@
   case DiagnosticArgumentKind::Integer:
     os << getAsInteger();
     break;
-  case DiagnosticArgumentKind::Operation:
-    getAsOperation().print(os, OpPrintingFlags().useLocalScope());
-    break;
   case DiagnosticArgumentKind::String:
     os << getAsString();
     break;
@@ -131,6 +128,14 @@
   return *this;
 }
 
+/// Stream in an Operation.
+Diagnostic &Diagnostic::operator<<(Operation &val) {
+  std::string str;
+  llvm::raw_string_ostream os(str);
+  os << val;
+  return *this << os.str();
+}
+
 /// Outputs this diagnostic to a stream.
 void Diagnostic::print(raw_ostream &os) const {
   for (auto &arg : getArguments())
Index: mlir/include/mlir/IR/Diagnostics.h
===================================================================
--- mlir/include/mlir/IR/Diagnostics.h
+++ mlir/include/mlir/IR/Diagnostics.h
@@ -57,7 +57,6 @@
     Attribute,
     Double,
     Integer,
-    Operation,
     String,
     Type,
     Unsigned,
@@ -84,12 +83,6 @@
     return static_cast<int64_t>(opaqueVal);
   }
 
-  /// Returns this argument as an operation.
-  Operation &getAsOperation() const {
-    assert(getKind() == DiagnosticArgumentKind::Operation);
-    return *reinterpret_cast<Operation *>(opaqueVal);
-  }
-
   /// Returns this argument as a string.
   StringRef getAsString() const {
     assert(getKind() == DiagnosticArgumentKind::String);
@@ -132,14 +125,6 @@
                                      sizeof(T) <= sizeof(uint64_t)>::type * = 0)
       : kind(DiagnosticArgumentKind::Unsigned), opaqueVal(uint64_t(val)) {}
 
-  // Construct from an operation reference.
-  explicit DiagnosticArgument(Operation &val) : DiagnosticArgument(&val) {}
-  explicit DiagnosticArgument(Operation *val)
-      : kind(DiagnosticArgumentKind::Operation),
-        opaqueVal(reinterpret_cast<intptr_t>(val)) {
-    assert(val && "expected valid operation");
-  }
-
   // Construct from a string reference.
   explicit DiagnosticArgument(StringRef val)
       : kind(DiagnosticArgumentKind::String), stringVal(val) {}
@@ -229,6 +214,12 @@
   /// Stream in an OperationName.
   Diagnostic &operator<<(OperationName val);
 
+  /// Stream in an Operation.
+  Diagnostic &operator<<(Operation &val);
+  Diagnostic &operator<<(Operation *val) {
+    return *this << *val;
+  }
+
   /// Stream in a range.
   template <typename T> Diagnostic &operator<<(iterator_range<T> range) {
     return appendRange(range);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D77675.255775.patch
Type: text/x-patch
Size: 3190 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200407/5b035a22/attachment.bin>