hazohelet added a comment.
I've noticed several shortcoming/limitations of this patch.
1. Function arguments: When we parse the arguments to a function call, the
callee isn't still resolved, so we don't know whether it's consteval. If the
callee is consteval, its argument is known to be constant-evaluated regardless
of its surrounding evaluation context.
e.g.
consteval int ce(int n){ return n;};
int f() {
int a = ce(__builtin_is_constant_evaluated()); // tautologically true
}
2. init-statement of constexpr-if:
The condition is required to be a constant expression, but the init-stmt before
it isn't.
e.g. Given `if constexpr (InitStmt; Cond) {}`, `InitStmt` is evaluated in the
surrounding evaluation context.
If the init-statement is a declaration we can push initializer evaluation
context like we do in other variables. However, if the init-statement is an
expression, when clang parses the expression, clang doesn't know whether it is
parsing the init-statement or the condition.
https://github.com/llvm/llvm-project/blob/e3c57fdd8439ba82c67347629a3c66f293e1f3d0/clang/lib/Parse/ParseExprCXX.cpp#L2102
If we are to handle these cases with perfection, we need to defer warning
emissions, but I'm skeptical about the value we obtain from this effort.
For the first problem, we can make `IsRuntimeEvaluated` false while parsing
arguments to avoid emitting incorrect diagnostics although it makes false
negatives on tautologically-false uses in arguments.
The second problem is difficult because we cannot avoid incorrect diagnostics
in `InitStmt` expression of constexpr-if with a simple fix. But I think it's
acceptable for the time being because it would be a pretty rare case to use
`is_constant_evaluated` there.
@cor3ntin
BTW, can I separate the initializer evaluation context bug fix part to make
another PR? Another patch I'm working on locally also suffers from the
evaluation context bug, and I want to land that part relatively soon.
Also, it might be nice to separate the NFC libc++ test modifications I recently
uploaded because it's too much despite being NFC. Or we can turn off this
tautology warning against macros. Do you think it's reasonable?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155064/new/
https://reviews.llvm.org/D155064
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
