aaron.ballman added inline comments.
================
Comment at: lib/AST/ExprConstant.cpp:1110
@@ -1022,1 +1109,3 @@
void set(APValue::LValueBase B, unsigned I = 0, bool BInvalid = false) {
+#ifndef NDEBUG
+ // We only allow a few types of invalid bases. Enforce that here.
----------------
> I was under the impression that asserts are only enabled if !NDEBUG. The code
> only exists to make a few assertions, so I think guarding it with "if
> assertions are enabled" makes that purpose more clear. If my assumption is
> wrong, I'm happy to remove the ifndef.
That's a good point; I forgot that we use NDEBUG as our release+asserts flag
(derp). That being said, this is basically the same as:
```
assert((!BInvalid || isa<MemberExpr>(B.get<Expr*>()) ||
tryUnwrapAllocSizeCall(B.get<Expr*>())) && "Unexpected type of invalid base");
```
Uncertain if that is an improvement or not, but it is shorter-ish. :-)
================
Comment at: lib/Sema/SemaDeclAttr.cpp:728
@@ +727,3 @@
+
+ const ParmVarDecl *Param = FD->getParamDecl(FuncParamNo - 1);
+ if (!Param->getType()->isIntegerType() && !Param->getType()->isCharType()) {
----------------
> Boolean makes no sense, but char is fine here. Thanks
isIntegerType() already covers isCharType(). I think what you want is:
```
if (!Param->getType()->isIntegerType() || Param->getType()->isBooleanType())
```
http://reviews.llvm.org/D14274
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
