From fdef03321033af75c363d7f40ab33ada91c965bb Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Tue, 21 Feb 2023 21:07:35 +0000 Subject: [PATCH] tint/resolver: Fix ICE in ResolvedIdentifier::String() That resolve to parameters. Bug: chromium:1417513 Change-Id: Id7722524acbd9a9a26543cb73f48c8823dd51356 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/120840 Kokoro: Ben Clayton Reviewed-by: Dan Sinclair Commit-Queue: Ben Clayton --- src/tint/resolver/dependency_graph.cc | 3 + src/tint/resolver/expression_kind_test.cc | 96 ++++++++++++++++------- 2 files changed, 70 insertions(+), 29 deletions(-) diff --git a/src/tint/resolver/dependency_graph.cc b/src/tint/resolver/dependency_graph.cc index e913789525..e0f0656664 100644 --- a/src/tint/resolver/dependency_graph.cc +++ b/src/tint/resolver/dependency_graph.cc @@ -878,6 +878,9 @@ std::string ResolvedIdentifier::String(const SymbolTable& symbols, diag::List& d [&](const ast::Function* n) { // return "function '" + symbols.NameFor(n->name->symbol) + "'"; }, + [&](const ast::Parameter* n) { // + return "parameter '" + symbols.NameFor(n->name->symbol) + "'"; + }, [&](Default) { TINT_UNREACHABLE(Resolver, diagnostics) << "unhandled ast::Node: " << node->TypeInfo().name; diff --git a/src/tint/resolver/expression_kind_test.cc b/src/tint/resolver/expression_kind_test.cc index 01248b3d9e..fb23e1438a 100644 --- a/src/tint/resolver/expression_kind_test.cc +++ b/src/tint/resolver/expression_kind_test.cc @@ -31,6 +31,7 @@ enum class Def { kFunction, kInterpolationSampling, kInterpolationType, + kParameter, kStruct, kTexelFormat, kTypeAlias, @@ -55,6 +56,8 @@ std::ostream& operator<<(std::ostream& out, Def def) { return out << "Def::kInterpolationSampling"; case Def::kInterpolationType: return out << "Def::kInterpolationType"; + case Def::kParameter: + return out << "Def::kParameter"; case Def::kStruct: return out << "Def::kStruct"; case Def::kTexelFormat: @@ -138,6 +141,11 @@ using ResolverExpressionKindTest = ResolverTestWithParam; TEST_P(ResolverExpressionKindTest, Test) { Symbol sym; std::function check_expr; + + utils::Vector fn_params; + utils::Vector fn_stmts; + utils::Vector fn_attrs; + switch (GetParam().def) { case Def::kAccess: { sym = Sym("write"); @@ -185,8 +193,8 @@ TEST_P(ResolverExpressionKindTest, Test) { break; } case Def::kFunction: { - auto* fn = Func(kDefSource, "FUNCTION", utils::Empty, ty.i32(), Return(1_i)); sym = Sym("FUNCTION"); + auto* fn = Func(kDefSource, sym, utils::Empty, ty.i32(), Return(1_i)); check_expr = [fn](const sem::Expression* expr) { ASSERT_NE(expr, nullptr); auto* fn_expr = expr->As(); @@ -217,9 +225,21 @@ TEST_P(ResolverExpressionKindTest, Test) { }; break; } + case Def::kParameter: { + sym = Sym("PARAMETER"); + auto* param = Param(kDefSource, sym, ty.i32()); + fn_params.Push(param); + check_expr = [param](const sem::Expression* expr) { + ASSERT_NE(expr, nullptr); + auto* user = expr->As(); + ASSERT_NE(user, nullptr); + EXPECT_EQ(user->Variable()->Declaration(), param); + }; + break; + } case Def::kStruct: { - auto* s = Structure(kDefSource, "STRUCT", utils::Vector{Member("m", ty.i32())}); sym = Sym("STRUCT"); + auto* s = Structure(kDefSource, sym, utils::Vector{Member("m", ty.i32())}); check_expr = [s](const sem::Expression* expr) { ASSERT_NE(expr, nullptr); auto* ty_expr = expr->As(); @@ -241,8 +261,8 @@ TEST_P(ResolverExpressionKindTest, Test) { break; } case Def::kTypeAlias: { - Alias(kDefSource, "ALIAS", ty.i32()); sym = Sym("ALIAS"); + Alias(kDefSource, sym, ty.i32()); check_expr = [](const sem::Expression* expr) { ASSERT_NE(expr, nullptr); auto* ty_expr = expr->As(); @@ -252,8 +272,8 @@ TEST_P(ResolverExpressionKindTest, Test) { break; } case Def::kVariable: { - auto* c = GlobalConst(kDefSource, "VARIABLE", Expr(1_i)); sym = Sym("VARIABLE"); + auto* c = GlobalConst(kDefSource, sym, Expr(1_i)); check_expr = [c](const sem::Expression* expr) { ASSERT_NE(expr, nullptr); auto* var_expr = expr->As(); @@ -271,62 +291,67 @@ TEST_P(ResolverExpressionKindTest, Test) { break; case Use::kAddressSpace: Enable(builtin::Extension::kChromiumExperimentalFullPtrParameters); - Func("f", utils::Vector{Param("p", ty("ptr", expr, ty.f32()))}, ty.void_(), + Func(Symbols().New(), utils::Vector{Param("p", ty("ptr", expr, ty.f32()))}, ty.void_(), utils::Empty); break; case Use::kCallExpr: - Func("f", utils::Empty, ty.void_(), Decl(Var("v", Call(expr)))); + fn_stmts.Push(Decl(Var("v", Call(expr)))); break; case Use::kCallStmt: - Func("f", utils::Empty, ty.void_(), CallStmt(Call(expr))); + fn_stmts.Push(CallStmt(Call(expr))); break; case Use::kBinaryOp: - GlobalVar("v", builtin::AddressSpace::kPrivate, Mul(1_a, expr)); + fn_stmts.Push(Decl(Var("v", Mul(1_a, expr)))); break; case Use::kBuiltinValue: - Func("f", utils::Vector{Param("p", ty.vec4(), utils::Vector{Builtin(expr)})}, + Func(Symbols().New(), + utils::Vector{Param("p", ty.vec4(), utils::Vector{Builtin(expr)})}, ty.void_(), utils::Empty, utils::Vector{Stage(ast::PipelineStage::kFragment)}); break; case Use::kFunctionReturnType: - Func("f", utils::Empty, ty(expr), Return(Call(sym))); + Func(Symbols().New(), utils::Empty, ty(expr), Return(Call(sym))); break; case Use::kInterpolationSampling: { - Func("f", - utils::Vector{Param("p", ty.vec4(), - utils::Vector{ - Location(0_a), - Interpolate(builtin::InterpolationType::kLinear, expr), - })}, - ty.void_(), utils::Empty, utils::Vector{Stage(ast::PipelineStage::kFragment)}); + fn_params.Push(Param("p", ty.vec4(), + utils::Vector{ + Location(0_a), + Interpolate(builtin::InterpolationType::kLinear, expr), + })); + fn_attrs.Push(Stage(ast::PipelineStage::kFragment)); break; } case Use::kInterpolationType: { - Func("f", - utils::Vector{Param("p", ty.vec4(), - utils::Vector{ - Location(0_a), - Interpolate(expr, builtin::InterpolationSampling::kCenter), - })}, - ty.void_(), utils::Empty, utils::Vector{Stage(ast::PipelineStage::kFragment)}); + fn_params.Push(Param("p", ty.vec4(), + utils::Vector{ + Location(0_a), + Interpolate(expr, builtin::InterpolationSampling::kCenter), + })); + fn_attrs.Push(Stage(ast::PipelineStage::kFragment)); break; } case Use::kMemberType: - Structure("s", utils::Vector{Member("m", ty(expr))}); + Structure(Symbols().New(), utils::Vector{Member("m", ty(expr))}); break; case Use::kTexelFormat: - GlobalVar("v", ty("texture_storage_2d", ty(expr), "write"), Group(0_u), Binding(0_u)); + GlobalVar(Symbols().New(), ty("texture_storage_2d", ty(expr), "write"), Group(0_u), + Binding(0_u)); break; case Use::kValueExpression: - GlobalVar("v", builtin::AddressSpace::kPrivate, expr); + fn_stmts.Push(Decl(Var("v", expr))); break; case Use::kVariableType: - GlobalVar("v", builtin::AddressSpace::kPrivate, ty(expr)); + fn_stmts.Push(Decl(Var("v", ty(expr)))); break; case Use::kUnaryOp: - GlobalVar("v", builtin::AddressSpace::kPrivate, Negation(expr)); + fn_stmts.Push(Assign(Phony(), Negation(expr))); break; } + if (!fn_params.IsEmpty() || !fn_stmts.IsEmpty()) { + Func(Symbols().New(), std::move(fn_params), ty.void_(), std::move(fn_stmts), + std::move(fn_attrs)); + } + if (GetParam().error == kPass) { EXPECT_TRUE(r()->Resolve()); EXPECT_EQ(r()->error(), ""); @@ -560,6 +585,19 @@ INSTANTIATE_TEST_SUITE_P( {Def::kInterpolationType, Use::kUnaryOp, R"(5:6 error: cannot use interpolation type 'linear' as value)"}, + {Def::kParameter, Use::kBinaryOp, kPass}, + {Def::kParameter, Use::kCallStmt, + R"(5:6 error: cannot use parameter 'PARAMETER' as call target +1:2 note: parameter 'PARAMETER' declared here)"}, + {Def::kParameter, Use::kCallExpr, + R"(5:6 error: cannot use parameter 'PARAMETER' as call target +1:2 note: parameter 'PARAMETER' declared here)"}, + {Def::kParameter, Use::kValueExpression, kPass}, + {Def::kParameter, Use::kVariableType, + R"(5:6 error: cannot use parameter 'PARAMETER' as type +1:2 note: parameter 'PARAMETER' declared here)"}, + {Def::kParameter, Use::kUnaryOp, kPass}, + {Def::kStruct, Use::kAccess, R"(5:6 error: cannot use type 'STRUCT' as access)"}, {Def::kStruct, Use::kAddressSpace, R"(5:6 error: cannot use type 'STRUCT' as address space)"},