Improved SkSL error positions for return types

Bug: skia:13169
Change-Id: Icf0b720d3e3a13d490aba8495cf9db83d1d62318
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/532762
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-22 13:45:14 -04:00 committed by SkCQ
parent 9431b073fe
commit d418ddbfc5
12 changed files with 39 additions and 27 deletions

View File

@ -64,7 +64,6 @@ generated_cc_atom(
":DSLExpression_hdr",
":DSLModifiers_hdr",
":DSLStatement_hdr",
":DSLType_hdr",
":DSLVar_hdr",
":SkSLPosition_hdr",
"//include/private:SkSLDefines_hdr",

View File

@ -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 <typename T> class DSLWrapper;
class DSLFunction {

View File

@ -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<DSLField> fields, Position pos);

View File

@ -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",

View File

@ -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) {

View File

@ -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<DSLField> fields, Position pos) {
@ -317,7 +319,7 @@ DSLType Struct(std::string_view name, SkSpan<DSLField> fields, Position pos) {
}
ThreadContext::ProgramElements().push_back(std::make_unique<SkSL::StructDefinition>(Position(),
*result));
return result;
return DSLType(result, pos);
}
} // namespace dsl

View File

@ -290,6 +290,7 @@ static bool find_existing_declaration(const Context& context,
Position pos,
std::string_view name,
std::vector<std::unique_ptr<Variable>>& 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<std::unique_ptr<Variable>> 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<const Variable*> finalParameters;
@ -400,8 +403,11 @@ const FunctionDeclaration* FunctionDeclaration::Convert(
if (decl) {
return decl;
}
auto result = std::make_unique<FunctionDeclaration>(pos, modifiers, name,
std::move(finalParameters), returnType,
auto result = std::make_unique<FunctionDeclaration>(pos,
modifiers,
name,
std::move(finalParameters),
returnType,
context.fConfig->fIsBuiltinCode);
return symbols.add(std::move(result));
}

View File

@ -51,6 +51,7 @@ public:
const Modifiers* modifiers,
std::string_view name,
std::vector<std::unique_ptr<Variable>> parameters,
Position returnTypePos,
const Type* returnType);
const Modifiers& modifiers() const {

View File

@ -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

View File

@ -2,5 +2,5 @@
error: 2: functions 'void func()' and 'int func()' differ only in return type
void func() {}
^^^^^^^^^^^
^^^^
1 error

View File

@ -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; }
^^^^^^^^

View File

@ -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); }
^^^^^^^^^^