[PATCH] D21678: Fix For pr28288 - Error message in shift of vector values

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8597
@@ -8596,4 +8596,3 @@
 ///        by a scalar or vector shift amount.
-static QualType checkOpenCLVectorShift(Sema &S,
-                                       ExprResult &LHS, ExprResult &RHS,
-                                       SourceLocation Loc, bool IsCompAssign) {
+static QualType checkVectorShift(Sema &S, ExprResult &LHS, ExprResult &RHS,
+                                 SourceLocation Loc, bool IsCompAssign) {
----------------
Why does this drop the mention of OpenCL in the function name?

================
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8638
@@ -8638,4 +8637,3 @@
   if (RHSVecTy) {
-    // OpenCL v1.1 s6.3.j says that for vector types, the operators
-    // are applied component-wise. So if RHS is a vector, then ensure
-    // that the number of elements is the same as LHS...
+    // For vector types, the operators are applied component-wise. So if RHS is
+    // a vector, then ensure that the number of elements is the same as LHS...
----------------
It's good to keep language references in the comments, so I would leave the reference in even though this is being expanded for non-OpenCL vector types (unless the reference is incorrect).

================
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8681-8683
@@ -8680,5 +8675,3 @@
     }
-    return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign,
-                               /*AllowBothBool*/true,
-                               /*AllowBoolConversions*/false);
   }
----------------
Why is it correct to remove semantic checking for vector operands?

================
Comment at: llvm/tools/clang/test/Sema/shift.c:75
@@ +74,3 @@
+void
+vect_shift_1 (vec16 *x)
+{
----------------
Please clang-format the test case.


https://reviews.llvm.org/D21678