[PATCH] D146771: [support] Provide overload to PrintNumber that use C++ types

[Paul Kirth via Phabricator via llvm-commits]

paulkirth added inline comments.


================
Comment at: llvm/include/llvm/Support/JSON.h:350
   // Integers (except boolean and uint64_t).
   // Must be non-narrowing convertible to int64_t.
+  template <
----------------
I didn't love that I had to change these, but the non-narrowing issue forced me to add `unsigned long long`. I looked into just pushing all of the unsigned integers into the `uint64_t` version, but ran into some strange behavior and test failures that I couldn't explain.

For example an MCA test failed, but the JSON appeared equivalent. The difference was that despite being equivalent "numbers", the values were not integers that the `getInteger()` API for json::Objects could resolve (i.e. it relies on them being `int64_t`, and some were now `uint64_t`).

Some of that likely is tied to the JSON rules for numeric types, but someone with more knowledge here should probably weigh in. Also template metaprogramming isn't something I do often, so there may be better ways to express these constraints.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146771/new/

https://reviews.llvm.org/D146771