[PATCH] D101519: [C++4OpenCL] Fix reinterpret_cast of vectors

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: clang/lib/Sema/SemaCast.cpp:2333
+      if (DestType->isExtVectorType() || SrcType->isExtVectorType()) {
+        // FIXME: Allow for reinterpret cast between 3 and 4 element vectors
+        if (Self.areVectorTypesSameSize(SrcType, DestType)) {
----------------
Just a small nit - add `.` at the end.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7347
 
+bool Sema::areVectorTypesSameSize(QualType srcTy, QualType destTy) {
+  assert(destTy->isVectorType() || srcTy->isVectorType());
----------------
Btw I understand that this function is copied over from the old code that has wrong formatting but since we are rewriting this now let's convert it to the current style:
https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

In partucular variable name should start from the upper case letter. 


================
Comment at: clang/test/SemaOpenCLCXX/reinterpret-cast.clcpp:14
+
+  reserve_id_t r_id1;
+  auto r_id2 = reinterpret_cast<reserve_id_t>(r_id1); // expected-error{{reinterpret_cast from 'reserve_id_t' to 'reserve_id_t' is not allowed}}
----------------
Let's add a comment here that it is testing casting OpenCL types to itself.

Btw I assume casting vector to the same vector type will work? Maybe we can add a test case for this too?

At the top of the function we can add a comment about teting the vector type.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101519/new/

https://reviews.llvm.org/D101519