ping

ping

This was fixed by https://reviews.llvm.org/D86854.

The parenthesis initialization does not work as documented for GCC or OpenCL vectors, so that has to be fixed too.

Is this patch still being reviewed?

LGTM

It looks like this still doesn't check null correctly (i.e., compare to -1) for data member pointers. Is that correct?

I don't think it would be hard to make this pass and ObjCARCOpt function passes, but we can do that later.

Patch looks good to me, but do you have to change anything to fix the pre-merge check test failure?

OK, thank you. LGTM, but you can wait for an approval from someone more familiar with the pass manager.

The use case for this is a macro in which the call to __builtin_os_log_format that writes to the buffer and the call that uses the buffer appear in two different statements. For example:

Update comment in test case.

In case it wasn't clear, the calls to mayThrow() in the test cases are needed to prevent TryMarkNoThrow from annotating the functions with nounwind, which would cause a lot of churn.

Assert in EmitReturnFromThunk that the result type isn't an aggregate.

In D82513#2142596, @rjmccall wrote:

This seems fine. I do wonder if the "real" bug is that this ought to be handled properly in EmitReturnFromThunk, but regardless, the fix seems acceptable.

Add test case for trivial_abi.

Does the updated patch look okay?

In D82999#2129855, @rjmccall wrote:

In D82999#2129417, @ahatanak wrote:

In test case test13 in clang/test/CodeGenCXX/exceptions.cpp, I think you can turn invoke void @_ZN6test131AC1Ev into call void @_ZN6test131AC1Ev, no? If the false expression throws, there is nothing to clean up in the false expression and also nothing in the true expression has to be cleaned up.

Yes, this is true. It would be possible to enhance Clang's cleanup stack to support this sort of thing — we'd want to be able to mark a cleanup as "currently known inactive" without potentially popping it off the cleanup stack, and then we could have conditional scopes remember the cleanups that were added, deactivate them this way, and then reactivate them after the merge. Swift's cleanup manager supports something similar.

This looks good to me.

And if the constructor call for A in the true expression throws, it's not necessary to call the destructor either since the object hasn't been constructed.

In test case test13 in clang/test/CodeGenCXX/exceptions.cpp, I think you can turn invoke void @_ZN6test131AC1Ev into call void @_ZN6test131AC1Ev, no? If the false expression throws, there is nothing to clean up in the false expression and also nothing in the true expression has to be cleaned up.

After-full-expression cleanup looks fine to me. pushCleanupAfterFullExpr sets the flags and saves the values when it's in a conditional branch.

Thank you, LGTM.

LGTM

To clarify my comment, the lifetime of the block capture in test22 gets extended to the end of the enclosing scope, but of course not beyond the end of the return statement.

Yes, I'm still interested in pursuing this. I'll rebase the patch and commit it if I don't get any more feedback.

Rebase patch.