Improved error reporting ranges for swizzles

Bug: skia:13171
Change-Id: I6dffb98ac2464f930995cf8ea57e422091d20fd2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/531743
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This commit is contained in:
Ethan Nicholas 2022-04-20 14:55:03 -04:00 committed by SkCQ
parent df76b7341f
commit 87e472722f
12 changed files with 98 additions and 63 deletions

View File

@ -214,25 +214,29 @@ DSLStatement While(DSLExpression test, DSLStatement stmt,
*/
DSLExpression Swizzle(DSLExpression base,
SkSL::SwizzleComponent::Type a,
Position pos = {});
Position pos = {},
Position maskPos = {});
DSLExpression Swizzle(DSLExpression base,
SkSL::SwizzleComponent::Type a,
SkSL::SwizzleComponent::Type b,
Position pos = {});
Position pos = {},
Position maskPos = {});
DSLExpression Swizzle(DSLExpression base,
SkSL::SwizzleComponent::Type a,
SkSL::SwizzleComponent::Type b,
SkSL::SwizzleComponent::Type c,
Position pos = {});
Position pos = {},
Position maskPos = {});
DSLExpression Swizzle(DSLExpression base,
SkSL::SwizzleComponent::Type a,
SkSL::SwizzleComponent::Type b,
SkSL::SwizzleComponent::Type c,
SkSL::SwizzleComponent::Type d,
Position pos = {});
Position pos = {},
Position maskPos = {});
/**
* Returns the absolute value of x. If x is a vector, operates componentwise.

View File

@ -1842,7 +1842,7 @@ DSLExpression DSLParser::postfixExpression() {
}
DSLExpression DSLParser::swizzle(Position pos, DSLExpression base,
std::string_view swizzleMask) {
std::string_view swizzleMask, Position maskPos) {
SkASSERT(swizzleMask.length() > 0);
if (!base.type().isVector() && !base.type().isScalar()) {
return base.field(swizzleMask, pos);
@ -1851,7 +1851,10 @@ DSLExpression DSLParser::swizzle(Position pos, DSLExpression base,
SkSL::SwizzleComponent::Type components[4];
for (int i = 0; i < length; ++i) {
if (i >= 4) {
this->error(pos, "too many components in swizzle mask");
Position errorPos = maskPos.valid() ? Position::Range(maskPos.startOffset() + 4,
maskPos.endOffset())
: pos;
this->error(errorPos, "too many components in swizzle mask");
return DSLExpression::Poison(pos);
}
switch (swizzleMask[i]) {
@ -1873,19 +1876,22 @@ DSLExpression DSLParser::swizzle(Position pos, DSLExpression base,
case 'w': components[i] = SwizzleComponent::W; break;
case 'q': components[i] = SwizzleComponent::Q; break;
case 'B': components[i] = SwizzleComponent::UB; break;
default:
this->error(pos, String::printf("invalid swizzle component '%c'",
default: {
Position componentPos = Position::Range(maskPos.startOffset() + i,
maskPos.startOffset() + i + 1);
this->error(componentPos, String::printf("invalid swizzle component '%c'",
swizzleMask[i]).c_str());
return DSLExpression::Poison(pos);
}
}
}
switch (length) {
case 1: return dsl::Swizzle(std::move(base), components[0], pos);
case 2: return dsl::Swizzle(std::move(base), components[0], components[1], pos);
case 1: return dsl::Swizzle(std::move(base), components[0], pos, maskPos);
case 2: return dsl::Swizzle(std::move(base), components[0], components[1], pos, maskPos);
case 3: return dsl::Swizzle(std::move(base), components[0], components[1], components[2],
pos);
pos, maskPos);
case 4: return dsl::Swizzle(std::move(base), components[0], components[1], components[2],
components[3], pos);
components[3], pos, maskPos);
default: SkUNREACHABLE;
}
}
@ -1919,7 +1925,8 @@ DSLExpression DSLParser::suffix(DSLExpression base) {
std::string_view text;
if (this->identifier(&text)) {
Position pos = this->rangeFrom(base.position());
return this->swizzle(pos, std::move(base), text);
return this->swizzle(pos, std::move(base), text,
this->rangeFrom(this->position(next).after()));
}
[[fallthrough]];
}
@ -1932,17 +1939,22 @@ DSLExpression DSLParser::suffix(DSLExpression base) {
// use the next *raw* token so we don't ignore whitespace - we only care about
// identifiers that directly follow the float
Position pos = this->rangeFrom(base.position());
Position start = this->position(next);
// skip past the "."
start = Position::Range(start.startOffset() + 1, start.endOffset());
Position maskPos = this->rangeFrom(start);
Token id = this->nextRawToken();
if (id.fKind == Token::Kind::TK_IDENTIFIER) {
pos = this->rangeFrom(base.position());
maskPos = this->rangeFrom(start);
return this->swizzle(pos, std::move(base), std::string(field) +
std::string(this->text(id)));
std::string(this->text(id)), maskPos);
} else if (field.empty()) {
this->error(pos, "expected field name or swizzle mask after '.'");
return {{DSLExpression::Poison(pos)}};
}
this->pushback(id);
return this->swizzle(pos, std::move(base), field);
return this->swizzle(pos, std::move(base), field, maskPos);
}
case Token::Kind::TK_LPAREN: {
ExpressionArray args;

View File

@ -271,7 +271,7 @@ private:
dsl::DSLExpression postfixExpression();
dsl::DSLExpression swizzle(Position pos, dsl::DSLExpression base,
std::string_view swizzleMask);
std::string_view swizzleMask, Position maskPos);
dsl::DSLExpression call(Position pos, dsl::DSLExpression base, ExpressionArray args);

View File

@ -315,18 +315,19 @@ public:
}
static DSLExpression Swizzle(DSLExpression base, SkSL::SwizzleComponent::Type a,
Position pos) {
return DSLExpression(Swizzle::Convert(ThreadContext::Context(), pos, base.release(),
ComponentArray{a}),
Position pos, Position maskPos) {
return DSLExpression(Swizzle::Convert(ThreadContext::Context(), pos, maskPos,
base.release(), ComponentArray{a}),
pos);
}
static DSLExpression Swizzle(DSLExpression base,
SkSL::SwizzleComponent::Type a,
SkSL::SwizzleComponent::Type b,
Position pos) {
return DSLExpression(Swizzle::Convert(ThreadContext::Context(), pos, base.release(),
ComponentArray{a, b}),
Position pos,
Position maskPos) {
return DSLExpression(Swizzle::Convert(ThreadContext::Context(), pos, maskPos,
base.release(), ComponentArray{a, b}),
pos);
}
@ -334,8 +335,9 @@ public:
SkSL::SwizzleComponent::Type a,
SkSL::SwizzleComponent::Type b,
SkSL::SwizzleComponent::Type c,
Position pos) {
return DSLExpression(Swizzle::Convert(ThreadContext::Context(), pos, base.release(),
Position pos,
Position maskPos) {
return DSLExpression(Swizzle::Convert(ThreadContext::Context(), pos, maskPos, base.release(),
ComponentArray{a, b, c}),
pos);
}
@ -345,8 +347,9 @@ public:
SkSL::SwizzleComponent::Type b,
SkSL::SwizzleComponent::Type c,
SkSL::SwizzleComponent::Type d,
Position pos) {
return DSLExpression(Swizzle::Convert(ThreadContext::Context(), pos, base.release(),
Position pos,
Position maskPos) {
return DSLExpression(Swizzle::Convert(ThreadContext::Context(), pos, maskPos, base.release(),
ComponentArray{a,b,c,d}),
pos);
}
@ -699,23 +702,25 @@ DSLExpression Step(DSLExpression edge, DSLExpression x, Position pos) {
}
DSLExpression Swizzle(DSLExpression base, SkSL::SwizzleComponent::Type a,
Position pos) {
return DSLCore::Swizzle(std::move(base), a, pos);
Position pos, Position maskPos) {
return DSLCore::Swizzle(std::move(base), a, pos, maskPos);
}
DSLExpression Swizzle(DSLExpression base,
SkSL::SwizzleComponent::Type a,
SkSL::SwizzleComponent::Type b,
Position pos) {
return DSLCore::Swizzle(std::move(base), a, b, pos);
Position pos,
Position maskPos) {
return DSLCore::Swizzle(std::move(base), a, b, pos, maskPos);
}
DSLExpression Swizzle(DSLExpression base,
SkSL::SwizzleComponent::Type a,
SkSL::SwizzleComponent::Type b,
SkSL::SwizzleComponent::Type c,
Position pos) {
return DSLCore::Swizzle(std::move(base), a, b, c, pos);
Position pos,
Position maskPos) {
return DSLCore::Swizzle(std::move(base), a, b, c, pos, maskPos);
}
DSLExpression Swizzle(DSLExpression base,
@ -723,8 +728,9 @@ DSLExpression Swizzle(DSLExpression base,
SkSL::SwizzleComponent::Type b,
SkSL::SwizzleComponent::Type c,
SkSL::SwizzleComponent::Type d,
Position pos) {
return DSLCore::Swizzle(std::move(base), a, b, c, d, pos);
Position pos,
Position maskPos) {
return DSLCore::Swizzle(std::move(base), a, b, c, d, pos, maskPos);
}
DSLExpression Tan(DSLExpression x, Position pos) {

View File

@ -235,10 +235,12 @@ static std::unique_ptr<Expression> optimize_constructor_swizzle(const Context& c
std::unique_ptr<Expression> Swizzle::Convert(const Context& context,
Position pos,
Position maskPos,
std::unique_ptr<Expression> base,
std::string_view maskString) {
ComponentArray components;
for (char field : maskString) {
for (size_t i = 0; i < maskString.length(); ++i) {
char field = maskString[i];
switch (field) {
case '0': components.push_back(SwizzleComponent::ZERO); break;
case '1': components.push_back(SwizzleComponent::ONE); break;
@ -259,12 +261,13 @@ std::unique_ptr<Expression> Swizzle::Convert(const Context& context,
case 'q': components.push_back(SwizzleComponent::Q); break;
case 'B': components.push_back(SwizzleComponent::UB); break;
default:
context.fErrors->error(pos,
context.fErrors->error(Position::Range(maskPos.startOffset() + i,
maskPos.startOffset() + i + 1),
String::printf("invalid swizzle component '%c'", field));
return nullptr;
}
}
return Convert(context, pos, std::move(base), std::move(components));
return Convert(context, pos, maskPos, std::move(base), std::move(components));
}
// Swizzles are complicated due to constant components. The most difficult case is a mask like
@ -274,10 +277,12 @@ std::unique_ptr<Expression> Swizzle::Convert(const Context& context,
// 'float4(base.xw, 1, 0).xzyw'.
std::unique_ptr<Expression> Swizzle::Convert(const Context& context,
Position pos,
Position rawMaskPos,
std::unique_ptr<Expression> base,
ComponentArray inComponents) {
Position maskPos = rawMaskPos.valid() ? rawMaskPos : pos;
if (!validate_swizzle_domain(inComponents)) {
context.fErrors->error(pos, "invalid swizzle mask '" + mask_string(inComponents) + "'");
context.fErrors->error(maskPos, "invalid swizzle mask '" + mask_string(inComponents) + "'");
return nullptr;
}
@ -290,7 +295,10 @@ std::unique_ptr<Expression> Swizzle::Convert(const Context& context,
}
if (inComponents.count() > 4) {
context.fErrors->error(pos,
Position errorPos = rawMaskPos.valid() ? Position::Range(maskPos.startOffset() + 4,
maskPos.endOffset())
: pos;
context.fErrors->error(errorPos,
"too many components in swizzle mask '" + mask_string(inComponents) + "'");
return nullptr;
}
@ -342,14 +350,17 @@ std::unique_ptr<Expression> Swizzle::Convert(const Context& context,
[[fallthrough]];
default:
// The swizzle component references a field that doesn't exist in the base type.
context.fErrors->error(pos, String::printf("invalid swizzle component '%c'",
mask_char(inComponents[i])));
context.fErrors->error(
Position::Range(maskPos.startOffset() + i,
maskPos.startOffset() + i + 1),
String::printf("invalid swizzle component '%c'",
mask_char(inComponents[i])));
return nullptr;
}
}
if (!foundXYZW) {
context.fErrors->error(pos, "swizzle must refer to base expression");
context.fErrors->error(maskPos, "swizzle must refer to base expression");
return nullptr;
}

View File

@ -36,11 +36,13 @@ struct Swizzle final : public Expression {
// swizzles (comprised solely of X/Y/W/Z).
static std::unique_ptr<Expression> Convert(const Context& context,
Position pos,
Position maskPos,
std::unique_ptr<Expression> base,
ComponentArray inComponents);
static std::unique_ptr<Expression> Convert(const Context& context,
Position pos,
Position maskPos,
std::unique_ptr<Expression> base,
std::string_view maskString);

View File

@ -11,7 +11,7 @@ void not_a_bvec() { S s; s.f = bool3(true); }
^^^^^^^^^^^^^^^^^
error: 6: invalid swizzle component 'm'
void not_a_struct() { S s; s.f.missing; }
^^^^^^^^^^^
^
error: 7: expected array, but found 'float'
void not_an_array() { S s; s.f[0]; }
^^^

View File

@ -5,7 +5,7 @@ void m(){ix;void[(0).r1(((5).ss0s.ss0s.sss0.ss0s+(5).ss0s.sss.00ss.ss0s.ss .ss0.
^^
error: 1: invalid swizzle component 'c'
void m(){ix;void[(0).r1(((5).ss0s.ss0s.sss0.ss0s+(5).ss0s.sss.00ss.ss0s.ss .ss0.ss00.ss0s+(5).ss0s.ss0.s0s.ss00.sssch (int) {case 0:{{{{{{{{{{{{{{{{{{{{{{{{e;void n(){;; int m;;half x;
^^^^^^^^^^^^^^^^^^^^^^^^^^^
^
error: 1: expected ')' to complete expression, but found '{'
void m(){ix;void[(0).r1(((5).ss0s.ss0s.sss0.ss0s+(5).ss0s.sss.00ss.ss0s.ss .ss0.ss00.ss0s+(5).ss0s.ss0.s0s.ss00.sssch (int) {case 0:{{{{{{{{{{{{{{{{{{{{{{{{e;void n(){;; int m;;half x;
^

View File

@ -2,50 +2,50 @@
error: 5: invalid swizzle mask 'xyra'
float4 xyra() { return v.xyra; }
^^^^^^
^^^^
error: 6: invalid swizzle mask 'zxtq'
float4 zxtq() { return v.zxtq; }
^^^^^^
^^^^
error: 7: invalid swizzle mask 'wwRB'
float4 wwRB() { return v.wwRB; }
^^^^^^
^^^^
error: 9: invalid swizzle mask 'rgxy'
float4 rgxy() { return v.rgxy; }
^^^^^^
^^^^
error: 10: invalid swizzle mask 'bast'
float4 bast() { return v.bast; }
^^^^^^
^^^^
error: 11: invalid swizzle mask 'gbLT'
float4 gbLT() { return v.gbLT; }
^^^^^^
^^^^
error: 13: invalid swizzle mask 'sxyz'
float4 sxyz() { return v.sxyz; }
^^^^^^
^^^^
error: 14: invalid swizzle mask 'tpbb'
float4 tpbb() { return v.tpbb; }
^^^^^^
^^^^
error: 15: invalid swizzle mask 'qsTR'
float4 qsTR() { return v.qsTR; }
^^^^^^
^^^^
error: 17: invalid swizzle mask 'LTxy'
float4 LTxy() { return v.LTxy; }
^^^^^^
^^^^
error: 18: invalid swizzle mask 'TRba'
float4 TRba() { return v.TRba; }
^^^^^^
^^^^
error: 19: invalid swizzle mask 'RBst'
float4 RBst() { return v.RBst; }
^^^^^^
^^^^
error: 21: invalid swizzle mask 'xrsL'
float4 xrsL() { return v.xrsL; }
^^^^^^
^^^^
error: 22: invalid swizzle mask 'ygtT'
float4 ygtT() { return v.ygtT; }
^^^^^^
^^^^
error: 23: invalid swizzle mask 'zbpR'
float4 zbpR() { return v.zbpR; }
^^^^^^
^^^^
error: 24: invalid swizzle mask 'waqB'
float4 waqB() { return v.waqB; }
^^^^^^
^^^^
16 errors

View File

@ -2,5 +2,5 @@
error: 1: swizzle must refer to base expression
void func() { float x = 1.0; x = x.0; }
^^^
^
1 error

View File

@ -2,5 +2,5 @@
error: 1: invalid swizzle component 'z'
void func() { float3 test = float2(1).xyz; }
^^^^^^^^^^^^^
^
1 error

View File

@ -2,5 +2,5 @@
error: 1: too many components in swizzle mask
void func() { float4 test = float2(1).xxxxx; }
^^^^^^^^^^^^^^^
^
1 error