cor3ntin added inline comments.
================
Comment at: clang/lib/Parse/ParseStmt.cpp:1445-1450
Sema::ExpressionEvaluationContext Context =
Sema::ExpressionEvaluationContext::DiscardedStatement;
- if (NotLocation.isInvalid() && IsConsteval)
+ if (NotLocation.isInvalid() && IsConsteval) {
Context = Sema::ExpressionEvaluationContext::ImmediateFunctionContext;
+ ShouldEnter = true;
+ }
----------------
rsmith wrote:
> rsmith wrote:
> > It looks to me like this is still incorrectly updating the context in some
> > cases. We shouldn't enter an immediate function context if we're already in
> > a discarded statement context. For example, Clang currently rejects this
> > valid code:
> >
> > ```
> > auto f() {
> > if constexpr (false) {
> > if consteval {
> > return 0;
> > }
> > }
> > return 0.0;
> > }
> > ```
> >
> > ... and it looks like it still will after this change. I think we should
> > not enter a new context here if the existing context is a discarded
> > statement context.
> Hm, not updating the context also seems wrong; then we'd reject things like
> this:
> ```
> consteval int *make() { return new int; }
> auto f() {
> if constexpr (false) {
> if consteval {
> // Immediate function context, so call to `make()` is valid.
> // Discarded statement context, so `return 0;` is valid too.
> delete make();
> return 0;
> }
> }
> return 0.0;
> }
> ```
> Perhaps we need to track whether we're in a discarded statement and whether
> we're in an immediate function context separately rather than treating them
> as mutually exclusive. I think it makes sense to go ahead with this patch
> as-is and deal with the bug that we overwrite a discarded statement context
> with an immediate function context separately.
Hum, I missed these scenarios entirely.
But I agree with you, not entering an immediate context would be equally bad.
I think the solution in both cases if to test whether any of the parent context
is discarded or immediate, instead of just looking at the current context
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112089/new/
https://reviews.llvm.org/D112089
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
