aaron.ballman added inline comments.
================
Comment at: clang/docs/LanguageExtensions.rst:2844
+The builtins ``__builtin_align_up``, ``__builtin_align_down``, return their
+first argument aligned up/down to the next multiple of the second argument.
+The builtin ``__builtin_is_aligned`` returns whether the first argument is
----------------
Should explicitly specify whether the arguments are expressed in bytes or bits.
================
Comment at: clang/docs/LanguageExtensions.rst:2853
+
+If Clang can determined that the alignment is be not a power of two at
+compile-time, it will result in a compilation failure. If the alignment
argument
----------------
determined -> determine
is be not -> is not
================
Comment at: clang/lib/AST/ExprConstant.cpp:8154
+static CharUnits getBaseAlignment(EvalInfo &Info, const LValue &Value) {
+ if (const ValueDecl *VD = Value.Base.dyn_cast<const ValueDecl *>()) {
+ return Info.Ctx.getDeclAlign(VD);
----------------
Can go with `const auto *` where the type is spelled out in the initialization
like this (here and elsewhere).
================
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 {
----------------
No `else` after a `return`.
================
Comment at: clang/lib/AST/ExprConstant.cpp:8173
+ }
+ const unsigned SrcWidth = Info.Ctx.getIntWidth(ForType);
+ APSInt MaxValue(APInt::getOneBitSet(SrcWidth, SrcWidth - 1));
----------------
Should drop the top-level `const` qualifier as that isn't an idiom we typically
use for locals.
================
Comment at: clang/lib/AST/ExprConstant.cpp:10685
+ if (Src.isLValue()) {
+ // If we evaluated a pointer, check the minimum known alignment
+ LValue Ptr;
----------------
Comment missing a full stop at the end.
================
Comment at: clang/lib/AST/ExprConstant.cpp:10720
+ return Error(E);
+ assert(Src.isInt() && "Adjusting pointer alignment in IntExprEvaluator?");
+ APSInt AlignedVal =
----------------
This assertion doesn't add much given the above line of code.
================
Comment at: clang/lib/AST/ExprConstant.cpp:10734
+ return Error(E);
+ assert(Src.isInt() && "Adjusting pointer alignment in IntExprEvaluator?");
+ APSInt AlignedVal =
----------------
Same here as above.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14267
+ QualType AstType = E->getArg(0)->getType();
+ if (AstType->isArrayType()) {
+ Src = CGF.EmitArrayToPointerDecay(E->getArg(0)).getPointer();
----------------
Can elide braces.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:224
+ SrcTy->isFunctionPointerType()) {
+ // XXX: this is not quite the right error message since we don't allow
+ // floating point types, or member pointers
----------------
Comment should probably be a FIXME unless you intend to address it as part of
this patch. Also, the comment is missing a full stop (please check all comments
in the patch as many of them are missing one).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71499/new/
https://reviews.llvm.org/D71499
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
