[PATCH] D57390: OpenCL: Don't promote vector args to printf

[John McCall via Phabricator via cfe-commits]

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