[PATCH] D153294: [clang] Do not create ExprWithCleanups while checking immediate invocation

Wed Jun 28 03:21:45 PDT 2023

ilya-biryukov added a comment.

In D153294#4448342 <https://reviews.llvm.org/D153294#4448342>, @Fznamznon wrote:

> I'm having a slight trouble with understanding why this part is required and how to implement the test ...
>
> Also, simply adding a flag to `MaybeCreateExprWithCleanup` signaling that we're processing an immediate invocation to not discard cleanups also passes check-clang and fixes original memory leak problem. Maybe we should just do that?

I also struggle to see how any of the cleanup objects may end up being inside `ExprWithCleanups` under immediate invocation.
Any use of block literals inside constant expressions results in an error and compound literals are handled like temporaries in C++ and don't create cleanup objects (see the reference above).

I would maybe guard the code doing the processing immediate invocations with `assert(LangOpts.CPlusPlus)` to sanity-check that we don't end up calling this in C in some very distant future.
I believe we should make the proposed change (add potentially redundant `ExprWithCleanups` without touching the cleanups objects or flags) without blocking on Richard's approval.
It definitely fixes the somewhat common problem we have now and potentially leaves behind a few corner cases with blocks which are much less likely (or even impossible) to occur in practice.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153294