diff --git a/include/sksl/BUILD.bazel b/include/sksl/BUILD.bazel index e71ae2ffda..ebc6106065 100644 --- a/include/sksl/BUILD.bazel +++ b/include/sksl/BUILD.bazel @@ -64,7 +64,6 @@ generated_cc_atom( ":DSLExpression_hdr", ":DSLModifiers_hdr", ":DSLStatement_hdr", - ":DSLType_hdr", ":DSLVar_hdr", ":SkSLPosition_hdr", "//include/private:SkSLDefines_hdr", diff --git a/include/sksl/DSLFunction.h b/include/sksl/DSLFunction.h index a2ddcc1933..4966a217af 100644 --- a/include/sksl/DSLFunction.h +++ b/include/sksl/DSLFunction.h @@ -14,7 +14,6 @@ #include "include/sksl/DSLExpression.h" #include "include/sksl/DSLModifiers.h" #include "include/sksl/DSLStatement.h" -#include "include/sksl/DSLType.h" #include "include/sksl/DSLVar.h" #include "include/sksl/SkSLPosition.h" @@ -27,6 +26,7 @@ class FunctionDeclaration; namespace dsl { +class DSLType; template class DSLWrapper; class DSLFunction { diff --git a/include/sksl/DSLType.h b/include/sksl/DSLType.h index baea65a4e3..6708caefc5 100644 --- a/include/sksl/DSLType.h +++ b/include/sksl/DSLType.h @@ -82,12 +82,13 @@ enum TypeConstant : uint8_t { class DSLType { public: - DSLType(TypeConstant tc) - : fTypeConstant(tc) {} + DSLType(TypeConstant tc, Position pos = {}) + : fTypeConstant(tc) + , fPosition(pos) {} - DSLType(const SkSL::Type* type); + DSLType(const SkSL::Type* type, Position pos = {}); - DSLType(std::string_view name); + DSLType(std::string_view name, Position pos = {}); DSLType(std::string_view name, DSLModifiers* modifiers, @@ -171,8 +172,8 @@ private: const SkSL::Type& skslType() const; const SkSL::Type* fSkSLType = nullptr; - TypeConstant fTypeConstant = kPoison_Type; + Position fPosition; friend DSLType Array(const DSLType& base, int count, Position pos); friend DSLType Struct(std::string_view name, SkSpan fields, Position pos); diff --git a/src/sksl/dsl/BUILD.bazel b/src/sksl/dsl/BUILD.bazel index 6bbad41cd5..0d12bd9a81 100644 --- a/src/sksl/dsl/BUILD.bazel +++ b/src/sksl/dsl/BUILD.bazel @@ -116,6 +116,7 @@ generated_cc_atom( "//include/private:SkSLStatement_hdr", "//include/private:SkSLString_hdr", "//include/sksl:DSLFunction_hdr", + "//include/sksl:DSLType_hdr", "//include/sksl:DSLVar_hdr", "//include/sksl:DSLWrapper_hdr", "//src/sksl:SkSLProgramSettings_hdr", diff --git a/src/sksl/dsl/DSLFunction.cpp b/src/sksl/dsl/DSLFunction.cpp index 4e1e5da0b0..4022a7c5fe 100644 --- a/src/sksl/dsl/DSLFunction.cpp +++ b/src/sksl/dsl/DSLFunction.cpp @@ -12,6 +12,7 @@ #include "include/private/SkSLProgramElement.h" #include "include/private/SkSLStatement.h" #include "include/private/SkSLString.h" +#include "include/sksl/DSLType.h" #include "include/sksl/DSLVar.h" #include "include/sksl/DSLWrapper.h" #include "src/sksl/SkSLProgramSettings.h" @@ -70,7 +71,8 @@ void DSLFunction::init(DSLModifiers modifiers, const DSLType& returnType, std::s modifiers.fPosition, ThreadContext::Modifiers(modifiers.fModifiers), name == "main" ? name : DSLWriter::Name(name), - std::move(paramVars), &returnType.skslType()); + std::move(paramVars), returnType.fPosition, + &returnType.skslType()); ThreadContext::ReportErrors(pos); if (fDecl) { for (size_t i = 0; i < params.size(); ++i) { diff --git a/src/sksl/dsl/DSLType.cpp b/src/sksl/dsl/DSLType.cpp index 524124b4eb..a46eab13ae 100644 --- a/src/sksl/dsl/DSLType.cpp +++ b/src/sksl/dsl/DSLType.cpp @@ -187,16 +187,18 @@ static const SkSL::Type* get_type_from_type_constant(const Context& context, Typ } } -DSLType::DSLType(std::string_view name) - : fSkSLType(find_type(ThreadContext::Context(), Position(), name)) {} +DSLType::DSLType(std::string_view name, Position pos) + : fSkSLType(find_type(ThreadContext::Context(), pos, name)) + , fPosition(pos) {} DSLType::DSLType(std::string_view name, DSLModifiers* modifiers, Position pos) : fSkSLType(find_type(ThreadContext::Context(), pos, name, modifiers->fPosition, - &modifiers->fModifiers)) {} + &modifiers->fModifiers)) + , fPosition(pos) {} -DSLType::DSLType(const SkSL::Type* type) - : fSkSLType(verify_type(ThreadContext::Context(), type, /*allowPrivateTypes=*/true, - Position())) {} +DSLType::DSLType(const SkSL::Type* type, Position pos) + : fSkSLType(verify_type(ThreadContext::Context(), type, /*allowPrivateTypes=*/true, pos)) + , fPosition(pos) {} bool DSLType::isBoolean() const { return this->skslType().isBoolean(); @@ -278,7 +280,7 @@ DSLType Array(const DSLType& base, int count, Position pos) { if (!count) { return DSLType(kPoison_Type); } - return ThreadContext::SymbolTable()->addArrayDimension(&base.skslType(), count); + return DSLType(ThreadContext::SymbolTable()->addArrayDimension(&base.skslType(), count), pos); } DSLType Struct(std::string_view name, SkSpan fields, Position pos) { @@ -317,7 +319,7 @@ DSLType Struct(std::string_view name, SkSpan fields, Position pos) { } ThreadContext::ProgramElements().push_back(std::make_unique(Position(), *result)); - return result; + return DSLType(result, pos); } } // namespace dsl diff --git a/src/sksl/ir/SkSLFunctionDeclaration.cpp b/src/sksl/ir/SkSLFunctionDeclaration.cpp index 9834595800..c2c246e8a9 100644 --- a/src/sksl/ir/SkSLFunctionDeclaration.cpp +++ b/src/sksl/ir/SkSLFunctionDeclaration.cpp @@ -290,6 +290,7 @@ static bool find_existing_declaration(const Context& context, Position pos, std::string_view name, std::vector>& parameters, + Position returnTypePos, const Type* returnType, const FunctionDeclaration** outExistingDecl) { ErrorReporter& errors = *context.fErrors; @@ -335,7 +336,7 @@ static bool find_existing_declaration(const Context& context, std::move(paramPtrs), returnType, context.fConfig->fIsBuiltinCode); - errors.error(pos, + errors.error(returnTypePos, "functions '" + invalidDecl.description() + "' and '" + other->description() + "' differ only in return type"); return false; @@ -381,15 +382,17 @@ const FunctionDeclaration* FunctionDeclaration::Convert( const Modifiers* modifiers, std::string_view name, std::vector> parameters, + Position returnTypePos, const Type* returnType) { bool isMain = (name == "main"); const FunctionDeclaration* decl = nullptr; if (!check_modifiers(context, modifiersPosition, *modifiers) || - !check_return_type(context, pos, *returnType) || + !check_return_type(context, returnTypePos, *returnType) || !check_parameters(context, parameters, isMain) || (isMain && !check_main_signature(context, pos, *returnType, parameters)) || - !find_existing_declaration(context, symbols, pos, name, parameters, returnType, &decl)) { + !find_existing_declaration(context, symbols, pos, name, parameters, returnTypePos, + returnType, &decl)) { return nullptr; } std::vector finalParameters; @@ -400,8 +403,11 @@ const FunctionDeclaration* FunctionDeclaration::Convert( if (decl) { return decl; } - auto result = std::make_unique(pos, modifiers, name, - std::move(finalParameters), returnType, + auto result = std::make_unique(pos, + modifiers, + name, + std::move(finalParameters), + returnType, context.fConfig->fIsBuiltinCode); return symbols.add(std::move(result)); } diff --git a/src/sksl/ir/SkSLFunctionDeclaration.h b/src/sksl/ir/SkSLFunctionDeclaration.h index 95ce678505..60695ad33c 100644 --- a/src/sksl/ir/SkSLFunctionDeclaration.h +++ b/src/sksl/ir/SkSLFunctionDeclaration.h @@ -51,6 +51,7 @@ public: const Modifiers* modifiers, std::string_view name, std::vector> parameters, + Position returnTypePos, const Type* returnType); const Modifiers& modifiers() const { diff --git a/tests/sksl/errors/ArrayReturnTypes.glsl b/tests/sksl/errors/ArrayReturnTypes.glsl index cdb1721898..b5b5722f4e 100644 --- a/tests/sksl/errors/ArrayReturnTypes.glsl +++ b/tests/sksl/errors/ArrayReturnTypes.glsl @@ -2,8 +2,8 @@ error: 1: functions may not return type 'float4x4[2]' float4x4[2] return_float4x4_2() {} -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +^^^^^^^^^^^ error: 2: functions may not return type 'int[1]' int[1] return_int_1() {} -^^^^^^^^^^^^^^^^^^^^^^^^^^ +^^^^^^ 2 errors diff --git a/tests/sksl/errors/ReturnDifferentType.glsl b/tests/sksl/errors/ReturnDifferentType.glsl index 789f0e382f..7dc309a34f 100644 --- a/tests/sksl/errors/ReturnDifferentType.glsl +++ b/tests/sksl/errors/ReturnDifferentType.glsl @@ -2,5 +2,5 @@ error: 2: functions 'void func()' and 'int func()' differ only in return type void func() {} -^^^^^^^^^^^ +^^^^ 1 error diff --git a/tests/sksl/runtime_errors/IllegalArrayOps.skvm b/tests/sksl/runtime_errors/IllegalArrayOps.skvm index 062afd8c28..4ba6c64dcb 100644 --- a/tests/sksl/runtime_errors/IllegalArrayOps.skvm +++ b/tests/sksl/runtime_errors/IllegalArrayOps.skvm @@ -11,10 +11,10 @@ void assign_T() { t1 = t2; } ^^^^^^^ error: 21: functions may not return structs containing arrays S return_S() { return s1; } -^^^^^^^^^^^^ +^ error: 22: functions may not return structs containing arrays T return_T() { return t1; } -^^^^^^^^^^^^ +^ error: 24: operator '==' can not operate on arrays (or structs containing arrays) bool equals_A() { return a1 == a2; } ^^^^^^^^ diff --git a/tests/sksl/runtime_errors/IllegalShaderUse.skvm b/tests/sksl/runtime_errors/IllegalShaderUse.skvm index ac36f7fd3d..4e0ceaab0f 100644 --- a/tests/sksl/runtime_errors/IllegalShaderUse.skvm +++ b/tests/sksl/runtime_errors/IllegalShaderUse.skvm @@ -56,7 +56,7 @@ half4 parameter(shader s) { return s.eval(xy); } ^ error: 29: functions may not return opaque type 'shader' shader returned() { return s1; } -^^^^^^^^^^^^^^^^^ +^^^^^^ error: 30: cannot construct 'shader' half4 constructed() { return shader(s1).eval(xy); } ^^^^^^^^^^