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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits