[PATCH] D57797: Variable auto-init: fix __block initialization

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 5 21:16:01 PST 2019


rjmccall added inline comments.


================
Comment at: lib/CodeGen/CGDecl.cpp:1611
       drillIntoBlockVariable(*this, Dst, &D);
+    }
     defaultInitNonTrivialCStructVar(Dst);
----------------
Why don't you just initialize the payload right here, after we've drilled down to it?


================
Comment at: lib/CodeGen/CGDecl.cpp:1642
 
     CharUnits Size = getContext().getTypeSizeInChars(type);
     if (!Size.isZero()) {
----------------
Does this check handle flexible arrays on zero-sized structures properly?  I suppose they're always initialized.


================
Comment at: lib/CodeGen/CGDecl.cpp:1643
     CharUnits Size = getContext().getTypeSizeInChars(type);
     if (!Size.isZero()) {
       switch (trivialAutoVarInit) {
----------------
Can't you just drill to the byref storage right here and avoid all the other complexity and subtle ordering interactions?


================
Comment at: lib/CodeGen/CGDecl.cpp:1663
     const VariableArrayType *VlaType =
         dyn_cast_or_null<VariableArrayType>(getContext().getAsArrayType(type));
     if (!VlaType)
----------------
This is a late comment on the main patch, but there's an `ASTContext::getAsVariableArrayType` method.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57797





More information about the cfe-commits mailing list