[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9716
+def err_bit_cast_non_trivially_copyable : Error<
+  "__builtin_bit_cast %select{source|destination}0 type must be a trivially copyable">;
+def err_bit_cast_type_size_mismatch : Error<
----------------
Drop the "a".


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9718
+def err_bit_cast_type_size_mismatch : Error<
+  "__builtin_bit_cast source size does not match destination size (%0 and %1)">;
 } // end of sema component.
----------------
What does "match" mean? Can this be clearer?


================
Comment at: clang/lib/AST/ExprConstant.cpp:5364
+public:
+  APBuffer(CharUnits Width, bool TargetIsLittleEndian)
+      : Bytes(Width.getQuantity()),
----------------
Use an `enum class` instead of a `bool`.


================
Comment at: clang/lib/AST/ExprConstant.cpp:5368
+
+  bool readObject(CharUnits Offset, CharUnits Width,
+                  SmallVectorImpl<unsigned char> &Output) const {
----------------
`nodiscard` seems useful here.


================
Comment at: clang/lib/AST/ExprConstant.cpp:5377
+    }
+    if (llvm::sys::IsLittleEndianHost != TargetIsLittleEndian)
+      std::reverse(Output.begin(), Output.end());
----------------
Similar to this, you probably want to assert that `CHAR_BIT == ` whatever we use to denote target bitwidth.


================
Comment at: clang/lib/AST/ExprConstant.cpp:5459
+
+  bool visitRecord(const APValue &Val, QualType Ty, CharUnits Offset) {
+    const RecordDecl *RD = Ty->getAsRecordDecl();
----------------
Does this properly fail when there's a vtable?


Repository:
  rC Clang

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

https://reviews.llvm.org/D62825