aaron.ballman added a comment.
I think this LGTM, but I'm holding off on signing off until @shafik is
satisfied with the part he was asking about. You should add a release note for
the fix, too.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:17891-17894
+ // It is possible that some subexpression of current immediate invocation
+ // was handled from another expression evaluation context. Do not handle
+ // current immediate invocation if some of its subexpressions failed
+ // before.
----------------
Minor grammar nits.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:17973
- /// When we have more then 1 ImmediateInvocationCandidates we need to check
- /// for nested ImmediateInvocationCandidates. when we have only 1 we only
- /// need to remove ReferenceToConsteval in the immediate invocation.
- if (Rec.ImmediateInvocationCandidates.size() > 1) {
+ /// When we have more then 1 ImmediateInvocationCandidates or previously
+ /// failed immediate invocations, we need to check for nested
----------------
================
Comment at: clang/lib/Sema/SemaExpr.cpp:17979
+ if (Rec.ImmediateInvocationCandidates.size() > 1 ||
+ SemaRef.FailedImmediateInvocations.size()) {
----------------
Fznamznon wrote:
> cor3ntin wrote:
> > Shouln't we clear `FailedImmediateInvocations` at the end of a full
> > expression to avoid going there if there was an error in another expression
> The idea sounds reasonable, but I'm not sure it can work with current
> algorithm of handling immediate invocations. Immediate invocations are
> handled when expression evaluation context is popped, so we need to remember
> previously failed immediate invocations until then, even though they could
> happen in two different full expressions.
>
> ```
> consteval foo() { ... }
> void bar() {
> int a = foo(/* there might be a failed immediate invocation attached to
> initializer context */); // this is a full-expression
> int b = foo(); // this is another full-expression
> } // This is where immediate invocations that appear in function context are
> processed
> ```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146234/new/
https://reviews.llvm.org/D146234
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
