aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land.
LGTM with a few nits to be fixed. ================ Comment at: clang/lib/AST/ExprConstant.cpp:8156-8158 + } else if (const Expr *E = Value.Base.dyn_cast<const Expr *>()) { + return GetAlignOfExpr(Info, E, UETT_AlignOf); + } else { ---------------- aaron.ballman wrote: > No `else` after a `return`. You can still drop this `else` (and use a regular `if`) because of the preceding `return`. ================ Comment at: clang/lib/AST/ExprConstant.cpp:10685 + if (Src.isLValue()) { + // If we evaluated a pointer, check the minimum known alignment + LValue Ptr; ---------------- aaron.ballman wrote: > Comment missing a full stop at the end. The comment is still missing its full stop at the end. ================ Comment at: clang/lib/Sema/SemaChecking.cpp:246-259 + if (AlignValue < 1) { + S.Diag(AlignOp->getExprLoc(), diag::err_alignment_too_small) << 1; + return true; + } else if (llvm::APSInt::compareValues(AlignValue, MaxValue) > 0) { + S.Diag(AlignOp->getExprLoc(), diag::err_alignment_too_big) + << MaxValue.toString(10); + return true; ---------------- There are a bunch of `else` after `return` issues in this ladder. I would rearrange to put the error cases first and the warning case last. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71499/new/ https://reviews.llvm.org/D71499 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits