[Support][JSON][NFC] Silence GCC warning about broken strict aliasing rules

[kimgr]

Summary

The as<T>() method would trigger the following warning on GCC <7:

warning: dereferencing type-punned pointer will break
    strict-aliasing rules [-Wstrict-aliasing]

  return *reinterpret_cast<T *>(Union.buffer);
                                            ^

Union.buffer is guaranteed to be aligned to whatever types it contains,
and json::Value maintains the invariant that it only calls as<T>() for a
T it has previously placement-newed into Union.buffer. This should
follow the rules for strict aliasing.

Using two static_cast via void * instead of reinterpret_cast
silences the warning and presumably makes GCC understand that no
strict-aliasing violation is happening.

No functional change intended.

Patch by: kimgr (Kim Gräsman)

[xbolva00]

I am in favour of this version.

[kimgr]

Thanks, me too!

Could you commit this for me, or should we maybe wait for @sammccall?

[xbolva00]

Yes, wait for his opinion too :)

[sammccall]

I also prefer this version over suppressing the warning with the pragma.
Thanks!

include/llvm/Support/JSON.h

456 ↗

(On Diff #160255)

Nit: seems worth mentioning that the warning is a false positive, and the upper bound on GCC version (so we know when it's obsolete)

457 ↗

(On Diff #160255)

nit: I guess this should be P (or Storage)

[kimgr]

• Clarify comment
• Fix naming
• Update differential title/summary

[kimgr]

If you're happy with this, could you also commit it for me? Thanks!

[xbolva00]

In D50608#1196560, @kimgr wrote:

If you're happy with this, could you also commit it for me? Thanks!

Yes, thanks!

llvm/trunk/include/llvm/Support/JSON.h

private:
   void moveFrom(const Value &&M);
   friend class Array;
   friend class Object;
 
   template <typename T, typename... U> void create(U &&... V) {
     new (reinterpret_cast<T *>(Union.buffer)) T(std::forward<U>(V)...);
   }
   template <typename T> T &as() const {
-    return *reinterpret_cast<T *>(Union.buffer);
+    // Using this two-step static_cast via void * instead of reinterpret_cast
+    // silences a -Wstrict-aliasing false positive from GCC6 and earlier.
+    void *Storage = static_cast<void *>(Union.buffer);
+    return *static_cast<T *>(Storage);
   }
 
   template <typename Indenter>
   void print(llvm::raw_ostream &, const Indenter &) const;
   friend struct llvm::format_provider<llvm::json::Value>;
 
   enum ValueType : char {
     T_Null,