From 044c51d8d433da885438fcb141b15c80d7b62eda Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Wed, 25 Mar 2020 11:32:00 -0700 Subject: [PATCH] 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 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. --- clang/lib/Sema/SemaExpr.cpp | 8 ++++- clang/test/SemaCXX/vector.cpp | 74 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 7cb12ec..8f4a7f2 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 67e5a94..cabc525 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 +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 +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 +struct DiagTrunc { + float4 f(float4 x) { + // expected-error@+1{{as implicit conversion would cause truncation}} + return k * x; + } + static constexpr double k = 1340282346638528859811704183484516925443.000000; +}; +template +struct DiagTruncDependentType { + float4 f(float4 x) { + // expected-error@+1{{as implicit conversion would cause truncation}} + return k * x; + } + static constexpr T k = 1340282346638528859811704183484516925443.000000; +}; + +template +struct PR45298 { + T k1 = T(0); +}; + +// Ensure this no longer asserts. +template +struct PR45298Consumer { + float4 f(float4 x) { + return (float)s.k1 * x; + } + + PR45298 s; +}; +#endif // __cplusplus >= 201103L + +void use() { + float4 theFloat4; + NormalMember().f(theFloat4); +#if __cplusplus >= 201103L + ConstantValueNoDiag().f(theFloat4); + // expected-note@+1{{in instantiation of member function}} + DiagTrunc().f(theFloat4); + // expected-note@+1{{in instantiation of member function}} + DiagTruncDependentType().f(theFloat4); + PR45298Consumer().f(theFloat4); +#endif // __cplusplus >= 201103L +} +} -- 2.7.4