[Mlir-commits] [mlir] [mlir] Cleanup dump() functions. (PR #89216)

[Mehdi Amini]

================
@@ -1019,16 +1019,12 @@ class alignas(8) Operation final
 
   /// Expose a few methods explicitly for the debugger to call for
   /// visualization.
-#ifndef NDEBUG
-  LLVM_DUMP_METHOD operand_range debug_getOperands() { return getOperands(); }
-  LLVM_DUMP_METHOD result_range debug_getResults() { return getResults(); }
-  LLVM_DUMP_METHOD SuccessorRange debug_getSuccessors() {
-    return getSuccessors();
-  }
-  LLVM_DUMP_METHOD MutableArrayRef<Region> debug_getRegions() {
-    return getRegions();
-  }
-#endif
+  /// @{
+  operand_range debug_getOperands();
----------------
joker-eph wrote:

> So the FIXME was fixed by llvm-config.h? However, the LLVM_DUMP_METHOD makes use of the NDEBUG macro which is under the including application's control and LLVM_ENABLE_DEBUG is not present in llvm-config.h.

No it's not fixed, it should be fixed by making use of llvm-config.h to not rely on NDEBUG.

> This is by design of the LLVM_DUMP_METHOD pattern established in https://github.com/llvm/llvm-project/commit/8c209aa8779de57771b64896f805e14cc0016dcd.

I don't think the aspect of "not failing at compile time" is actually desirable "by design" here, why would it be better than relying on llvm-config.h ?


https://github.com/llvm/llvm-project/pull/89216