diff --git a/gn/sksl_tests.gni b/gn/sksl_tests.gni index ce930cc6c7..c88190b40b 100644 --- a/gn/sksl_tests.gni +++ b/gn/sksl_tests.gni @@ -749,6 +749,7 @@ sksl_rte_error_tests = [ "/sksl/runtime_errors/IllegalArrayOps.rts", "/sksl/runtime_errors/IllegalIndexing.rts", "/sksl/runtime_errors/IllegalLayoutFlags.rts", + "/sksl/runtime_errors/IllegalModifiers.rts", "/sksl/runtime_errors/IllegalOperators.rts", "/sksl/runtime_errors/IllegalPrecisionQualifiers.rts", "/sksl/runtime_errors/IllegalShaderSampling.rts", diff --git a/resources/sksl/errors/BadModifiers.sksl b/resources/sksl/errors/BadModifiers.sksl index 7884a4b27f..750a217ec5 100644 --- a/resources/sksl/errors/BadModifiers.sksl +++ b/resources/sksl/errors/BadModifiers.sksl @@ -6,6 +6,7 @@ void func2(const in out uniform flat noperspective sk_has_side_effects const in out uniform flat noperspective sk_has_side_effects inline noinline float var; /*%%* +'sk_has_side_effects' is not permitted here 'const' is not permitted here 'in' is not permitted here 'out' is not permitted here @@ -13,14 +14,15 @@ const in out uniform flat noperspective sk_has_side_effects inline noinline floa 'flat' is not permitted here 'noperspective' is not permitted here functions cannot be both 'inline' and 'noinline' +'sk_has_side_effects' is not permitted here 'uniform' is not permitted here 'flat' is not permitted here 'noperspective' is not permitted here 'sk_has_side_effects' is not permitted here 'inline' is not permitted here 'noinline' is not permitted here -'in uniform' variables not permitted 'sk_has_side_effects' is not permitted here +'in uniform' variables not permitted 'inline' is not permitted here 'noinline' is not permitted here 'const' variables must be initialized diff --git a/resources/sksl/runtime_errors/IllegalModifiers.rts b/resources/sksl/runtime_errors/IllegalModifiers.rts new file mode 100644 index 0000000000..46f617528c --- /dev/null +++ b/resources/sksl/runtime_errors/IllegalModifiers.rts @@ -0,0 +1,40 @@ +flat float _flat; +noperspective float _noperspective; +in float _in; +out float _out; +threadgroup float _threadgroup; +$es3 float _es3; +sk_has_side_effects float _sk_has_side_effects; +inline float _inline; +noinline float _noinline; + +flat void flat_fn() {} +noperspective void noperspective_fn() {} +in void in_fn() {} +out void out_fn() {} +threadgroup void threadgroup_fn() {} +$es3 void es3_fn() {} +sk_has_side_effects void sk_has_side_effect_fn() {} + +float4 main(float2 xy) { + return float4(1); +} + +/*%%* +'flat' is not permitted here +'noperspective' is not permitted here +'in' is not permitted here +'out' is not permitted here +'threadgroup' is not permitted here +'$es3' is not permitted here +'sk_has_side_effects' is not permitted here +'inline' is not permitted here +'noinline' is not permitted here +'flat' is not permitted here +'noperspective' is not permitted here +'in' is not permitted here +'out' is not permitted here +'threadgroup' is not permitted here +'$es3' is not permitted here +'sk_has_side_effects' is not permitted here +*%%*/ diff --git a/resources/sksl/runtime_errors/IllegalShaderUse.rts b/resources/sksl/runtime_errors/IllegalShaderUse.rts index 5cd71c87b2..d867adba97 100644 --- a/resources/sksl/runtime_errors/IllegalShaderUse.rts +++ b/resources/sksl/runtime_errors/IllegalShaderUse.rts @@ -33,8 +33,8 @@ half4 dangling_eval() { s1.eval; } /*%%* variables of type 'shader' must be uniform -'in' variables not permitted in runtime effects variables of type 'shader' must be uniform +'in' is not permitted here opaque type 'shader' is not permitted in a struct variables of type 'S' may not be uniform opaque type 'shader' may not be used in an array diff --git a/src/sksl/SkSLDSLParser.cpp b/src/sksl/SkSLDSLParser.cpp index a1d196728b..fa31ed232c 100644 --- a/src/sksl/SkSLDSLParser.cpp +++ b/src/sksl/SkSLDSLParser.cpp @@ -940,8 +940,14 @@ DSLModifiers DSLParser::modifiers() { if (!tokenFlag) { break; } + Token modifier = this->nextToken(); + // We have to check for this (internal) modifier here. It's automatically added to user + // functions before the IR is built, so testing for it in Convert gives false positives. + if (tokenFlag == Modifiers::kHasSideEffects_Flag && !ThreadContext::IsModule()) { + this->error(modifier, "'sk_has_side_effects' is not permitted here"); + } flags |= tokenFlag; - end = this->position(this->nextToken()).endOffset(); + end = this->position(modifier).endOffset(); } return DSLModifiers(std::move(layout), flags, Position::Range(start, end)); } diff --git a/src/sksl/ir/SkSLVarDeclarations.cpp b/src/sksl/ir/SkSLVarDeclarations.cpp index 088649d408..f10f1e369d 100644 --- a/src/sksl/ir/SkSLVarDeclarations.cpp +++ b/src/sksl/ir/SkSLVarDeclarations.cpp @@ -179,11 +179,6 @@ void VarDeclaration::ErrorCheck(const Context& context, if ((modifiers.fFlags & Modifiers::kUniform_Flag)) { check_valid_uniform_type(pos, baseType, context); } - if (ProgramConfig::IsRuntimeEffect(context.fConfig->fKind)) { - if (modifiers.fFlags & Modifiers::kIn_Flag) { - context.fErrors->error(pos, "'in' variables not permitted in runtime effects"); - } - } if (baseType->isEffectChild() && !(modifiers.fFlags & Modifiers::kUniform_Flag)) { context.fErrors->error(pos, "variables of type '" + baseType->displayName() + "' must be uniform"); @@ -213,14 +208,25 @@ void VarDeclaration::ErrorCheck(const Context& context, int permitted = Modifiers::kConst_Flag | Modifiers::kHighp_Flag | Modifiers::kMediump_Flag | Modifiers::kLowp_Flag; if (storage == Variable::Storage::kGlobal) { - permitted |= Modifiers::kIn_Flag | Modifiers::kOut_Flag; if (!ProgramConfig::IsCompute(context.fConfig->fKind)) { - permitted |= Modifiers::kUniform_Flag | Modifiers::kFlat_Flag | - Modifiers::kNoPerspective_Flag; - } else if (!baseType->isOpaque()) { - permitted |= Modifiers::kThreadgroup_Flag; + permitted |= Modifiers::kUniform_Flag; + } + + // No other modifiers are allowed in runtime effects + if (!ProgramConfig::IsRuntimeEffect(context.fConfig->fKind)) { + permitted |= Modifiers::kIn_Flag | Modifiers::kOut_Flag; + if (!ProgramConfig::IsCompute(context.fConfig->fKind)) { + permitted |= Modifiers::kFlat_Flag | Modifiers::kNoPerspective_Flag; + } else if (!baseType->isOpaque()) { + permitted |= Modifiers::kThreadgroup_Flag; + } } } + // This modifier isn't actually allowed on variables, at all. However, it's restricted to only + // appear in module code by the parser. We "allow" it here, to avoid double-reporting errors. + // This means that module code could put it on a variable (to no effect). We'll live with that. + permitted |= Modifiers::kHasSideEffects_Flag; + // TODO(skbug.com/11301): Migrate above checks into building a mask of permitted layout flags int permittedLayoutFlags = ~0; diff --git a/tests/sksl/errors/BadModifiers.glsl b/tests/sksl/errors/BadModifiers.glsl index 8c0db2fd5e..16da716d3b 100644 --- a/tests/sksl/errors/BadModifiers.glsl +++ b/tests/sksl/errors/BadModifiers.glsl @@ -1,5 +1,8 @@ ### Compilation failed: +error: 1: 'sk_has_side_effects' is not permitted here +const in out uniform flat noperspective sk_has_side_effects inline noinline void func1() {} + ^^^^^^^^^^^^^^^^^^^ error: 1: 'const' is not permitted here const in out uniform flat noperspective sk_has_side_effects inline noinline void func1() {} ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -21,6 +24,9 @@ const in out uniform flat noperspective sk_has_side_effects inline noinline void error: 1: functions cannot be both 'inline' and 'noinline' const in out uniform flat noperspective sk_has_side_effects inline noinline void func1() {} ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +error: 3: 'sk_has_side_effects' is not permitted here +void func2(const in out uniform flat noperspective sk_has_side_effects + ^^^^^^^^^^^^^^^^^^^ error: 3: 'uniform' is not permitted here void func2(const in out uniform flat noperspective sk_has_side_effects ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^... @@ -39,12 +45,12 @@ void func2(const in out uniform flat noperspective sk_has_side_effects error: 3: 'noinline' is not permitted here void func2(const in out uniform flat noperspective sk_has_side_effects ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^... +error: 6: 'sk_has_side_effects' is not permitted here +const in out uniform flat noperspective sk_has_side_effects inline noinline float var; + ^^^^^^^^^^^^^^^^^^^ error: 6: 'in uniform' variables not permitted const in out uniform flat noperspective sk_has_side_effects inline noinline float var; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: 6: 'sk_has_side_effects' is not permitted here -const in out uniform flat noperspective sk_has_side_effects inline noinline float var; -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: 6: 'inline' is not permitted here const in out uniform flat noperspective sk_has_side_effects inline noinline float var; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -54,4 +60,4 @@ const in out uniform flat noperspective sk_has_side_effects inline noinline floa error: 6: 'const' variables must be initialized const in out uniform flat noperspective sk_has_side_effects inline noinline float var; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -18 errors +20 errors diff --git a/tests/sksl/runtime_errors/IllegalModifiers.skvm b/tests/sksl/runtime_errors/IllegalModifiers.skvm new file mode 100644 index 0000000000..3317f095e7 --- /dev/null +++ b/tests/sksl/runtime_errors/IllegalModifiers.skvm @@ -0,0 +1,51 @@ +### Compilation failed: + +error: 1: 'flat' is not permitted here +flat float _flat; +^^^^ +error: 2: 'noperspective' is not permitted here +noperspective float _noperspective; +^^^^^^^^^^^^^ +error: 3: 'in' is not permitted here +in float _in; +^^ +error: 4: 'out' is not permitted here +out float _out; +^^^ +error: 5: 'threadgroup' is not permitted here +threadgroup float _threadgroup; +^^^^^^^^^^^ +error: 6: '$es3' is not permitted here +$es3 float _es3; +^^^^ +error: 7: 'sk_has_side_effects' is not permitted here +sk_has_side_effects float _sk_has_side_effects; +^^^^^^^^^^^^^^^^^^^ +error: 8: 'inline' is not permitted here +inline float _inline; +^^^^^^ +error: 9: 'noinline' is not permitted here +noinline float _noinline; +^^^^^^^^ +error: 11: 'flat' is not permitted here +flat void flat_fn() {} +^^^^ +error: 12: 'noperspective' is not permitted here +noperspective void noperspective_fn() {} +^^^^^^^^^^^^^ +error: 13: 'in' is not permitted here +in void in_fn() {} +^^ +error: 14: 'out' is not permitted here +out void out_fn() {} +^^^ +error: 15: 'threadgroup' is not permitted here +threadgroup void threadgroup_fn() {} +^^^^^^^^^^^ +error: 16: '$es3' is not permitted here +$es3 void es3_fn() {} +^^^^ +error: 17: 'sk_has_side_effects' is not permitted here +sk_has_side_effects void sk_has_side_effect_fn() {} +^^^^^^^^^^^^^^^^^^^ +16 errors diff --git a/tests/sksl/runtime_errors/IllegalShaderUse.skvm b/tests/sksl/runtime_errors/IllegalShaderUse.skvm index 661da863fd..a871a95349 100644 --- a/tests/sksl/runtime_errors/IllegalShaderUse.skvm +++ b/tests/sksl/runtime_errors/IllegalShaderUse.skvm @@ -3,12 +3,12 @@ error: 10: variables of type 'shader' must be uniform shader s3; ^^^^^^^^^ -error: 11: 'in' variables not permitted in runtime effects -in shader s4; -^^^^^^^^^^^^ error: 11: variables of type 'shader' must be uniform in shader s4; ^^^^^^^^^^^^ +error: 11: 'in' is not permitted here +in shader s4; +^^ error: 13: opaque type 'shader' is not permitted in a struct struct S { shader sh; }; ^^^^^^^^^