[JSON][NFC] Support print and dump methods in json::Value

[npanchen]

No description provided.

[llvmbot]

@llvm/pr-subscribers-llvm-support

Author: Nikolay Panchenko (npanchen)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/129302.diff

2 Files Affected:

• (modified) llvm/include/llvm/Support/JSON.h (+8)
• (modified) llvm/lib/Support/JSON.cpp (+4)

diff --git a/llvm/include/llvm/Support/JSON.h b/llvm/include/llvm/Support/JSON.h
index 14a5c7142ed8c..3e3f783ffb857 100644
--- a/llvm/include/llvm/Support/JSON.h
+++ b/llvm/include/llvm/Support/JSON.h
@@ -472,6 +472,14 @@ class Value {
     return LLVM_LIKELY(Type == T_Array) ? &as<json::Array>() : nullptr;
   }
 
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+  void print(llvm::raw_ostream &OS) const;
+  LLVM_DUMP_METHOD void dump() const {
+    print(llvm::dbgs());
+    llvm::dbgs() << '\n';
+  }
+#endif // !NDEBUG || LLVM_ENABLE_DUMP
+
 private:
   void destroy();
   void copyFrom(const Value &M);
diff --git a/llvm/lib/Support/JSON.cpp b/llvm/lib/Support/JSON.cpp
index a5c617bb4a076..98fef5dcf68a3 100644
--- a/llvm/lib/Support/JSON.cpp
+++ b/llvm/lib/Support/JSON.cpp
@@ -182,6 +182,10 @@ void Value::destroy() {
   }
 }
 
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+void Value::print(llvm::raw_ostream &OS) const { OS << *this; }
+#endif // !NDEBUG || LLVM_ENABLE_DUMP
+
 bool operator==(const Value &L, const Value &R) {
   if (L.kind() != R.kind())
     return false;

[npanchen]

@nikic ping

[nikic]

Do we need print() if we already have operator<<? Esp. as they don't quite match in this implementation.

[npanchen]

Can you elaborate why they don't match ?
I've added print more for completeness as usually classes do have a pair to print (w/o \n) and dump(w/ \n)

[nikic]

Can you elaborate why they don't match ?

Nevermind, I misread the code.

[npanchen]

@nikic pin

[nikic]

Can you elaborate why they don't match ?

Nevermind, I misread the code.

[mscuttari]

@npanchen Having this conditional compilation inside a header file breaks the linking phase in out-of-tree projects which are built in debug mode (that is, with the NDEBUG variable not defined) and at the same time link with a release build of LLVM (that is, with the NDEBUG variable defined).

[npanchen]

@npanchen Having this conditional compilation inside a header file breaks the linking phase in out-of-tree projects which are built in debug mode (that is, with the NDEBUG variable not defined) and at the same time link with a release build of LLVM (that is, with the NDEBUG variable defined).

@mscuttari #131639 that should fix it. However, I'm not sure using debug header file and linking with release is what needs to be handled by community.

[mscuttari]

Thanks for the update, I will build the proposed version and see it actually fixes the problem.
I don't agree with the rest of your sentence though. Compiling LLVM in release mode, and the out-of-tree project in debug mode, is a very common approach to reduce compilation times. In general, looking beyond LLVM, a debug build of the software stack should not be needed to build your own downstream software.

[npanchen]

Thanks for the update, I will build the proposed version and see it actually fixes the problem. I don't agree with the rest of your sentence though. Compiling LLVM in release mode, and the out-of-tree project in debug mode, is a very common approach to reduce compilation times. In general, looking beyond LLVM, a debug build of the software stack should not be needed to build your own downstream software.

That's why I'm not sure. On one hand I do agree that it makes life of the downstream project easier, on another I was hearing
"downstream's problem is downstream's problem", as when you include header file as if debug is enabled, it's rightful to expect proper symbols are defined.