rjmccall added inline comments.
================ Comment at: include/clang/AST/FormatString.h:70 AsShort, // 'h' + AsInt, // 'hl' (OpenCL float/int vector element) AsLong, // 'l' ---------------- I think giving this a weird name like `AsShortLong` might help make it clearer to readers below that it's a non-standard modifier. ================ Comment at: lib/AST/FormatString.cpp:728 + return true; + } + ---------------- Your comment here doesn't match the code: you're accepting if this is a FP vector, but otherwise you're falling through. The comment sounds like this check should be `if (CS.isDoubleArg()) return !VectorNumElts.isInvalid();`. ================ Comment at: lib/AST/FormatString.cpp:775 + if (LO.OpenCL && VectorNumElts.isInvalid() && CS.isDoubleArg()) + return false; + ---------------- Can you just break the FP args out of the switch below and add this check there? ================ Comment at: lib/Sema/SemaExpr.cpp:755 } } ---------------- Can you just do this as a separate patch without the printf checking? We're going to want to merge this to the release branch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57390/new/ https://reviews.llvm.org/D57390 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits