rjmccall added a comment. Thank you, that's a lot better. Just a few suggestions to promote exhaustive checking.
================ Comment at: lib/AST/ExprConstant.cpp:6246 @@ +6245,3 @@ + + default: + break; ---------------- Again, try to avoid adding default cases. There's an x-macro file for BuiltinTypes, too. ================ Comment at: lib/AST/ExprConstant.cpp:6262 @@ +6261,3 @@ + else if (CanTy->isMemberFunctionPointerType()) + return method_type_class; + break; ---------------- Please turn the else-if into an else with an assert. ================ Comment at: lib/AST/ExprConstant.cpp:6277 @@ +6276,3 @@ + return union_type_class; + break; + ---------------- You can switch over the tag kind here. ================ Comment at: lib/AST/ExprConstant.cpp:6285 @@ +6284,3 @@ + + default: + break; ---------------- Instead of having a default case, you should use an x-macro to exhaustively fill in all the cases that you're assuming can't happen. That way, if somebody adds a new canonical type kind, they'll automatically get a warning here if they don't add it. In this case, since you're working with a canonical type and it can't be dependent, this would look like: #define TYPE(ID, BASE) #define NON_CANONICAL_TYPE(ID, BASE) case Type::ID: #define NON_CANONICAL_UNLESS_DEPENDENT_TYPE(ID, BASE) case Type::ID: #define DEPENDENT_TYPE(ID, BASE) case Type::ID: #include "clang/AST/TypeNodes.def" http://reviews.llvm.org/D16846 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
