Anastasia added inline comments.
================
Comment at: lib/Sema/SemaExprMember.cpp:287
+// OpenCL spec (Section 6.1.7 Vector Components):
+// The component group notation can occur on the left hand side of an
expression. To form an lvalue,
----------------
The format is normally: OpenCL v1.1, s6.1.7
================
Comment at: lib/Sema/SemaExprMember.cpp:288
+// OpenCL spec (Section 6.1.7 Vector Components):
+// The component group notation can occur on the left hand side of an
expression. To form an lvalue,
+// swizzling must be applied to an l-value of vector type, contain no
duplicate components,
----------------
Does this text have much relation to the function? I don't feel that copying it
from the spec is helping much in understanding of this function. It's probably
just Ok to have the reference to the spec section and concisely summarize that
the component swizzle length must be in accordance with the acceptable vector
sizes.
Also would it be better to rename this to something like:
IsValidOpenCLComponentSwizzleLength
================
Comment at: lib/Sema/SemaExprMember.cpp:389
+ if (S.getLangOpts().OpenCL && !HalvingSwizzle) {
+ compStr = CompName->getNameStart();
----------------
I think the logic in this function is all for OpenCL, but we didn't check it .
Anyways, let's leave it for now.
================
Comment at: lib/Sema/SemaExprMember.cpp:395
+
+ unsigned swizzleLength = StringRef(compStr).size();
+ if (IsValidSwizzleLength(swizzleLength) == false) {
----------------
Variable name doesn't adhere the coding style:
http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
Also you could use CompName->getLength() instead of constructing a new
StringRef object.
================
Comment at: test/SemaOpenCL/vector_swizzle_length.cl:8
+
+ f2.s01234; // expected-error {{vector component access has invalid length
5. Supported: 1,2,3,4,8,16}}
+}
----------------
Could we add a non-numeric swizzle too?
https://reviews.llvm.org/D30937
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
