[PATCH] D25866: [Sema] Support implicit scalar to vector conversions

[Bruno Cardoso Lopes via cfe-commits]

bruno added inline comments.


================
Comment at: lib/Sema/SemaExpr.cpp:7978
+/// without causing truncation of Scalar.
+
+static bool tryGCCVectorConvertAndSpalt(Sema &S, ExprResult *Scalar,
----------------
Remove this empty line.


================
Comment at: lib/Sema/SemaExpr.cpp:7991
+      (!ScalarTy->isIntegralType(S.Context) &&
+       !ScalarTy->isRealFloatingType()))
+    return true;
----------------
This can be simplified by checking isArithmeticType() for each instead.


================
Comment at: lib/Sema/SemaExpr.cpp:8005
+    // type and then perform the rest of the checks here.
+    if (!ScalarTy->isIntegralType(S.Context))
+      return true;
----------------
You already checked this condition in the `if` above, this will never trigger.


================
Comment at: lib/Sema/SemaExpr.cpp:8043
+  } else if (VectorEltTy->isRealFloatingType()) {
+    if (ScalarTy->isRealFloatingType() && VectorEltTy != ScalarTy) {
+
----------------
I don't see how `VectorEltTy != ScalarTy` is possible here.


================
Comment at: lib/Sema/SemaExpr.cpp:8064
+      ScalarCast = CK_FloatingCast;
+    } else if (ScalarTy->isIntegralType(S.Context)) {
+      // Determine if the integer constant can be expressed as a floating point
----------------
I don't see why it's necessary to check for all specific cases where the scalar is a constant. For all the others scenarios it should be enough to get the right answer via `getIntegerTypeOrder` or `getFloatTypeOrder`. For this is specific condition, the `else` part for the `CstScalar` below should also handle the constant case, right? 




================
Comment at: lib/Sema/SemaExpr.cpp:8267
+  }
+
   // Otherwise, use the generic diagnostic.
----------------
This change seems orthogonal to this patch. Can you make it a separated patch with the changes from test/Sema/vector-cast.c?


================
Comment at: test/Sema/vector-gcc-compat.c:2
+// RUN: %clang_cc1 %s -verify -fsyntax-only -Weverything
+
+typedef long long v2i64 __attribute__((vector_size(16)));
----------------
These are really nice tests. Some cosmetic cleanups: can you run it through clang-format and also remove (a) newlines between comments and codes, (b) places with double newlines?


https://reviews.llvm.org/D25866