[clang] 044c51d - Fix vector type scalar checking when the scalar operand is dependent

[Erich Keane via cfe-commits]

Author: Erich Keane
Date: 2020-03-25T11:38:58-07:00
New Revision: 044c51d8d433da885438fcb141b15c80d7b62eda

URL: https://github.com/llvm/llvm-project/commit/044c51d8d433da885438fcb141b15c80d7b62eda
DIFF: https://github.com/llvm/llvm-project/commit/044c51d8d433da885438fcb141b15c80d7b62eda.diff

LOG: Fix vector type scalar checking when the scalar operand is dependent

As reported in PR45298 and PR45299, vector_size type checking would
crash when done in a situation where the scalar is dependent, such as
a member of the current instantiation.

This is because the scalar checking ensures that you can implicitly
convert a value to a vector-type as long as it doesn't require
truncation. It does this by using the constant evaluator to get the
value as a float. Unfortunately, if the scalar is dependent (such as a
member of the current instantiation), we would hit the assert in the
evaluator.

This patch suppresses the truncation- of-value check in the first phase
of translation. All values are properly errored upon instantiation. This
has one minor regression, in that previously in a non-asserts build,

template<typename T>
struct S {
  float4 f(float4 f) {
    return k + f;
  }
  static constexpr k = 1.1; // causes a truncation on conversion.
};

would error immediately. Because 'k' is value dependent (as a
member-of-the-current-instantiation), this would still be evaluatable
(despite normally asserting).  Due to this patch, this diagnostic is
delayed until instantiation time.

Added: 
    

Modified: 
    clang/lib/Sema/SemaExpr.cpp
    clang/test/SemaCXX/vector.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 7cb12ec9c405..8f4a7f240de1 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -9201,7 +9201,13 @@ static bool tryGCCVectorConvertAndSplat(Sema &S, ExprResult *Scalar,
       // Reject cases where the scalar type is not a constant and has a higher
       // Order than the vector element type.
       llvm::APFloat Result(0.0);
-      bool CstScalar = Scalar->get()->EvaluateAsFloat(Result, S.Context);
+
+      // Determine whether this is a constant scalar. In the event that the
+      // value is dependent (and thus cannot be evaluated by the constant
+      // evaluator), skip the evaluation. This will then diagnose once the
+      // expression is instantiated.
+      bool CstScalar = Scalar->get()->isValueDependent() ||
+                       Scalar->get()->EvaluateAsFloat(Result, S.Context);
       int Order = S.Context.getFloatingTypeOrder(VectorEltTy, ScalarTy);
       if (!CstScalar && Order < 0)
         return true;

diff  --git a/clang/test/SemaCXX/vector.cpp b/clang/test/SemaCXX/vector.cpp
index 67e5a94cb108..cabc525771c3 100644
--- a/clang/test/SemaCXX/vector.cpp
+++ b/clang/test/SemaCXX/vector.cpp
@@ -400,3 +400,77 @@ namespace swizzle_typo_correction {
     return A.xyzw < B.x && B.y > A.y; // OK, not a typo for 'xyzv'
   }
 }
+
+namespace PR45299 {
+typedef float float4 __attribute__((vector_size(16)));
+
+// In this example, 'k' is value dependent. PR45299 reported that this asserted
+// because of that, since the truncation check attempted to constant evaluate k,
+// which it could not do because it is dependent.
+template <typename T>
+struct NormalMember {
+  float4 f(float4 x) {
+    return k * x;
+  }
+  float k;
+};
+
+#if __cplusplus >= 201103L
+// This should not diagnose, since the constant evaluator (during instantiation)
+// can tell that this isn't a truncation.
+template <typename T>
+struct ConstantValueNoDiag {
+  float4 f(float4 x) {
+    return k * x;
+  }
+  static constexpr double k = 1;
+};
+
+// The following two both diagnose because they cause a truncation.  Test both
+// the dependent type and non-dependent type versions.
+template <typename T>
+struct DiagTrunc {
+  float4 f(float4 x) {
+    // expected-error at +1{{as implicit conversion would cause truncation}}
+    return k * x;
+  }
+  static constexpr double k = 1340282346638528859811704183484516925443.000000;
+};
+template <typename T>
+struct DiagTruncDependentType {
+  float4 f(float4 x) {
+    // expected-error at +1{{as implicit conversion would cause truncation}}
+    return k * x;
+  }
+  static constexpr T k = 1340282346638528859811704183484516925443.000000;
+};
+
+template <typename T>
+struct PR45298 {
+    T k1 = T(0);
+};
+
+// Ensure this no longer asserts.
+template <typename T>
+struct PR45298Consumer {
+  float4 f(float4 x) {
+    return (float)s.k1 * x;
+  }
+
+  PR45298<T> s;
+};
+#endif // __cplusplus >= 201103L
+
+void use() {
+  float4 theFloat4;
+  NormalMember<double>().f(theFloat4);
+#if __cplusplus >= 201103L
+  ConstantValueNoDiag<double>().f(theFloat4);
+  // expected-note at +1{{in instantiation of member function}}
+  DiagTrunc<double>().f(theFloat4);
+  // expected-note at +1{{in instantiation of member function}}
+  DiagTruncDependentType<double>().f(theFloat4);
+  PR45298Consumer<double>().f(theFloat4);
+#endif // __cplusplus >= 201103L
+}
+}