Add permittedLayoutFlags to checkModifiers
For now, just use this to prevent *any* layout qualifiers from appearing
on functions, or their parameters.
Bug: skia:11301
Change-Id: I05d8118c7121048c6ef49695a54e3714a8f8687e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376796
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
diff --git a/gn/sksl_tests.gni b/gn/sksl_tests.gni
index 88ba6f1..fefbb09 100644
--- a/gn/sksl_tests.gni
+++ b/gn/sksl_tests.gni
@@ -124,6 +124,7 @@
"/sksl/errors/InvalidOutParams.sksl",
"/sksl/errors/InvalidToken.sksl",
"/sksl/errors/InvalidUnary.sksl",
+ "/sksl/errors/LayoutInFunctions.sksl",
"/sksl/errors/LayoutMultiplePrimitiveTypes.sksl",
"/sksl/errors/LayoutRepeatedQualifiers.sksl",
"/sksl/errors/MismatchedNumbers.sksl",
diff --git a/resources/sksl/errors/LayoutInFunctions.sksl b/resources/sksl/errors/LayoutInFunctions.sksl
new file mode 100644
index 0000000..3b4aa9f
--- /dev/null
+++ b/resources/sksl/errors/LayoutInFunctions.sksl
@@ -0,0 +1,43 @@
+layout (
+ origin_upper_left,
+ override_coverage,
+ push_constant,
+ blend_support_all_equations,
+ tracked,
+ srgb_unpremul,
+ key,
+ location = 1,
+ offset = 1,
+ binding = 1,
+ index = 1,
+ set = 1,
+ builtin = 1,
+ input_attachment_index = 1,
+ max_vertices = 1,
+ invocations = 1,
+ marker = one,
+ when = one,
+ ctype = int)
+void on_return() {}
+
+void on_param(
+layout (
+ origin_upper_left,
+ override_coverage,
+ push_constant,
+ blend_support_all_equations,
+ tracked,
+ srgb_unpremul,
+ key,
+ location = 1,
+ offset = 1,
+ binding = 1,
+ index = 1,
+ set = 1,
+ builtin = 1,
+ input_attachment_index = 1,
+ max_vertices = 1,
+ invocations = 1,
+ marker = one,
+ when = one,
+ ctype = int) float x) {}
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index c6881fd..7534900 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -376,7 +376,8 @@
Modifiers::kFlat_Flag | Modifiers::kVarying_Flag |
Modifiers::kNoPerspective_Flag;
}
- this->checkModifiers(offset, modifiers, permitted);
+ // TODO(skbug.com/11301): Migrate above checks into building a mask of permitted layout flags
+ this->checkModifiers(offset, modifiers, permitted, /*permittedLayoutFlags=*/~0);
}
std::unique_ptr<Variable> IRGenerator::convertVar(int offset, const Modifiers& modifiers,
@@ -861,26 +862,63 @@
return ExpressionStatement::Make(fContext, std::move(result));
}
-void IRGenerator::checkModifiers(int offset, const Modifiers& modifiers, int permitted) {
+void IRGenerator::checkModifiers(int offset,
+ const Modifiers& modifiers,
+ int permittedModifierFlags,
+ int permittedLayoutFlags) {
int flags = modifiers.fFlags;
- #define CHECK(flag, name) \
- if (!flags) return; \
- if (flags & flag) { \
- if (!(permitted & flag)) { \
- this->errorReporter().error(offset, "'" name "' is not permitted here"); \
- } \
- flags &= ~flag; \
+ auto checkModifier = [&](Modifiers::Flag flag, const char* name) {
+ if (flags & flag) {
+ if (!(permittedModifierFlags & flag)) {
+ this->errorReporter().error(offset, "'" + String(name) + "' is not permitted here");
+ }
+ flags &= ~flag;
}
- CHECK(Modifiers::kConst_Flag, "const")
- CHECK(Modifiers::kIn_Flag, "in")
- CHECK(Modifiers::kOut_Flag, "out")
- CHECK(Modifiers::kUniform_Flag, "uniform")
- CHECK(Modifiers::kFlat_Flag, "flat")
- CHECK(Modifiers::kNoPerspective_Flag, "noperspective")
- CHECK(Modifiers::kHasSideEffects_Flag, "sk_has_side_effects")
- CHECK(Modifiers::kVarying_Flag, "varying")
- CHECK(Modifiers::kInline_Flag, "inline")
+ };
+
+ checkModifier(Modifiers::kConst_Flag, "const");
+ checkModifier(Modifiers::kIn_Flag, "in");
+ checkModifier(Modifiers::kOut_Flag, "out");
+ checkModifier(Modifiers::kUniform_Flag, "uniform");
+ checkModifier(Modifiers::kFlat_Flag, "flat");
+ checkModifier(Modifiers::kNoPerspective_Flag, "noperspective");
+ checkModifier(Modifiers::kHasSideEffects_Flag, "sk_has_side_effects");
+ checkModifier(Modifiers::kVarying_Flag, "varying");
+ checkModifier(Modifiers::kInline_Flag, "inline");
SkASSERT(flags == 0);
+
+ int layoutFlags = modifiers.fLayout.fFlags;
+ auto checkLayout = [&](Layout::Flag flag, const char* name) {
+ if (layoutFlags & flag) {
+ if (!(permittedLayoutFlags & flag)) {
+ this->errorReporter().error(
+ offset, "layout qualifier '" + String(name) + "' is not permitted here");
+ }
+ layoutFlags &= ~flag;
+ }
+ };
+
+ checkLayout(Layout::kOriginUpperLeft_Flag, "origin_upper_left");
+ checkLayout(Layout::kOverrideCoverage_Flag, "override_coverage");
+ checkLayout(Layout::kPushConstant_Flag, "push_constant");
+ checkLayout(Layout::kBlendSupportAllEquations_Flag, "blend_support_all_equations");
+ checkLayout(Layout::kTracked_Flag, "tracked");
+ checkLayout(Layout::kSRGBUnpremul_Flag, "srgb_unpremul");
+ checkLayout(Layout::kKey_Flag, "key");
+ checkLayout(Layout::kLocation_Flag, "location");
+ checkLayout(Layout::kOffset_Flag, "offset");
+ checkLayout(Layout::kBinding_Flag, "binding");
+ checkLayout(Layout::kIndex_Flag, "index");
+ checkLayout(Layout::kSet_Flag, "set");
+ checkLayout(Layout::kBuiltin_Flag, "builtin");
+ checkLayout(Layout::kInputAttachmentIndex_Flag, "input_attachment_index");
+ checkLayout(Layout::kPrimitive_Flag, "primitive-type");
+ checkLayout(Layout::kMaxVertices_Flag, "max_vertices");
+ checkLayout(Layout::kInvocations_Flag, "invocations");
+ checkLayout(Layout::kMarker_Flag, "marker");
+ checkLayout(Layout::kWhen_Flag, "when");
+ checkLayout(Layout::kCType_Flag, "ctype");
+ SkASSERT(layoutFlags == 0);
}
void IRGenerator::finalizeFunction(FunctionDefinition& f) {
@@ -1009,16 +1047,17 @@
return;
}
const ASTNode::FunctionData& funcData = f.getFunctionData();
- this->checkModifiers(f.fOffset, funcData.fModifiers, Modifiers::kHasSideEffects_Flag |
- Modifiers::kInline_Flag);
+ this->checkModifiers(f.fOffset, funcData.fModifiers,
+ Modifiers::kHasSideEffects_Flag | Modifiers::kInline_Flag,
+ /*permittedLayoutFlags=*/0);
std::vector<const Variable*> parameters;
for (size_t i = 0; i < funcData.fParameterCount; ++i) {
const ASTNode& param = *(iter++);
SkASSERT(param.fKind == ASTNode::Kind::kParameter);
ASTNode::ParameterData pd = param.getParameterData();
- this->checkModifiers(param.fOffset, pd.fModifiers, Modifiers::kConst_Flag |
- Modifiers::kIn_Flag |
- Modifiers::kOut_Flag);
+ this->checkModifiers(param.fOffset, pd.fModifiers,
+ Modifiers::kConst_Flag | Modifiers::kIn_Flag | Modifiers::kOut_Flag,
+ /*permittedLayoutFlags=*/0);
auto paramIter = param.begin();
const Type* type = this->convertType(*(paramIter++));
if (!type) {
diff --git a/src/sksl/SkSLIRGenerator.h b/src/sksl/SkSLIRGenerator.h
index ce1c6b3..06dcc59 100644
--- a/src/sksl/SkSLIRGenerator.h
+++ b/src/sksl/SkSLIRGenerator.h
@@ -150,8 +150,13 @@
*/
std::unique_ptr<ModifiersPool> releaseModifiers();
- void checkModifiers(int offset, const Modifiers& modifiers, int permitted);
- void checkVarDeclaration(int offset, const Modifiers& modifiers, const Type* baseType,
+ void checkModifiers(int offset,
+ const Modifiers& modifiers,
+ int permittedModifierFlags,
+ int permittedLayoutFlags);
+ void checkVarDeclaration(int offset,
+ const Modifiers& modifiers,
+ const Type* baseType,
Variable::Storage storage);
std::unique_ptr<Variable> convertVar(int offset, const Modifiers& modifiers,
const Type* baseType, StringFragment name, bool isArray,
diff --git a/tests/sksl/errors/LayoutInFunctions.glsl b/tests/sksl/errors/LayoutInFunctions.glsl
new file mode 100644
index 0000000..7a10dd3
--- /dev/null
+++ b/tests/sksl/errors/LayoutInFunctions.glsl
@@ -0,0 +1,41 @@
+### Compilation failed:
+
+error: 21: layout qualifier 'origin_upper_left' is not permitted here
+error: 21: layout qualifier 'override_coverage' is not permitted here
+error: 21: layout qualifier 'push_constant' is not permitted here
+error: 21: layout qualifier 'blend_support_all_equations' is not permitted here
+error: 21: layout qualifier 'tracked' is not permitted here
+error: 21: layout qualifier 'srgb_unpremul' is not permitted here
+error: 21: layout qualifier 'key' is not permitted here
+error: 21: layout qualifier 'location' is not permitted here
+error: 21: layout qualifier 'offset' is not permitted here
+error: 21: layout qualifier 'binding' is not permitted here
+error: 21: layout qualifier 'index' is not permitted here
+error: 21: layout qualifier 'set' is not permitted here
+error: 21: layout qualifier 'builtin' is not permitted here
+error: 21: layout qualifier 'input_attachment_index' is not permitted here
+error: 21: layout qualifier 'max_vertices' is not permitted here
+error: 21: layout qualifier 'invocations' is not permitted here
+error: 21: layout qualifier 'marker' is not permitted here
+error: 21: layout qualifier 'when' is not permitted here
+error: 21: layout qualifier 'ctype' is not permitted here
+error: 43: layout qualifier 'origin_upper_left' is not permitted here
+error: 43: layout qualifier 'override_coverage' is not permitted here
+error: 43: layout qualifier 'push_constant' is not permitted here
+error: 43: layout qualifier 'blend_support_all_equations' is not permitted here
+error: 43: layout qualifier 'tracked' is not permitted here
+error: 43: layout qualifier 'srgb_unpremul' is not permitted here
+error: 43: layout qualifier 'key' is not permitted here
+error: 43: layout qualifier 'location' is not permitted here
+error: 43: layout qualifier 'offset' is not permitted here
+error: 43: layout qualifier 'binding' is not permitted here
+error: 43: layout qualifier 'index' is not permitted here
+error: 43: layout qualifier 'set' is not permitted here
+error: 43: layout qualifier 'builtin' is not permitted here
+error: 43: layout qualifier 'input_attachment_index' is not permitted here
+error: 43: layout qualifier 'max_vertices' is not permitted here
+error: 43: layout qualifier 'invocations' is not permitted here
+error: 43: layout qualifier 'marker' is not permitted here
+error: 43: layout qualifier 'when' is not permitted here
+error: 43: layout qualifier 'ctype' is not permitted here
+38 errors
