[PATCH] D126540: PowerPC] Emit warning for incompatible vector types that are currently diagnosed with -fno-lax-vector-conversions

[Maryam Moghadas via Phabricator via cfe-commits]

maryammo added inline comments.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7715
 
+bool Sema::areAnyVectorTypesAltivec(QualType SrcTy, QualType DestTy) {
+  assert(DestTy->isVectorType() || SrcTy->isVectorType());
----------------
amyk wrote:
> Can we add some brief documentation for this function, like what is done for other functions in this file?
sure


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7716
+bool Sema::areAnyVectorTypesAltivec(QualType SrcTy, QualType DestTy) {
+  assert(DestTy->isVectorType() || SrcTy->isVectorType());
+
----------------
amyk wrote:
> Can we add a message to this `assert()`?
I followed what was done for other similar functions like : areMatrixTypesOfTheSameDimension


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7716
+bool Sema::areAnyVectorTypesAltivec(QualType SrcTy, QualType DestTy) {
+  assert(DestTy->isVectorType() || SrcTy->isVectorType());
+
----------------
maryammo wrote:
> amyk wrote:
> > Can we add a message to this `assert()`?
> I followed what was done for other similar functions like : areMatrixTypesOfTheSameDimension
same as others


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7722
+  if (SrcTy->isVectorType())
+  {
+    VectorType::VectorKind SrcVecKind = SrcTy->castAs<VectorType>()->getVectorKind();
----------------
amyk wrote:
> nit: Can we put braces on the same line? (And same goes for 7727)
sure


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7735
+
+bool Sema::areVectorTypesSameElmType(QualType SrcTy, QualType DestTy) {
+  assert(DestTy->isVectorType() || SrcTy->isVectorType());
----------------
amyk wrote:
> Can we add some brief documentation for this function, like what is done for other functions in this file?
sure


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7736
+bool Sema::areVectorTypesSameElmType(QualType SrcTy, QualType DestTy) {
+  assert(DestTy->isVectorType() || SrcTy->isVectorType());
+
----------------
amyk wrote:
> Can we add a message to this assert?
I followed what was done for other similar functions like : areMatrixTypesOfTheSameDimension


================
Comment at: clang/lib/Sema/SemaOverload.cpp:13027
   // end up here.
+  //
   return SemaRef.BuildCallExpr(/*Scope*/ nullptr, NewFn.get(), LParenLoc,
----------------
amyk wrote:
> amyk wrote:
> > Unnecessary change?
> Unnecessary change?
yes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126540