aaron.ballman added inline comments.
================ Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1237-1240 + if (DiscardResult) + return this->emitIncPop(*T, E); + + return this->emitInc(*T, E); ---------------- Style question: should we prefer doing something like: `return DiscardResult ? this->emitIncPop(*T, E) : this->emitInc(*T, E);` to discourage accidentally adding code between the two increment operations? (And applying this style more generally for discarded results?) Alternatively, do we want to add a helper function to `ByteCodeExprGen` called `doEmitInc()` that hides the discarded result behavior so the `Visit*` functions don't have to keep doing that dance? ================ Comment at: clang/lib/AST/Interp/Interp.h:331 +template <typename T, bool IsInc, bool PushVal> +bool IncDecHelper(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { ---------------- Do we want to use enums for `IsInc` and `PushVal` so that call sites use `IncDeclHelper<SomeType, Increment::Yes, Push::Yes>` for clarity? ================ Comment at: clang/lib/AST/Interp/Interp.h:370-373 + } else { + S.CCEDiag(E, diag::note_constexpr_overflow) << APResult << Type; + return S.noteUndefinedBehavior(); + } ---------------- ================ Comment at: clang/test/AST/Interp/literals.cpp:376 + static_assert(zero() == 0, ""); + + constexpr int three() { ---------------- I'd like some tests that the computed values are correct. ``` constexpr int zero() { int a = 0; return a++; } static_assert(zero() == 0); constexpr int one() { int a = 0; return ++a; } static_assert(one() == 1); constexpr int zero_again() { int a = 0; return a--; } static_assert(zero_again() == 0); constexpr int neg_one() { int a = 0; return --a; } static_assert(neg_one() == -1); ``` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136423/new/ https://reviews.llvm.org/D136423 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits