From 964997a143cfa0356d1cd21eb3e6d2170c975c38 Mon Sep 17 00:00:00 2001
From: wpmed92 <ahmedharmouche92@gmail.com>
Date: Mon, 27 Feb 2023 12:51:28 -0800
Subject: [PATCH] [mlir][core] Fix ValueRange printing in AsmPrinter

The ValueRange printing behaviour of `OpAsmPrinter` and `AsmPrinter` is different, as reported [[ https://github.com/llvm/llvm-project/issues/59334 | here ]]

```
static void testPrint(AsmPrinter &p, Operation *op, ValueRange operands) {
  p << '(' << operands << ')';
}
```
Although the base `AsmPrinter` is passed as the first parameter (and not `OpAsmPrinter`), the code compiles fine. However, instead of the SSA values, the types for the operands will be printed. This is a violation of the Liskov Substitution Principle.
The desired behaviour would be that the above code does not compile. The reason it compiles, is that for the above code, the `TypeRange` version will be selected for the `<<` operator, since `ValueRange` is implicitly converted to `TypeRange`:
```
template <typename AsmPrinterT>
inline std::enable_if_t<std::is_base_of<AsmPrinter, AsmPrinterT>::value,
                        AsmPrinterT &>
operator<<(AsmPrinterT &p, const TypeRange &types) {
  llvm::interleaveComma(types, p);
  return p;
}
```
---
 mlir/include/mlir/IR/OpImplementation.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/mlir/include/mlir/IR/OpImplementation.h b/mlir/include/mlir/IR/OpImplementation.h
index 92f4c61..e2cd8dc 100644
--- a/mlir/include/mlir/IR/OpImplementation.h
+++ b/mlir/include/mlir/IR/OpImplementation.h
@@ -302,6 +302,7 @@ operator<<(AsmPrinterT &p, const ValueTypeRange<ValueRangeT> &types) {
   llvm::interleaveComma(types, p);
   return p;
 }
+
 template <typename AsmPrinterT>
 inline std::enable_if_t<std::is_base_of<AsmPrinter, AsmPrinterT>::value,
                         AsmPrinterT &>
@@ -309,6 +310,18 @@ operator<<(AsmPrinterT &p, const TypeRange &types) {
   llvm::interleaveComma(types, p);
   return p;
 }
+
+// Prevent matching the TypeRange version above for ValueRange
+// printing through base AsmPrinter. This is needed so that the
+// ValueRange printing behaviour does not change from printing
+// the SSA values to printing the types for the operands when
+// using AsmPrinter instead of OpAsmPrinter.
+template <typename AsmPrinterT, typename T>
+inline std::enable_if_t<std::is_same<AsmPrinter, AsmPrinterT>::value &&
+                            std::is_convertible<T &, ValueRange>::value,
+                        AsmPrinterT &>
+operator<<(AsmPrinterT &p, const T &other) = delete;
+
 template <typename AsmPrinterT, typename ElementT>
 inline std::enable_if_t<std::is_base_of<AsmPrinter, AsmPrinterT>::value,
                         AsmPrinterT &>
-- 
2.7.4