aaron.ballman added a comment.
In D144943#4334500 <https://reviews.llvm.org/D144943#4334500>, @tbaeder wrote:
> Pig
đˇ
================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:74
+ // FIXME: Diagnostics.
+ if (ToType->isArrayType() || ToType->isPointerType())
+ return false;
----------------
Member pointer types? Unions? volatile-qualified types? There's quite a few
restrictions here for constexpr contexts.
================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:87
+ if (!ToT) {
+ // Conversion to an array or record type.
+ return this->emitBitCastPtr(E);
----------------
We returned earlier if `ToType` is an array, so this comment is a bit
suspicious.
================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:101
+ // Conversion to a primitive type. FromType can be another
+ // primitive type, or a record/array.
+ //
----------------
All the same restrictions for `ToType` apply to `FromType`:
http://eel.is/c++draft/bit.cast#3
================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:92
+ // FIXME: Diagnostics.
+ if (*ToT == PT_Ptr)
+ return false;
----------------
tbaeder wrote:
> One of the problems here is that right now, //all// diagnostics are emitted
> during interpretation time.
Didn't we early return in this case as well?
================
Comment at: clang/lib/AST/Interp/Integral.h:179
+
+ memcpy(&V, Buff, sizeof(ReprT));
+ return Integral(V);
----------------
Consistency with below.
================
Comment at: clang/lib/AST/Interp/Interp.h:1366
+ size_t BuffSize = APFloat::semanticsSizeInBits(*Sem) / 8;
+ std::byte Buff[BuffSize];
+
----------------
tbaeder wrote:
> This is a variable sized array. That needs to go of course, but is the best
> way to heap allocate? Or can we actually use `alloca` in clang code?
We usually go with:
```
std::vector<std::byte> Buff(BuffSize);
if (!DoBitCast(S, FromPtr, Buff.data(), BuffSize))
return false;
```
================
Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:35
+ std::byte *Buff;
+ unsigned Offset;
+ size_t BuffSize;
----------------
We should probably be consistent about using `size_t` in this class so there's
no chance of size conversion between types.
================
Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:33
+struct Foo {
+ std::byte *Buff;
----------------
tbaeder wrote:
> This needs to be renamed obviously, but I was wondering if this already
> exists somewhere in the llvm/clang codebase...
I'm not aware of one, but perhaps this exists in LLVM somewhere?
================
Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:81
+/// Rotate things around for big endian targets.
+static void fiddleMemory(std::byte *M, size_t N) {
+ for (size_t I = 0; I != (N / 2); ++I)
----------------
tbaeder wrote:
> Same here, I feel like this should already be available?
See
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Support/SwapByteOrder.h
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144943/new/
https://reviews.llvm.org/D144943
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
