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

JF Bastien via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 6 15:00:04 PST 2019


jfb marked 5 inline comments as done.
jfb added inline comments.


================
Comment at: lib/CodeGen/CGDecl.cpp:1642
 
     CharUnits Size = getContext().getTypeSizeInChars(type);
     if (!Size.isZero()) {
----------------
rjmccall wrote:
> Does this check handle flexible arrays on zero-sized structures properly?  I suppose they're always initialized.
You mean something like
```
void foo(int size) {
  struct Z {};
  Z arr[size];
}
```
?


================
Comment at: lib/CodeGen/CGDecl.cpp:1643
     CharUnits Size = getContext().getTypeSizeInChars(type);
     if (!Size.isZero()) {
       switch (trivialAutoVarInit) {
----------------
rjmccall wrote:
> Can't you just drill to the byref storage right here and avoid all the other complexity and subtle ordering interactions?
We're in the lambda that does the initialization here. The tricky order part is that code that calls the lambda does:

- Block (which was missing the early auto-init)
- Trivial initializer (which has auto-init, then early exit)
- Constant aggregate / constexpr (which might auto-init if it wasn't constant, and then early-exit)
- The other stuff

I don't think here is the right place to do anything... and I'm not sure what you're suggesting.


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


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