aaron.ballman added inline comments.
================
Comment at: include/clang/Basic/AttrDocs.td:1502-1503
+ evaluated at compile-time;
+ - A dependent integral value which depends on one or more template arguments,
+ be they type or non-type e.g. ``sizeof(T)`` within the scope of
+ ``template<typename T> struct S;``.
----------------
Slight wording tweak:
"A type or non-type dependent integral value..."
drop the "be they type or non-type"
Same comment applies below.
================
Comment at: lib/CodeGen/TargetInfo.cpp:7665
+static llvm::APSInt getConstexprInt(const Expr *E, const ASTContext& Ctx)
+{
+ assert(E);
----------------
The curly brace should bind to the end of the function parameter list. I'd just
run the whole patch through clang-format and then fix up anything that goes
wrong in the .td files.
================
Comment at: lib/CodeGen/TargetInfo.cpp:7669
+ llvm::APSInt Tmp{32, 0};
+ E->EvaluateAsInt(Tmp, Ctx);
+
----------------
Are you expecting this to always succeed, or are you relying on the initialized
value in the case where this fails?
================
Comment at: lib/Sema/SemaDeclAttr.cpp:5473
+static bool checkAllAreIntegral(Sema &S, const AttributeList &Attr) {
+ for (auto i = 0u; i != Attr.getNumArgs(); ++i) {
+ Expr* E = Attr.getArgAsExpr(i);
----------------
Don't use `auto` here either, just use `unsigned`. Also `i` doesn't match our
coding standard.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:5474
+ for (auto i = 0u; i != Attr.getNumArgs(); ++i) {
+ Expr* E = Attr.getArgAsExpr(i);
+ if (E && !E->getType()->isIntegralOrEnumerationType()) {
----------------
* binds to `E` not to `Expr`
================
Comment at: lib/Sema/SemaDeclAttr.cpp:5484
+
+ return true;
+}
----------------
This still returns `true` even if `E` is null; is that intended?
https://reviews.llvm.org/D39857
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
