Disallow assignment to unfoldable ternaries in SkSL.
GLSL does not support assigning to ternaries, and will fail to compile
and/or generate non-functional shaders if we pass in a shader that tries
to assign into a ternary expression.
If SkSL is able to completely eliminate the ternary (e.g. if it boils
down to a simple `true ? x : y` or `false ? x : y`), SkSL can strip out
the ternary entirely and generate valid GLSL. This case is harmless and
so it is still allowed.
Change-Id: I960f119fb9934f998697634e6c4e519cd77d3780
Bug: skia:10767
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319679
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
diff --git a/gn/sksl_tests.gni b/gn/sksl_tests.gni
index e3b75e5..0f81684 100644
--- a/gn/sksl_tests.gni
+++ b/gn/sksl_tests.gni
@@ -219,7 +219,6 @@
"$_tests/sksl/shared/SwizzleScalar.sksl",
"$_tests/sksl/shared/TernaryAsLValueEntirelyFoldable.sksl",
"$_tests/sksl/shared/TernaryAsLValueFoldableTest.sksl",
- "$_tests/sksl/shared/TernaryAsLValueUnfoldable.sksl",
"$_tests/sksl/shared/Texture.sksl",
"$_tests/sksl/shared/TextureSharpen.sksl",
"$_tests/sksl/shared/UnaryPositiveNegative.sksl",
diff --git a/src/sksl/SkSLAnalysis.cpp b/src/sksl/SkSLAnalysis.cpp
index ef6ed59..26e5999 100644
--- a/src/sksl/SkSLAnalysis.cpp
+++ b/src/sksl/SkSLAnalysis.cpp
@@ -249,12 +249,6 @@
this->visitExpression(*expr.as<IndexExpression>().fBase);
break;
- case Expression::Kind::kTernary: {
- const TernaryExpression& ternary = expr.as<TernaryExpression>();
- this->visitExpression(*ternary.fIfTrue);
- this->visitExpression(*ternary.fIfFalse);
- break;
- }
case Expression::Kind::kExternalValue: {
const ExternalValue* var = expr.as<ExternalValueReference>().fValue;
if (!var->canWrite()) {
diff --git a/tests/sksl/errors/InvalidAssignment.sksl b/tests/sksl/errors/InvalidAssignment.sksl
index 8e95fa7..fb778af 100644
--- a/tests/sksl/errors/InvalidAssignment.sksl
+++ b/tests/sksl/errors/InvalidAssignment.sksl
@@ -4,16 +4,16 @@
uniform int u;
-void assign_to_literal() { 1 = 2; }
-void assign_to_uniform() { u = 0; }
-void assign_to_const() { const int x; x = 0; }
-void assign_to_const_array() { const int x[1]; x[0] = 0; }
-void assign_to_const_swizzle() { const half4 x; x.w = 0; }
-void assign_to_repeated_swizzle() { half4 x; x.yy = half2(0); }
-void assign_to_const_struct() { const S s; s.f = 0; }
-void assign_to_ternary() { float l; float r; (true ? l : r) = 0; } // TODO(skbug.com/10767)
-void assign_to_ternary_const_left() { const float l; float r; (true ? l : r) = 0; }
-void assign_to_ternary_const_right() { float l; const float r; (false ? l : r) = 0; }
-void assign_to_ternary_const_both() { const float l; const float r; (true ? l : r) = 0; }
-void assign_to_unary_minus() { float x; -x = 0; }
-void assign_to_unary_plus() { float x; +x = 0; } // TODO(skbug.com/10766)
+void assign_to_literal() { 1 = 2; }
+void assign_to_uniform() { u = 0; }
+void assign_to_const() { const int x; x = 0; }
+void assign_to_const_array() { const int x[1]; x[0] = 0; }
+void assign_to_const_swizzle() { const half4 x; x.w = 0; }
+void assign_to_repeated_swizzle() { half4 x; x.yy = half2(0); }
+void assign_to_const_struct() { const S s; s.f = 0; }
+void assign_to_foldable_ternary_const_left() { const float l; float r; (true ? l : r) = 0; }
+void assign_to_foldable_ternary_const_right() { float l; const float r; (false ? l : r) = 0; }
+void assign_to_foldable_ternary_const_both() { const float l; const float r; (true ? l : r) = 0; }
+void assign_to_unfoldable_ternary() { float l, r; (sqrt(1) > 0 ? l : r) = 0; }
+void assign_to_unary_minus() { float x; -x = 0; }
+void assign_to_unary_plus() { float x; +x = 0; } // TODO(skbug.com/10766)
diff --git a/tests/sksl/errors/golden/InvalidAssignment.glsl b/tests/sksl/errors/golden/InvalidAssignment.glsl
index 1bb4da4..7ccc04b 100644
--- a/tests/sksl/errors/golden/InvalidAssignment.glsl
+++ b/tests/sksl/errors/golden/InvalidAssignment.glsl
@@ -7,8 +7,9 @@
error: 11: cannot modify immutable variable 'x'
error: 12: cannot write to the same swizzle field more than once
error: 13: cannot modify immutable variable 's'
-error: 15: cannot modify immutable variable 'l'
-error: 16: cannot modify immutable variable 'r'
-error: 17: cannot modify immutable variable 'l'
+error: 14: cannot modify immutable variable 'l'
+error: 15: cannot modify immutable variable 'r'
+error: 16: cannot modify immutable variable 'l'
+error: 17: cannot assign to this expression
error: 18: cannot assign to this expression
-11 errors
+12 errors
diff --git a/tests/sksl/shared/TernaryAsLValueUnfoldable.sksl b/tests/sksl/shared/TernaryAsLValueUnfoldable.sksl
deleted file mode 100644
index 45df8a6..0000000
--- a/tests/sksl/shared/TernaryAsLValueUnfoldable.sksl
+++ /dev/null
@@ -1,6 +0,0 @@
-void main() {
- half r, g;
- (sqrt(1) > 0 ? r : g) = half(sqrt(1));
- (sqrt(0) > 0 ? r : g) = half(sqrt(0));
- sk_FragColor = half4(r, g, 1, 1);
-}
diff --git a/tests/sksl/shared/golden/TernaryAsLValueUnfoldable.glsl b/tests/sksl/shared/golden/TernaryAsLValueUnfoldable.glsl
deleted file mode 100644
index 42deaba..0000000
--- a/tests/sksl/shared/golden/TernaryAsLValueUnfoldable.glsl
+++ /dev/null
@@ -1,8 +0,0 @@
-
-out vec4 sk_FragColor;
-void main() {
- float r, g;
- sqrt(1.0) > 0.0 ? r : g = sqrt(1.0);
- sqrt(0.0) > 0.0 ? r : g = sqrt(0.0);
- sk_FragColor = vec4(r, g, 1.0, 1.0);
-}
diff --git a/tests/sksl/shared/golden/TernaryAsLValueUnfoldable.metal b/tests/sksl/shared/golden/TernaryAsLValueUnfoldable.metal
deleted file mode 100644
index 6bf289f..0000000
--- a/tests/sksl/shared/golden/TernaryAsLValueUnfoldable.metal
+++ /dev/null
@@ -1,17 +0,0 @@
-#include <metal_stdlib>
-#include <simd/simd.h>
-using namespace metal;
-struct Inputs {
-};
-struct Outputs {
- float4 sk_FragColor [[color(0)]];
-};
-fragment Outputs fragmentMain(Inputs _in [[stage_in]], bool _frontFacing [[front_facing]], float4 _fragCoord [[position]]) {
- Outputs _outputStruct;
- thread Outputs* _out = &_outputStruct;
- float r, g;
- sqrt(1.0) > 0.0 ? r : g = sqrt(1.0);
- sqrt(0.0) > 0.0 ? r : g = sqrt(0.0);
- _out->sk_FragColor = float4(r, g, 1.0, 1.0);
- return *_out;
-}
