aaron.ballman added inline comments.
================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:749-751
+ const Descriptor::MetadataSize MDSize{sizeof(InlineDescriptor)};
+ Descriptor *D =
+ P.createDescriptor(Src, Ty, MDSize, IsConst, Src.is<const Expr *>());
----------------
Double checking that `Src.is<const Expr *>()` is correct here. That's being
passed as the `IsTemporary` argument. I presume the logic is that if the decl
type is an expression then it has to be a temporary, but that's not entirely
true. Consider C where a compound literal is *not* a temporary but actually
creates an lvalue: `(int){12}`. Will that be an issue?
================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:782
+ P.createDescriptor(Src, Ty.getTypePtr(), MDSize, Ty.isConstQualified(),
+ IsTemporary, false, Init);
if (!D)
----------------
================
Comment at: clang/lib/AST/Interp/Interp.h:950
return false;
if (auto *FD = Ptr.getField()) {
Ptr.deref<T>() = Value.truncate(FD->getBitWidthValue(S.getCtx()));
----------------
Do we need to `initialize()` here as well?
================
Comment at: clang/lib/AST/Interp/Interp.h:962
const Pointer &Ptr = S.Stk.pop<Pointer>();
if (!CheckStore(S, OpPC, Ptr))
return false;
----------------
Same here?
================
Comment at: clang/lib/AST/Interp/InterpBlock.h:77-86
+ char *data() {
+ // Data might contain metadata as well.
+ size_t DataOffset = Desc->getMetadataSize();
+ return reinterpret_cast<char *>(reinterpret_cast<char *>(this) +
+ sizeof(Block) + DataOffset);
+ }
+
----------------
Should we have `const`-correct overloads to provide read-only access?
================
Comment at: clang/lib/AST/Interp/InterpBlock.h:80-88
+ return reinterpret_cast<char *>(reinterpret_cast<char *>(this) +
+ sizeof(Block) + DataOffset);
+ }
+
+ /// Returns a pointer to the raw data, including metadata.
+ /// You are allowed to read Desc->getAllocSize() bytes from this address.
+ char *rawData() {
----------------
Why two levels of `reinterpret_cast` to the same type?
================
Comment at: clang/lib/AST/Interp/InterpBlock.h:97
void invokeCtor() {
- std::memset(data(), 0, getSize());
+ std::memset(rawData(), 0, Desc->getAllocSize());
if (Desc->CtorFn)
----------------
Why do we want to overwrite the metadata here?
================
Comment at: clang/lib/AST/Interp/InterpFrame.cpp:34
B->invokeCtor();
+ auto *ID = localInlineDesc(Local.Offset);
+ ID->Desc = Local.Desc;
----------------
================
Comment at: clang/lib/AST/Interp/Pointer.h:41
+/// The Base field is used to access metadata about the data. For primitive
+/// arrays, the Base is followed by an InitMap. In a variety of case, the
+/// Base is preceded by an InlineDescriptor, which is used to track the
----------------
================
Comment at: clang/lib/AST/Interp/Pointer.h:43
+/// Base is preceded by an InlineDescriptor, which is used to track the
+/// initialization state, among others.
+///
----------------
================
Comment at: clang/lib/AST/Interp/Program.cpp:334
Descriptor *ElemDesc =
- createDescriptor(D, ElemTy.getTypePtr(), IsConst, IsTemporary);
+ createDescriptor(D, ElemTy.getTypePtr(), Descriptor::NoMetadata,
+ IsConst, IsTemporary);
----------------
Why does this one get no metadata while the rest of the descriptors created do?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135750/new/
https://reviews.llvm.org/D135750
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
