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

2019-02-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. SGTM, thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57797/new/ https://reviews.llvm.org/D57797 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

2019-02-07 Thread JF Bastien via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rC353495: Variable auto-init: fix __block initialization (authored by jfb, committed by ). Changed prior to commit:

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

2019-02-07 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 185896. jfb added a comment. - Remove whitespace changes. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57797/new/ https://reviews.llvm.org/D57797 Files: lib/CodeGen/CGDecl.cpp test/CodeGenCXX/trivial-auto-var-init.cpp

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

2019-02-07 Thread JF Bastien via Phabricator via cfe-commits
jfb marked an inline comment as done. jfb added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1642 CharUnits Size = getContext().getTypeSizeInChars(type); if (!Size.isZero()) { jfb wrote: > rjmccall wrote: > > Does this check handle flexible

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

2019-02-07 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 185892. jfb marked 4 inline comments as done. jfb added a comment. - Simplify patch greatly. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57797/new/ https://reviews.llvm.org/D57797 Files: lib/CodeGen/CGDecl.cpp

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

2019-02-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1726 +emitByrefStructureInit(emission); + } + jfb wrote: > Note that we still want this to be pulled out in this way because > `emitByrefStructureInit` emits the call to the initializer (in

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

2019-02-07 Thread JF Bastien via Phabricator via cfe-commits
jfb marked 2 inline comments as done. jfb added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1605 -emitByrefStructureInit(emission); - // Initialize the variable here if it doesn't have a initializer and it is a rjmccall wrote: > Are these changes

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

2019-02-07 Thread JF Bastien via Phabricator via cfe-commits
jfb marked 2 inline comments as done. jfb added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1726 +emitByrefStructureInit(emission); + } + Note that we still want this to be pulled out in this way because `emitByrefStructureInit` emits the call to

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

2019-02-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1605 -emitByrefStructureInit(emission); - // Initialize the variable here if it doesn't have a initializer and it is a Are these changes still needed? Comment at:

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

2019-02-07 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 185883. jfb added a comment. - Only initialize __block's storage. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57797/new/ https://reviews.llvm.org/D57797 Files: lib/CodeGen/CGDecl.cpp

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

2019-02-07 Thread JF Bastien via Phabricator via cfe-commits
jfb marked 3 inline comments as done. jfb added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1643 CharUnits Size = getContext().getTypeSizeInChars(type); if (!Size.isZero()) { switch (trivialAutoVarInit) { rjmccall wrote: > jfb wrote: > >

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

2019-02-06 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1643 CharUnits Size = getContext().getTypeSizeInChars(type); if (!Size.isZero()) { switch (trivialAutoVarInit) { jfb wrote: > rjmccall wrote: > > Can't you just drill to the

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

2019-02-06 Thread JF Bastien via Phabricator via cfe-commits
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

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

2019-02-05 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1611 drillIntoBlockVariable(*this, Dst, ); +} defaultInitNonTrivialCStructVar(Dst); rjmccall wrote: > Why don't you just initialize the payload right here, after we've drilled >

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

2019-02-05 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1611 drillIntoBlockVariable(*this, Dst, ); +} defaultInitNonTrivialCStructVar(Dst); Why don't you just initialize the payload right here, after we've drilled down to it?

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

2019-02-05 Thread JF Bastien via Phabricator via cfe-commits
jfb created this revision. jfb added reviewers: rjmccall, pcc, kcc. Herald added subscribers: cfe-commits, dexonsmith, jkorous. Herald added a project: clang. Automatic initialization [1] of __block variables was happening too late, which caused self-init usage to crash, such as here: typedef