diff --git a/include/sksl/SkSLPosition.h b/include/sksl/SkSLPosition.h index 6b626e5444..47634199a1 100644 --- a/include/sksl/SkSLPosition.h +++ b/include/sksl/SkSLPosition.h @@ -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; diff --git a/src/sksl/analysis/SkSLFinalizationChecks.cpp b/src/sksl/analysis/SkSLFinalizationChecks.cpp index d3ae2efe50..87518c00b3 100644 --- a/src/sksl/analysis/SkSLFinalizationChecks.cpp +++ b/src/sksl/analysis/SkSLFinalizationChecks.cpp @@ -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().isStatic()) { - fContext.fErrors->error(stmt.fPosition, "static if has non-static test"); + fContext.fErrors->error(stmt.as().test()->fPosition, + "static if has non-static test"); } break; case Statement::Kind::kSwitch: if (stmt.as().isStatic()) { - fContext.fErrors->error(stmt.fPosition, "static switch has non-static test"); + fContext.fErrors->error(stmt.as().value()->fPosition, + "static switch has non-static test"); } break; diff --git a/src/sksl/dsl/BUILD.bazel b/src/sksl/dsl/BUILD.bazel index abe25e8783..fb4608e6c1 100644 --- a/src/sksl/dsl/BUILD.bazel +++ b/src/sksl/dsl/BUILD.bazel @@ -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", diff --git a/src/sksl/dsl/DSLFunction.cpp b/src/sksl/dsl/DSLFunction.cpp index af8c43f212..4e1e5da0b0 100644 --- a/src/sksl/dsl/DSLFunction.cpp +++ b/src/sksl/dsl/DSLFunction.cpp @@ -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 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. diff --git a/src/sksl/dsl/DSLType.cpp b/src/sksl/dsl/DSLType.cpp index b9a91f58e4..524124b4eb 100644 --- a/src/sksl/dsl/DSLType.cpp +++ b/src/sksl/dsl/DSLType.cpp @@ -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 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) { diff --git a/src/sksl/ir/SkSLExpression.cpp b/src/sksl/ir/SkSLExpression.cpp index a80582af00..d88549004b 100644 --- a/src/sksl/ir/SkSLExpression.cpp +++ b/src/sksl/ir/SkSLExpression.cpp @@ -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: diff --git a/src/sksl/ir/SkSLFunctionDefinition.cpp b/src/sksl/ir/SkSLFunctionDefinition.cpp index eab25d4db9..e8e98882fd 100644 --- a/src/sksl/ir/SkSLFunctionDefinition.cpp +++ b/src/sksl/ir/SkSLFunctionDefinition.cpp @@ -259,7 +259,7 @@ std::unique_ptr 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"); } diff --git a/src/sksl/ir/SkSLIndexExpression.cpp b/src/sksl/ir/SkSLIndexExpression.cpp index 112d337ba6..caf4503827 100644 --- a/src/sksl/ir/SkSLIndexExpression.cpp +++ b/src/sksl/ir/SkSLIndexExpression.cpp @@ -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 IndexExpression::Convert(const Context& context, // Convert an array type reference: `int[10]`. if (base->is()) { const Type& baseType = base->as().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 IndexExpression::Convert(const Context& context, const Expression* indexExpr = ConstantFolder::GetConstantValueForVariable(*index); if (indexExpr->isIntLiteral()) { SKSL_INT indexValue = indexExpr->as().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 IndexExpression::Make(const Context& context, const Expression* indexExpr = ConstantFolder::GetConstantValueForVariable(*index); if (indexExpr->isIntLiteral()) { SKSL_INT indexValue = indexExpr->as().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. diff --git a/src/sksl/ir/SkSLPostfixExpression.cpp b/src/sksl/ir/SkSLPostfixExpression.cpp index 051a71021e..d8aa542af2 100644 --- a/src/sksl/ir/SkSLPostfixExpression.cpp +++ b/src/sksl/ir/SkSLPostfixExpression.cpp @@ -18,7 +18,7 @@ std::unique_ptr PostfixExpression::Convert(const Context& context, P std::unique_ptr 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; } diff --git a/src/sksl/ir/SkSLPrefixExpression.cpp b/src/sksl/ir/SkSLPrefixExpression.cpp index b5734d9419..676ffe8546 100644 --- a/src/sksl/ir/SkSLPrefixExpression.cpp +++ b/src/sksl/ir/SkSLPrefixExpression.cpp @@ -35,7 +35,7 @@ static std::unique_ptr simplify_negation(const Context& context, double negated = -value->as().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); diff --git a/src/sksl/ir/SkSLSwitchStatement.cpp b/src/sksl/ir/SkSLSwitchStatement.cpp index e23b1223ad..31fd9b9d68 100644 --- a/src/sksl/ir/SkSLSwitchStatement.cpp +++ b/src/sksl/ir/SkSLSwitchStatement.cpp @@ -264,8 +264,7 @@ std::unique_ptr 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; } } diff --git a/src/sksl/ir/SkSLType.cpp b/src/sksl/ir/SkSLType.cpp index 99a92abadd..d54ce90212 100644 --- a/src/sksl/ir/SkSLType.cpp +++ b/src/sksl/ir/SkSLType.cpp @@ -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 size) const { +SKSL_INT Type::convertArraySize(const Context& context, Position arrayPos, + std::unique_ptr 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; diff --git a/src/sksl/ir/SkSLType.h b/src/sksl/ir/SkSLType.h index 7bcd9f5e56..4ac7d96b72 100644 --- a/src/sksl/ir/SkSLType.h +++ b/src/sksl/ir/SkSLType.h @@ -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 size) const; + SKSL_INT convertArraySize(const Context& context, Position arrayPos, + std::unique_ptr size) const; protected: Type(std::string_view name, const char* abbrev, TypeKind kind, diff --git a/src/sksl/ir/SkSLVariable.cpp b/src/sksl/ir/SkSLVariable.cpp index ecb81b4f59..1eccb99edb 100644 --- a/src/sksl/ir/SkSLVariable.cpp +++ b/src/sksl/ir/SkSLVariable.cpp @@ -36,7 +36,8 @@ std::unique_ptr 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::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; } diff --git a/tests/sksl/errors/ArrayInlinedIndexOutOfRange.glsl b/tests/sksl/errors/ArrayInlinedIndexOutOfRange.glsl index bd51087fa1..6e1272d6e9 100644 --- a/tests/sksl/errors/ArrayInlinedIndexOutOfRange.glsl +++ b/tests/sksl/errors/ArrayInlinedIndexOutOfRange.glsl @@ -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 diff --git a/tests/sksl/errors/MatrixInlinedIndexOutOfRange.glsl b/tests/sksl/errors/MatrixInlinedIndexOutOfRange.glsl index f10a161d9f..c460143b99 100644 --- a/tests/sksl/errors/MatrixInlinedIndexOutOfRange.glsl +++ b/tests/sksl/errors/MatrixInlinedIndexOutOfRange.glsl @@ -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 diff --git a/tests/sksl/errors/VectorInlinedIndexOutOfRange.glsl b/tests/sksl/errors/VectorInlinedIndexOutOfRange.glsl index d00358d4ab..9f5eaec5ac 100644 --- a/tests/sksl/errors/VectorInlinedIndexOutOfRange.glsl +++ b/tests/sksl/errors/VectorInlinedIndexOutOfRange.glsl @@ -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