SkSL: Disallow '%' and '%=' on non-integral types
Bug: skia:11072
Change-Id: Ic24e40bfea5bf1d2d14c0f681632228a5ecc7104
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342929
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
diff --git a/gn/sksl_tests.gni b/gn/sksl_tests.gni
index d0ca64f..5982371 100644
--- a/gn/sksl_tests.gni
+++ b/gn/sksl_tests.gni
@@ -101,6 +101,7 @@
"$_tests/sksl/errors/EnumValueMustBeConstInt.sksl",
"$_tests/sksl/errors/ErrorsInDeadCode.sksl",
"$_tests/sksl/errors/FieldAfterRuntimeArray.sksl",
+ "$_tests/sksl/errors/FloatRemainder.sksl",
"$_tests/sksl/errors/ForTypeMismatch.sksl",
"$_tests/sksl/errors/GenericArgumentMismatch.sksl",
"$_tests/sksl/errors/IfTypeMismatch.sksl",
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 71bec1a..2e7ae56 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -1532,20 +1532,22 @@
}
/**
- * Defines the set of operators which perform bitwise math.
+ * Defines the set of operators which are only valid on integral types.
*/
-static bool op_is_bitwise(Token::Kind op) {
+static bool op_only_valid_for_integral_types(Token::Kind op) {
switch (op) {
case Token::Kind::TK_SHL:
case Token::Kind::TK_SHR:
case Token::Kind::TK_BITWISEAND:
case Token::Kind::TK_BITWISEOR:
case Token::Kind::TK_BITWISEXOR:
+ case Token::Kind::TK_PERCENT:
case Token::Kind::TK_SHLEQ:
case Token::Kind::TK_SHREQ:
case Token::Kind::TK_BITWISEANDEQ:
case Token::Kind::TK_BITWISEOREQ:
case Token::Kind::TK_BITWISEXOREQ:
+ case Token::Kind::TK_PERCENTEQ:
return true;
default:
return false;
@@ -1716,7 +1718,7 @@
: left.coercionCost(right);
if ((left.isScalar() && right.isScalar()) || (leftIsVectorOrMatrix && validMatrixOrVectorOp)) {
- if (op_is_bitwise(op)) {
+ if (op_only_valid_for_integral_types(op)) {
if (!leftComponentType.isInteger() || !rightComponentType.isInteger()) {
return false;
}
diff --git a/tests/SkSLInterpreterTest.cpp b/tests/SkSLInterpreterTest.cpp
index 9dc5884..7e23e53 100644
--- a/tests/SkSLInterpreterTest.cpp
+++ b/tests/SkSLInterpreterTest.cpp
@@ -218,10 +218,6 @@
}
DEF_TEST(SkSLInterpreterRemainder, r) {
- test(r, "void main(inout half4 color) { color.r = color.r % color.g; }", 3.125, 2, 0, 0,
- 1.125, 2, 0, 0);
- test(r, "void main(inout half4 color) { color %= half4(1, 2, 3, 4); }", 9.5, 9.5, 9.5, 9.5,
- 0.5, 1.5, 0.5, 1.5);
test(r, "void main(inout half4 color) { color.r = int(color.r) % int(color.g); }", 8, 3, 0, 0,
2, 3, 0, 0);
test(r, "void main(inout half4 color) { color.rg = half2(int2(int(color.r), int(color.g)) % "
diff --git a/tests/sksl/errors/FloatRemainder.sksl b/tests/sksl/errors/FloatRemainder.sksl
new file mode 100644
index 0000000..fa77d2b
--- /dev/null
+++ b/tests/sksl/errors/FloatRemainder.sksl
@@ -0,0 +1,7 @@
+// Expect 2 errors (one per function)
+
+float x;
+float y;
+
+void rem() { x = x % y; }
+void rem_eq() { x %= y; }
diff --git a/tests/sksl/errors/golden/FloatRemainder.glsl b/tests/sksl/errors/golden/FloatRemainder.glsl
new file mode 100644
index 0000000..1ae811e
--- /dev/null
+++ b/tests/sksl/errors/golden/FloatRemainder.glsl
@@ -0,0 +1,5 @@
+### Compilation failed:
+
+error: 6: type mismatch: '%' cannot operate on 'float', 'float'
+error: 7: type mismatch: '%=' cannot operate on 'float', 'float'
+2 errors
diff --git a/tests/sksl/fp/GrModuloOp.fp b/tests/sksl/fp/GrModuloOp.fp
index fe0602a..957fd07 100644
--- a/tests/sksl/fp/GrModuloOp.fp
+++ b/tests/sksl/fp/GrModuloOp.fp
@@ -1,3 +1,4 @@
+// Test that '%' is expanded to '%%' in emitCode
void main() {
- sk_OutColor.r = half(1 % sqrt(2));
+ sk_OutColor.r = half(1 % int(sqrt(2)));
}
diff --git a/tests/sksl/fp/golden/GrModuloOp.cpp b/tests/sksl/fp/golden/GrModuloOp.cpp
index 7af7b83..53aec43 100644
--- a/tests/sksl/fp/golden/GrModuloOp.cpp
+++ b/tests/sksl/fp/golden/GrModuloOp.cpp
@@ -20,7 +20,7 @@
const GrModuloOp& _outer = args.fFp.cast<GrModuloOp>();
(void) _outer;
fragBuilder->codeAppendf(
-R"SkSL(%s.x = half(1.0 %% sqrt(2.0));
+R"SkSL(%s.x = half(1 %% int(sqrt(2.0)));
)SkSL"
, args.fOutputColor);
}
