rsmith accepted this revision.
rsmith 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:
> 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.
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
