Added position tracking for SkSL struct fields

Change-Id: Ibff49d1928d7f82d04930c8cfd9d574780732c0d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527497
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This commit is contained in:
Ethan Nicholas 2022-04-05 08:52:53 -04:00 committed by SkCQ
parent cedfb2b5a9
commit e0b6115a04
8 changed files with 45 additions and 31 deletions

View File

@ -660,6 +660,7 @@ std::optional<DSLType> DSLParser::structDeclaration() {
SkTArray<DSLField> fields;
std::unordered_set<std::string> field_names;
while (!this->checkNext(Token::Kind::TK_RBRACE)) {
Token fieldStart = this->peek();
DSLModifiers modifiers = this->modifiers();
std::optional<DSLType> type = this->type(&modifiers);
if (!type) {
@ -678,10 +679,11 @@ std::optional<DSLType> DSLParser::structDeclaration() {
if (!this->arraySize(&size)) {
return std::nullopt;
}
actualType = dsl::Array(actualType, size, this->position(memberName));
if (!this->expect(Token::Kind::TK_RBRACKET, "']'")) {
return std::nullopt;
}
actualType = dsl::Array(actualType, size,
this->rangeFrom(this->position(fieldStart)));
}
std::string key(this->text(memberName));
@ -690,11 +692,12 @@ std::optional<DSLType> DSLParser::structDeclaration() {
fields.push_back(DSLField(modifiers,
std::move(actualType),
this->text(memberName),
this->position(memberName)));
this->rangeFrom(fieldStart)));
field_names.emplace(key);
} else {
this->error(name, "field '" + key + "' was already defined in the same struct ('" +
std::string(this->text(name)) + "')");
this->error(memberName, "field '" + key +
"' was already defined in the same struct ('" + std::string(this->text(name)) +
"')");
}
} while (this->checkNext(Token::Kind::TK_COMMA));
if (!this->expect(Token::Kind::TK_SEMICOLON, "';'")) {

View File

@ -224,7 +224,7 @@ const Symbol* Rehydrator::symbol() {
Modifiers m = this->modifiers();
std::string_view fieldName = this->readString();
const Type* type = this->type();
fields.emplace_back(m, fieldName, type);
fields.emplace_back(Position(), m, fieldName, type);
}
bool interfaceBlock = this->readU8();
std::string_view nameChars(*fSymbolTable->takeOwnershipOfString(std::move(name)));

View File

@ -3063,7 +3063,8 @@ SpvId SPIRVCodeGenerator::writeInterfaceBlock(const InterfaceBlock& intf, bool a
// entirely new block when the variable is referenced. And we can't modify the existing
// block, so we instead create a modified copy of it and write that.
std::vector<Type::Field> fields = type.fields();
fields.emplace_back(Modifiers(Layout(/*flags=*/0,
fields.emplace_back(Position(),
Modifiers(Layout(/*flags=*/0,
/*location=*/-1,
fProgram.fConfig->fSettings.fRTFlipOffset,
/*binding=*/-1,
@ -3474,7 +3475,7 @@ void SPIRVCodeGenerator::writeUniformBuffer(std::shared_ptr<SymbolTable> topLeve
for (const VarDeclaration* topLevelUniform : fTopLevelUniforms) {
const Variable* var = &topLevelUniform->var();
fTopLevelUniformMap.set(var, (int)fields.size());
fields.emplace_back(var->modifiers(), var->name(), &var->type());
fields.emplace_back(var->fPosition, var->modifiers(), var->name(), &var->type());
}
fUniformBuffer.fStruct = Type::MakeStructType(Position(), kUniformBufferName, std::move(fields),
/*interfaceBlock=*/true);
@ -3510,7 +3511,8 @@ void SPIRVCodeGenerator::addRTFlipUniform(Position pos) {
if (fProgram.fConfig->fSettings.fRTFlipOffset < 0) {
fContext.fErrors->error(pos, "RTFlipOffset is negative");
}
fields.emplace_back(Modifiers(Layout(/*flags=*/0,
fields.emplace_back(pos,
Modifiers(Layout(/*flags=*/0,
/*location=*/-1,
fProgram.fConfig->fSettings.fRTFlipOffset,
/*binding=*/-1,

View File

@ -270,8 +270,8 @@ public:
field.fModifiers.fPosition, field.fModifiers.fModifiers, baseType,
Variable::Storage::kInterfaceBlock);
GetErrorReporter().reportPendingErrors(field.fPosition);
skslFields.push_back(SkSL::Type::Field(field.fModifiers.fModifiers, field.fName,
&field.fType.skslType()));
skslFields.push_back(SkSL::Type::Field(field.fPosition, field.fModifiers.fModifiers,
field.fName, &field.fType.skslType()));
}
const SkSL::Type* structType =
ThreadContext::SymbolTable()->takeOwnershipOfSymbol(SkSL::Type::MakeStructType(
@ -293,9 +293,8 @@ public:
if (varName.empty()) {
const std::vector<SkSL::Type::Field>& structFields = structType->fields();
for (size_t i = 0; i < structFields.size(); ++i) {
ThreadContext::SymbolTable()->add(std::make_unique<SkSL::Field>(pos,
skslVar,
i));
ThreadContext::SymbolTable()->add(std::make_unique<SkSL::Field>(
structFields[i].fPosition, skslVar, i));
}
} else {
AddToSymbolTable(var);

View File

@ -287,16 +287,16 @@ DSLType Struct(std::string_view name, SkSpan<DSLField> fields, Position pos) {
std::string desc = field.fModifiers.fModifiers.description();
desc.pop_back(); // remove trailing space
ThreadContext::ReportError("modifier '" + desc + "' is not permitted on a struct field",
field.fPosition);
field.fModifiers.fPosition);
}
if (field.fModifiers.fModifiers.fLayout.fFlags & Layout::kBinding_Flag) {
ThreadContext::ReportError(
"layout qualifier 'binding' is not permitted on a struct field",
field.fPosition);
field.fModifiers.fPosition);
}
if (field.fModifiers.fModifiers.fLayout.fFlags & Layout::kSet_Flag) {
ThreadContext::ReportError("layout qualifier 'set' is not permitted on a struct field",
field.fPosition);
field.fModifiers.fPosition);
}
const SkSL::Type& type = field.fType.skslType();
@ -306,7 +306,7 @@ DSLType Struct(std::string_view name, SkSpan<DSLField> fields, Position pos) {
ThreadContext::ReportError("opaque type '" + type.displayName() +
"' is not permitted in a struct", field.fPosition);
}
skslFields.emplace_back(field.fModifiers.fModifiers, field.fName, &type);
skslFields.emplace_back(field.fPosition, field.fModifiers.fModifiers, field.fName, &type);
}
const SkSL::Type* result = ThreadContext::SymbolTable()->add(Type::MakeStructType(pos, name,
skslFields));

View File

@ -60,8 +60,9 @@ public:
inline static constexpr int kMaxAbbrevLength = 3;
struct Field {
Field(Modifiers modifiers, std::string_view name, const Type* type)
: fModifiers(modifiers)
Field(Position pos, Modifiers modifiers, std::string_view name, const Type* type)
: fPosition(pos)
, fModifiers(modifiers)
, fName(name)
, fType(std::move(type)) {}
@ -69,6 +70,7 @@ public:
return fType->displayName() + " " + std::string(fName) + ";";
}
Position fPosition;
Modifiers fModifiers;
std::string_view fName;
const Type* fType;

View File

@ -66,19 +66,22 @@ DEF_TEST(SkSLMemoryLayout140Test, r) {
// struct 1
std::vector<SkSL::Type::Field> fields1;
fields1.emplace_back(SkSL::Modifiers(), std::string_view("a"), context.fTypes.fFloat3.get());
fields1.emplace_back(SkSL::Position(), SkSL::Modifiers(), std::string_view("a"),
context.fTypes.fFloat3.get());
std::unique_ptr<SkSL::Type> s1 = SkSL::Type::MakeStructType(SkSL::Position(),
std::string("s1"), fields1);
REPORTER_ASSERT(r, 16 == layout.size(*s1));
REPORTER_ASSERT(r, 16 == layout.alignment(*s1));
fields1.emplace_back(SkSL::Modifiers(), std::string_view("b"), context.fTypes.fFloat.get());
fields1.emplace_back(SkSL::Position(), SkSL::Modifiers(), std::string_view("b"),
context.fTypes.fFloat.get());
std::unique_ptr<SkSL::Type> s2 = SkSL::Type::MakeStructType(SkSL::Position(), std::string("s2"),
fields1);
REPORTER_ASSERT(r, 16 == layout.size(*s2));
REPORTER_ASSERT(r, 16 == layout.alignment(*s2));
fields1.emplace_back(SkSL::Modifiers(), std::string_view("c"), context.fTypes.fBool.get());
fields1.emplace_back(SkSL::Position(), SkSL::Modifiers(), std::string_view("c"),
context.fTypes.fBool.get());
std::unique_ptr<SkSL::Type> s3 = SkSL::Type::MakeStructType(SkSL::Position(), std::string("s3"),
fields1);
REPORTER_ASSERT(r, 32 == layout.size(*s3));
@ -86,13 +89,15 @@ DEF_TEST(SkSLMemoryLayout140Test, r) {
// struct 2
std::vector<SkSL::Type::Field> fields2;
fields2.emplace_back(SkSL::Modifiers(), std::string_view("a"), context.fTypes.fInt.get());
fields2.emplace_back(SkSL::Position(), SkSL::Modifiers(), std::string_view("a"),
context.fTypes.fInt.get());
std::unique_ptr<SkSL::Type> s4 = SkSL::Type::MakeStructType(SkSL::Position(), std::string("s4"),
fields2);
REPORTER_ASSERT(r, 16 == layout.size(*s4));
REPORTER_ASSERT(r, 16 == layout.alignment(*s4));
fields2.emplace_back(SkSL::Modifiers(), std::string_view("b"), context.fTypes.fFloat3.get());
fields2.emplace_back(SkSL::Position(), SkSL::Modifiers(), std::string_view("b"),
context.fTypes.fFloat3.get());
std::unique_ptr<SkSL::Type> s5 = SkSL::Type::MakeStructType(SkSL::Position(), std::string("s5"),
fields2);
REPORTER_ASSERT(r, 32 == layout.size(*s5));
@ -157,20 +162,22 @@ DEF_TEST(SkSLMemoryLayout430Test, r) {
// struct 1
std::vector<SkSL::Type::Field> fields1;
fields1.emplace_back(SkSL::Modifiers(), std::string_view("a"),
fields1.emplace_back(SkSL::Position(), SkSL::Modifiers(), std::string_view("a"),
context.fTypes.fFloat3.get());
std::unique_ptr<SkSL::Type> s1 = SkSL::Type::MakeStructType(SkSL::Position(), std::string("s1"),
fields1);
REPORTER_ASSERT(r, 16 == layout.size(*s1));
REPORTER_ASSERT(r, 16 == layout.alignment(*s1));
fields1.emplace_back(SkSL::Modifiers(), std::string_view("b"), context.fTypes.fFloat.get());
fields1.emplace_back(SkSL::Position(), SkSL::Modifiers(), std::string_view("b"),
context.fTypes.fFloat.get());
std::unique_ptr<SkSL::Type> s2 = SkSL::Type::MakeStructType(SkSL::Position(), std::string("s2"),
fields1);
REPORTER_ASSERT(r, 16 == layout.size(*s2));
REPORTER_ASSERT(r, 16 == layout.alignment(*s2));
fields1.emplace_back(SkSL::Modifiers(), std::string_view("c"), context.fTypes.fBool.get());
fields1.emplace_back(SkSL::Position(), SkSL::Modifiers(), std::string_view("c"),
context.fTypes.fBool.get());
std::unique_ptr<SkSL::Type> s3 = SkSL::Type::MakeStructType(SkSL::Position(), std::string("s3"),
fields1);
REPORTER_ASSERT(r, 32 == layout.size(*s3));
@ -178,13 +185,14 @@ DEF_TEST(SkSLMemoryLayout430Test, r) {
// struct 2
std::vector<SkSL::Type::Field> fields2;
fields2.emplace_back(SkSL::Modifiers(), std::string_view("a"), context.fTypes.fInt.get());
fields2.emplace_back(SkSL::Position(), SkSL::Modifiers(), std::string_view("a"),
context.fTypes.fInt.get());
std::unique_ptr<SkSL::Type> s4 = SkSL::Type::MakeStructType(SkSL::Position(), std::string("s4"),
fields2);
REPORTER_ASSERT(r, 4 == layout.size(*s4));
REPORTER_ASSERT(r, 4 == layout.alignment(*s4));
fields2.emplace_back(SkSL::Modifiers(), std::string_view("b"),
fields2.emplace_back(SkSL::Position(), SkSL::Modifiers(), std::string_view("b"),
context.fTypes.fFloat3.get());
std::unique_ptr<SkSL::Type> s5 = SkSL::Type::MakeStructType(SkSL::Position(), std::string("s5"),
fields2);

View File

@ -1,4 +1,4 @@
### Compilation failed:
error: 1: field 'var' was already defined in the same struct ('Varyings')
error: 3: field 'var' was already defined in the same struct ('Varyings')
1 error