erichkeane added a comment.
So I have some heartburn about `IsNegatedConsteval`. A part of me wants to
suggest ditching the `ConstexprKind` and creating a new scoped-enum
`IfStmtKind` (perhaps in the same file) that has just
`None,Constexpr,Consteval,NotConsteval`. Its already a bit hinky that we have
a value of `ConstexprKind` that we don't use, so I'm leaning toward it. Other
reviewers: WDYT?
Otherwise, I think clang-format REALLY needs a run here, and the codegen test
can be fleshed out a bit more. Otherwise, this seems right to me.
================
Comment at: clang/include/clang/AST/Stmt.h:1971
SourceLocation EL = SourceLocation(),
- Stmt *Else = nullptr);
+ Stmt *Else = nullptr, bool IfConstevalIsNegated =
false);
----------------
Not a fan of making this 'IfConstevalIsNegated' a default here... it means it
has to be at the end, where it doesn't particularly make sense. Plus, you have
to change all the calls to this function ANYWAY, right?
================
Comment at: clang/lib/AST/JSONNodeDumper.cpp:1493
+ attributeOnlyIfTrue("isConsteval", IS->isConsteval());
+ attributeOnlyIfTrue("constevalIsNegated", IS->isConsteval() &&
IS->isNegatedConsteval());
}
----------------
Is the first condition necessary here? I would think isNegatedConsteval should
NEVER be true if isConsteval?
================
Comment at: clang/lib/AST/StmtPrinter.cpp:242
+ if(If->isNegatedConsteval())
+ OS << "!";
+ OS << "consteval";
----------------
You had a choice here.... You COULD have blessed the 'c++ operator names', but
chose not to. I guess we all have to choose teams:D
================
Comment at: clang/test/CodeGenCXX/cxx2b-consteval-if.cpp:1
+// RUN: %clang_cc1 -std=c++2b %s -emit-llvm -o - | FileCheck %s
--implicit-check-not=should_not_be_used
+
----------------
I tend to like the 'CHECK' lines being inline with the code, it makes it easier
to follow in these cases. Additionally, I think the check-lines should be more
specific (that they are actually checking for 'call' instructions).
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
