cor3ntin added inline comments.
================
Comment at: clang/include/clang/AST/Stmt.h:166
+ /// True if this if statement is a if consteval statement.
+ unsigned IsConsteval : 1;
----------------
erichkeane wrote:
> Not sure how others feel here, but for me, I kinda hate that we're using
> 'unsigned' bitfields for all of this, particularly because these two are
> mutually exclusive. I'd prefer (though listen to others here first) if the
> type of this was something more like:
>
> IfEvalKind EvalKind : 2; // where IfEvalKind is enum class IfEvalKind {None,
> Constexpr, Consteval};
I was not quite sure where to put an enum I could reuse in different place.
But I think I'd agree with you otherwise.
Any suggestion for where to put it?
================
Comment at: clang/include/clang/Sema/Sema.h:9137
+ "Must be in an expression evaluation context");
+ for (auto it = ExprEvalContexts.rbegin(); it != ExprEvalContexts.rend();
+ it++) {
----------------
erichkeane wrote:
> What is our iterator type? I THINK at minimum require this to be `auto *`
The iterator type is ungodly. Likely
`SmallVector<ExpressionEvaluationContextRecord, 8>::reverse_iterator`. I think
auto here is more readable but I can change if you want to
================
Comment at: clang/lib/Parse/ParseStmt.cpp:1363
+ if (Tok.is(tok::kw_consteval)) {
+ Diag(Tok, getLangOpts().CPlusPlus17 ?
diag::warn_cxx14_compat_constexpr_if
+ : diag::ext_constexpr_if);
----------------
erichkeane wrote:
> Is this diag here right? We're in the `kw_consteval` branch, but it seems
> we're warning about constexpr-if?
Yup, forgot to remove that
================
Comment at: clang/lib/Parse/ParseStmt.cpp:1382
+ if (IsConsteval) {
+ Diag(Tok, getLangOpts().CPlusPlus2b ? diag::warn_cxx20_compat_consteval_if
----------------
erichkeane wrote:
> Why is this here instead of with the `kw_consteval` handling?
To not have a warning when there is an error. Maybe we don't care?
In any case, I've cleanup that
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110482/new/
https://reviews.llvm.org/D110482
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
