[PATCH] D98055: [ExtVectorType] Support conditional select operator for C++.

[Florian Hahn via Phabricator via cfe-commits]

fhahn added inline comments.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:5951
   const QualType EltTy =
-      cast<VectorType>(CondTy.getCanonicalType())->getElementType();
+      CondTy->isVectorType()
+          ? cast<VectorType>(CondTy.getCanonicalType())->getElementType()
----------------
erichkeane wrote:
> Why is this change necessary?  Doesn't ExtVectorType inherit from VectorType?  These should both be calling the same function here.
It's not really necessary, it turns out the template argument is not really needed and the whole function can be simplified a lot!


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:6012
     }
-    ResultType = Context.getVectorType(
-        ResultElementTy, CondType->castAs<VectorType>()->getNumElements(),
-        VectorType::GenericVector);
+    if (std::is_same<VectorTy, ExtVectorType>::value)
+      ResultType = Context.getExtVectorType(
----------------
erichkeane wrote:
> Oh boy, this is awkward as all heck...
> 
> Makes me think we should instead have a templated function Context.getVectorType<VectorType>() or <ExtVectorType>
That's gone now, it just checks for `ExtvectorType` now.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:6144
+    auto CondTy = Cond.get()->getType();
+    if (CondTy->isExtVectorType())
+      return CheckVectorConditionalTypes<ExtVectorType>(Cond, LHS, RHS,
----------------
erichkeane wrote:
> Again I wonder why the 'if' here is necessary?  Shouldn't just removing the 'error' diag in the call take care of this?
It's gone now. I also added a few additional test cases that mix vector_size/ext_vector_type vectors, which should be rejected I think (see `mix_vector_types` in `clang/test/SemaCXX/ext-vector-type-conditional.cpp`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98055