rjmccall added inline comments.
================ Comment at: include/clang/AST/Expr.h:1687 + // "Stmt *" for the predefined identifier. It is present if and only if + // hasFunctionName() is true and is in fact a "StringLiteral *". + ---------------- "always" would be clearer than "in fact". ================ Comment at: include/clang/AST/Stmt.h:279 + /// in PredefinedExpr::IdentType. + unsigned Type : 4; + ---------------- Since you're changing this around anyway, please make it a "kind" rather than a "type". Even if it's just the field name for now, it's progress. ================ Comment at: include/clang/AST/Stmt.h:283 + /// for the predefined identifier. + unsigned HasFnName : 1; + ---------------- Not sure why this is abbreviated. ================ Comment at: lib/AST/Expr.cpp:470 + "IdentType do not fit in PredefinedExprBitfields!"); + bool HasFunctionName = !!SL; + PredefinedExprBits.HasFnName = HasFunctionName; ---------------- `!= nullptr` is clearer. Repository: rC Clang https://reviews.llvm.org/D53605 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
