AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fixed remaining SkSL positions

Browse Source 
This addresses (hopefully) all of the remaining suboptimal positions in
SkSL error reporting.

Change-Id: I5bc977b03d51153b841a89fa687e54e3e9cb6ec3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527976
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This commit is contained in:
Ethan Nicholas 2022-04-06 11:33:00 -04:00 committed by SkCQ
parent b69a292dca
commit a98e077508
17 changed files with 59 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
include/sksl/SkSLPosition.h
View File

@ -73,6 +73,11 @@ public:
return Range(this->startOffset(), end.endOffset());
}
// Returns a position representing the character immediately after this position
Position after() const {
return Range(fEndOffset, fEndOffset + 1);
}
bool operator==(const Position& other) const {
return fStartOffsetOrLine == other.fStartOffsetOrLine &&
fEndOffset == other.fEndOffset;

8
src/sksl/analysis/SkSLFinalizationChecks.cpp
View File

@ -81,7 +81,7 @@ public:
if (!param->type().isStruct() && paramInout == Modifiers::Flag::kOut_Flag) {
ProgramUsage::VariableCounts counts = fUsage.get(*param);
if (counts.fWrite <= 0) {
fContext.fErrors->error(funcDecl.fPosition,
fContext.fErrors->error(funcDef.body()->fPosition,
"function '" + std::string(funcDecl.name()) +
"' never assigns a value to out parameter '" +
std::string(param->name()) + "'");
@ -94,13 +94,15 @@ public:
switch (stmt.kind()) {
case Statement::Kind::kIf:
if (stmt.as<IfStatement>().isStatic()) {
fContext.fErrors->error(stmt.fPosition, "static if has non-static test");
fContext.fErrors->error(stmt.as<IfStatement>().test()->fPosition,
"static if has non-static test");
}
break;
case Statement::Kind::kSwitch:
if (stmt.as<SwitchStatement>().isStatic()) {
fContext.fErrors->error(stmt.fPosition, "static switch has non-static test");
fContext.fErrors->error(stmt.as<SwitchStatement>().value()->fPosition,
"static switch has non-static test");
}
break;

1
src/sksl/dsl/BUILD.bazel
View File

@ -118,6 +118,7 @@ generated_cc_atom(
"//src/sksl:SkSLProgramSettings_hdr",
"//src/sksl:SkSLThreadContext_hdr",
"//src/sksl/dsl/priv:DSLWriter_hdr",
"//src/sksl/ir:SkSLBlock_hdr",
"//src/sksl/ir:SkSLExpression_hdr",
"//src/sksl/ir:SkSLFunctionCall_hdr",
"//src/sksl/ir:SkSLFunctionDeclaration_hdr",

4
src/sksl/dsl/DSLFunction.cpp
View File

@ -17,6 +17,7 @@
#include "src/sksl/SkSLProgramSettings.h"
#include "src/sksl/SkSLThreadContext.h"
#include "src/sksl/dsl/priv/DSLWriter.h"
#include "src/sksl/ir/SkSLBlock.h"
#include "src/sksl/ir/SkSLExpression.h"
#include "src/sksl/ir/SkSLFunctionCall.h"
#include "src/sksl/ir/SkSLFunctionDeclaration.h"
@ -30,8 +31,6 @@
namespace SkSL {
class Block;
namespace dsl {
void DSLFunction::init(DSLModifiers modifiers, const DSLType& returnType, std::string_view name,
@ -88,6 +87,7 @@ void DSLFunction::init(DSLModifiers modifiers, const DSLType& returnType, std::s
void DSLFunction::define(DSLBlock block, Position pos) {
std::unique_ptr<SkSL::Block> body = block.release();
body->fPosition = pos;
if (!fDecl) {
// Evidently we failed to create the declaration; error should already have been reported.
// Release the block so we don't fail its destructor assert.

26
src/sksl/dsl/DSLType.cpp
View File

@ -53,8 +53,8 @@ static const SkSL::Type* verify_type(const Context& context,
}
static const SkSL::Type* find_type(const Context& context,
std::string_view name,
Position pos) {
Position pos,
std::string_view name) {
const Symbol* symbol = (*ThreadContext::SymbolTable())[name];
if (!symbol) {
context.fErrors->error(String::printf("no symbol named '%.*s'",
@ -71,13 +71,14 @@ static const SkSL::Type* find_type(const Context& context,
}
static const SkSL::Type* find_type(const Context& context,
Position overallPos,
std::string_view name,
Modifiers* modifiers,
Position pos) {
const Type* type = find_type(context, name, pos);
Position modifiersPos,
Modifiers* modifiers) {
const Type* type = find_type(context, overallPos, name);
type = type->applyPrecisionQualifiers(context, modifiers, ThreadContext::SymbolTable().get(),
pos);
ThreadContext::ReportErrors(pos);
modifiersPos);
ThreadContext::ReportErrors(overallPos);
return type;
}
@ -187,14 +188,15 @@ static const SkSL::Type* get_type_from_type_constant(const Context& context, Typ
}
DSLType::DSLType(std::string_view name)
: fSkSLType(find_type(ThreadContext::Context(), name, Position())) {}
: fSkSLType(find_type(ThreadContext::Context(), Position(), name)) {}
DSLType::DSLType(std::string_view name, DSLModifiers* modifiers, Position pos)
: fSkSLType(find_type(ThreadContext::Context(), name, &modifiers->fModifiers, pos)) {}
: fSkSLType(find_type(ThreadContext::Context(), pos, name, modifiers->fPosition,
&modifiers->fModifiers)) {}
DSLType::DSLType(const SkSL::Type* type)
: fSkSLType(verify_type(ThreadContext::Context(), type, /*allowPrivateTypes=*/true,
Position())) {}
: fSkSLType(verify_type(ThreadContext::Context(), type, /*allowPrivateTypes=*/true,
Position())) {}
bool DSLType::isBoolean() const {
return this->skslType().isBoolean();
@ -270,7 +272,7 @@ DSLPossibleExpression DSLType::Construct(DSLType type, SkSpan<DSLExpression> arg
}
DSLType Array(const DSLType& base, int count, Position pos) {
count = base.skslType().convertArraySize(ThreadContext::Context(),
count = base.skslType().convertArraySize(ThreadContext::Context(), pos,
DSLExpression(count, pos).release());
ThreadContext::ReportErrors(pos);
if (!count) {

7
src/sksl/ir/SkSLExpression.cpp
View File

@ -15,15 +15,16 @@ bool Expression::isIncomplete(const Context& context) const {
switch (this->kind()) {
case Kind::kFunctionReference:
case Kind::kExternalFunctionReference:
context.fErrors->error(fPosition, "expected '(' to begin function call");
context.fErrors->error(fPosition.after(), "expected '(' to begin function call");
return true;
case Kind::kMethodReference:
context.fErrors->error(fPosition, "expected '(' to begin method call");
context.fErrors->error(fPosition.after(), "expected '(' to begin method call");
return true;
case Kind::kTypeReference:
context.fErrors->error(fPosition, "expected '(' to begin constructor invocation");
context.fErrors->error(fPosition.after(),
"expected '(' to begin constructor invocation");
return true;
default:

2
src/sksl/ir/SkSLFunctionDefinition.cpp
View File

@ -259,7 +259,7 @@ std::unique_ptr<FunctionDefinition> FunctionDefinition::Convert(const Context& c
}
if (Analysis::CanExitWithoutReturningValue(function, *body)) {
context.fErrors->error(function.fPosition, "function '" + std::string(function.name()) +
context.fErrors->error(body->fPosition, "function '" + std::string(function.name()) +
"' can exit without returning a value");
}

13
src/sksl/ir/SkSLIndexExpression.cpp
View File

@ -18,13 +18,14 @@
namespace SkSL {
static bool index_out_of_range(const Context& context, SKSL_INT index, const Expression& base) {
static bool index_out_of_range(const Context& context, Position pos, SKSL_INT index,
const Expression& base) {
if (index >= 0 && index < base.type().columns()) {
return false;
}
context.fErrors->error(base.fPosition, "index " + std::to_string(index) +
" out of range for '" + base.type().displayName() + "'");
context.fErrors->error(pos, "index " + std::to_string(index) + " out of range for '" +
base.type().displayName() + "'");
return true;
}
@ -57,7 +58,7 @@ std::unique_ptr<Expression> IndexExpression::Convert(const Context& context,
// Convert an array type reference: `int[10]`.
if (base->is<TypeReference>()) {
const Type& baseType = base->as<TypeReference>().value();
SKSL_INT arraySize = baseType.convertArraySize(context, std::move(index));
SKSL_INT arraySize = baseType.convertArraySize(context, pos, std::move(index));
if (!arraySize) {
return nullptr;
}
@ -81,7 +82,7 @@ std::unique_ptr<Expression> IndexExpression::Convert(const Context& context,
const Expression* indexExpr = ConstantFolder::GetConstantValueForVariable(*index);
if (indexExpr->isIntLiteral()) {
SKSL_INT indexValue = indexExpr->as<Literal>().intValue();
if (index_out_of_range(context, indexValue, *base)) {
if (index_out_of_range(context, index->fPosition, indexValue, *base)) {
return nullptr;
}
}
@ -99,7 +100,7 @@ std::unique_ptr<Expression> IndexExpression::Make(const Context& context,
const Expression* indexExpr = ConstantFolder::GetConstantValueForVariable(*index);
if (indexExpr->isIntLiteral()) {
SKSL_INT indexValue = indexExpr->as<Literal>().intValue();
if (!index_out_of_range(context, indexValue, *base)) {
if (!index_out_of_range(context, index->fPosition, indexValue, *base)) {
if (baseType.isVector()) {
// Constant array indexes on vectors can be converted to swizzles: `v[2]` --> `v.z`.
// Swizzling is harmless and can unlock further simplifications for some base types.

2
src/sksl/ir/SkSLPostfixExpression.cpp
View File

@ -18,7 +18,7 @@ std::unique_ptr<Expression> PostfixExpression::Convert(const Context& context, P
std::unique_ptr<Expression> base, Operator op) {
const Type& baseType = base->type();
if (!baseType.isNumber()) {
context.fErrors->error(base->fPosition, "'" + std::string(op.tightOperatorName()) +
context.fErrors->error(pos, "'" + std::string(op.tightOperatorName()) +
"' cannot operate on '" + baseType.displayName() + "'");
return nullptr;
}

2
src/sksl/ir/SkSLPrefixExpression.cpp
View File

@ -35,7 +35,7 @@ static std::unique_ptr<Expression> simplify_negation(const Context& context,
double negated = -value->as<Literal>().value();
// Don't simplify the expression if the type can't hold the negated value.
const Type& type = value->type();
if (type.checkForOutOfRangeLiteral(context, negated, value->fPosition)) {
if (type.checkForOutOfRangeLiteral(context, negated, pos)) {
return nullptr;
}
return Literal::Make(pos, negated, &type);

3
src/sksl/ir/SkSLSwitchStatement.cpp
View File

@ -264,8 +264,7 @@ std::unique_ptr<Statement> SwitchStatement::Make(const Context& context,
// Report an error if this was a static switch and BlockForCase failed us.
if (isStatic) {
context.fErrors->error(value->fPosition,
"static switch contains non-static conditional exit");
context.fErrors->error(pos, "static switch contains non-static conditional exit");
return nullptr;
}
}

11
src/sksl/ir/SkSLType.cpp
View File

@ -965,22 +965,23 @@ bool Type::checkForOutOfRangeLiteral(const Context& context, double value, Posit
return false;
}
SKSL_INT Type::convertArraySize(const Context& context, std::unique_ptr<Expression> size) const {
SKSL_INT Type::convertArraySize(const Context& context, Position arrayPos,
std::unique_ptr<Expression> size) const {
size = context.fTypes.fInt->coerceExpression(std::move(size), context);
if (!size) {
return 0;
}
if (this->isArray()) {
context.fErrors->error(size->fPosition, "multi-dimensional arrays are not supported");
context.fErrors->error(arrayPos, "multi-dimensional arrays are not supported");
return 0;
}
if (this->isVoid()) {
context.fErrors->error(size->fPosition, "type 'void' may not be used in an array");
context.fErrors->error(arrayPos, "type 'void' may not be used in an array");
return 0;
}
if (this->isOpaque()) {
context.fErrors->error(size->fPosition, "opaque type '" + std::string(this->name()) +
"' may not be used in an array");
context.fErrors->error(arrayPos, "opaque type '" + std::string(this->name()) +
"' may not be used in an array");
return 0;
}
SKSL_INT count;

3
src/sksl/ir/SkSLType.h
View File

@ -530,7 +530,8 @@ public:
* Verifies that the expression is a valid constant array size for this type. Returns the array
* size, or zero if the expression isn't a valid literal value.
*/
SKSL_INT convertArraySize(const Context& context, std::unique_ptr<Expression> size) const;
SKSL_INT convertArraySize(const Context& context, Position arrayPos,
std::unique_ptr<Expression> size) const;
protected:
Type(std::string_view name, const char* abbrev, TypeKind kind,

5
src/sksl/ir/SkSLVariable.cpp
View File

@ -36,7 +36,8 @@ std::unique_ptr<Variable> Variable::Convert(const Context& context, Position pos
if (modifiers.fLayout.fLocation == 0 && modifiers.fLayout.fIndex == 0 &&
(modifiers.fFlags & Modifiers::kOut_Flag) &&
context.fConfig->fKind == ProgramKind::kFragment && name != Compiler::FRAGCOLOR_NAME) {
context.fErrors->error(pos, "out location=0, index=0 is reserved for sk_FragColor");
context.fErrors->error(modifiersPos,
"out location=0, index=0 is reserved for sk_FragColor");
}
if (!context.fConfig->fIsBuiltinCode && skstd::starts_with(name, '$')) {
context.fErrors->error(pos, "name '" + std::string(name) + "' is reserved");
@ -54,7 +55,7 @@ std::unique_ptr<Variable> Variable::Make(const Context& context, Position pos,
int arraySizeValue = 0;
if (isArray) {
SkASSERT(arraySize);
arraySizeValue = type->convertArraySize(context, std::move(arraySize));
arraySizeValue = type->convertArraySize(context, pos, std::move(arraySize));
if (!arraySizeValue) {
return nullptr;
}

4
tests/sksl/errors/ArrayInlinedIndexOutOfRange.glsl
View File

@ -1,5 +1,5 @@
### Compilation failed:
error: 6: index -1 out of range for 'int[3]'
error: 6: index 3 out of range for 'int[3]'
error: 11: index -1 out of range for 'int[3]'
error: 11: index 3 out of range for 'int[3]'
2 errors

4
tests/sksl/errors/MatrixInlinedIndexOutOfRange.glsl
View File

@ -1,5 +1,5 @@
### Compilation failed:
error: 6: index -1 out of range for 'float3x3'
error: 6: index 3 out of range for 'float3x3'
error: 11: index -1 out of range for 'float3x3'
error: 11: index 3 out of range for 'float3x3'
2 errors

4
tests/sksl/errors/VectorInlinedIndexOutOfRange.glsl
View File

@ -1,5 +1,5 @@
### Compilation failed:
error: 6: index -1 out of range for 'int3'
error: 6: index 3 out of range for 'int3'
error: 11: index -1 out of range for 'int3'
error: 11: index 3 out of range for 'int3'
2 errors