nickdesaulniers added inline comments.
================ Comment at: clang/include/clang/AST/Expr.h:30 #include "clang/Basic/TypeTraits.h" +#include "llvm/ADT/Optional.h" #include "llvm/ADT/APFloat.h" ---------------- Please keep the list of `#include`s alphabetically sorted. ================ Comment at: clang/include/clang/AST/Expr.h:3808 /// be used to express the computation. +namespace { + ---------------- Let's keep additions before the comment block specific to `BinaryOperator`'s definition. ================ Comment at: clang/include/clang/AST/Expr.h:3821-3826 +struct ICEDiag { + ICEKind Kind; + SourceLocation Loc; + + ICEDiag(ICEKind IK, SourceLocation l) : Kind(IK), Loc(l) {} +}; ---------------- Move back. ================ Comment at: clang/include/clang/AST/Expr.h:3837 + /* ICEDiag Diag{IK_NotYetEvaluated, SourceLocation()}; */ + llvm::Optional<enum ICEKind> IK; + /* ICEKind IK = IK_NotYetEvaluated; */ ---------------- Let's make this private, initialize it to `llvm::None` (see llvm/include/llvm/ADT/None.h), then add getters and setters. They should accept/return `ICEKind`; basically, I don't think we should support un-setting the ICEKind once we know what it is. Also, prefer `ICEKind` in C++; `enum ICEKind` is required only in C. ================ Comment at: clang/lib/AST/ExprConstant.cpp:15516 + BinaryOperator *Exp = cast<BinaryOperator>(E); + if (Exp->IK.has_value()) return ICEDiag(Exp->IK.value(), E->getBeginLoc()); + /* if (Exp->IK != IK_NotYetEvaluated) return ICEDiag(Exp->IK, E->getBeginLoc()); */ ---------------- make this two lines? `$ git-clang-format HEAD~` should reformat this for you. ================ Comment at: clang/lib/AST/ExprConstant.cpp:15517 + if (Exp->IK.has_value()) return ICEDiag(Exp->IK.value(), E->getBeginLoc()); + /* if (Exp->IK != IK_NotYetEvaluated) return ICEDiag(Exp->IK, E->getBeginLoc()); */ switch (Exp->getOpcode()) { ---------------- remove ================ Comment at: clang/lib/AST/ExprConstant.cpp:15750 return EvaluateCPlusPlus11IntegralConstantExpr(Ctx, this, nullptr, Loc); - ICEDiag D = CheckICE(this, Ctx); ---------------- undo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131416/new/ https://reviews.llvm.org/D131416 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits