This revision was automatically updated to reflect the committed changes.
Closed by commit rC331016: [CodeGen] Avoid destructing a callee-destructued
struct type in a (authored by ahatanak, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D45382
Files:
lib/CodeGen/CGCall.cpp
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Thanks, LGTM.
Repository:
rC Clang
https://reviews.llvm.org/D45382
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
ahatanak updated this revision to Diff 144231.
ahatanak marked an inline comment as done.
ahatanak added a comment.
Rename variable to something OldCleanupScopeDepth.
Repository:
rC Clang
https://reviews.llvm.org/D45382
Files:
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGCleanup.cpp
rjmccall added inline comments.
Comment at: lib/CodeGen/CGCall.cpp:3069
+ if (hasAggregateEvaluationKind(type) &&
+ getContext().isParamDestroyedInCallee(type)) {
+EHScopeStack::stable_iterator cleanup =
ahatanak wrote:
> rjmccall wrote:
> > I wonder
ahatanak added a comment.
x.
Comment at: lib/CodeGen/CGCall.cpp:3069
+ if (hasAggregateEvaluationKind(type) &&
+ getContext().isParamDestroyedInCallee(type)) {
+EHScopeStack::stable_iterator cleanup =
rjmccall wrote:
> I wonder if this is something
rsmith added a comment.
In https://reviews.llvm.org/D45382#1060452, @ahatanak wrote:
> In https://reviews.llvm.org/D45382#1060163, @rjmccall wrote:
>
> > Hmm. I'm not actually sure *why* it's not okay to forward callee-cleanup
> > arguments. Do we just not have the necessary logic to disable
rjmccall added inline comments.
Comment at: lib/CodeGen/CGCall.cpp:3069
+ if (hasAggregateEvaluationKind(type) &&
+ getContext().isParamDestroyedInCallee(type)) {
+EHScopeStack::stable_iterator cleanup =
I wonder if this is something we should be
ahatanak updated this revision to Diff 143624.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Address review comments.
Repository:
rC Clang
https://reviews.llvm.org/D45382
Files:
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGCleanup.cpp
lib/CodeGen/CGDecl.cpp
rjmccall added inline comments.
Herald added a reviewer: javed.absar.
Comment at: lib/CodeGen/CodeGenFunction.h:847
+CurrentCleanupStackDepth = C;
+ }
+
You don't need (or want) these accessors, I think; this is just private state
of the CGF object, and
ahatanak updated this revision to Diff 142792.
ahatanak added a comment.
Deactivate the cleanups for callee-destructed parameters that are being
forwarded to a delegated constructor.
I also added a new member (CurrentCleanupStackDepth) to CodeGenFunction that
tracks the stack depth of the most
rjmccall added a comment.
If that's the problem, then I think the right design is for CallArg to include
an optional cleanup reference for a cleanup that can be deactivated at the
instant of the call (we should assert that this exists for parameters that are
destroyed in the callee), and then
ahatanak added a comment.
In https://reviews.llvm.org/D45382#1060163, @rjmccall wrote:
> Hmm. I'm not actually sure *why* it's not okay to forward callee-cleanup
> arguments. Do we just not have the necessary logic to disable the cleanup in
> the caller?
It seems that it would be OK if
rjmccall added a comment.
Hmm. I'm not actually sure *why* it's not okay to forward callee-cleanup
arguments. Do we just not have the necessary logic to disable the cleanup in
the caller?
Repository:
rC Clang
https://reviews.llvm.org/D45382
ahatanak created this revision.
ahatanak added reviewers: rjmccall, rsmith.
Herald added a subscriber: kristof.beyls.
This patch fixes a bug where a struct with an ObjC `__weak` field gets
destructed after it has already been destructed. This bug was introduced in
r328731, which made changes to
14 matches
Mail list logo
