[PATCH] D45753: Lift JSON library from clang-tools-extra/clangd to llvm/Support.

[Simon Tatham via Phabricator via llvm-commits]

simon_tatham added inline comments.


================
Comment at: include/llvm/Support/JSON.h:315
+  /// For pretty-printing, use the formatv() format_provider below.
+  friend llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Value &);
+
----------------
When I built this locally, I had a strange build failure involving this function with g++ 5.4.0 (i.e. the default compiler on Ubuntu 16.04). It reported, at the definition of this function in `JSON.cpp`, this error:
```
error: ‘llvm::raw_ostream& llvm::json::operator<<(llvm::raw_ostream&, const llvm::json::Value&)’ should have been declared inside ‘llvm::json’
```
for which, apparently, the fix was to add a repeated declaration of this function without the 'friend' keyword and outside the definition of `class Value`.

clang was happy with it, on the other hand. I've no idea :-)


================
Comment at: lib/Support/JSON.cpp:87
+  }
+};
+llvm::Optional<std::nullptr_t> Array::getNull(size_t I) const {
----------------
When I tried to build with this change locally, g++ pointed out a spurious semicolon here.


================
Comment at: lib/Support/JSON.cpp:560
+    case T_Number:
+      OS << format("%g", as<double>());
+      break;
----------------
Could this number formatting be changed? The default %g loses precision – you don't even get enough information to exactly reconstruct the same double you started with.

Also, over in D46054 I'm working on a JSON back end for TableGen, for which I'd find it useful to be able to pass an arbitrary 64-bit integer through this system and still have the full 64 bits of integer value visible in the JSON output file, for the benefit of JSON consumers (e.g. Python `json.load`) that go above the call of duty in returning it as an integer without rounding it to the nearest representable double.

So, would it be possible to have some method of constructing a json::Value that formats as a 64-bit integer literal?


Repository:
  rL LLVM

https://reviews.llvm.org/D45753