Resolver: process nodes in order of declaration

Formerly, the resolver would process arrays and structs first, then
global variables, and finally functions. As we move validation from
Validator to Resolver, we need to process these nodes in declaration
order instead so that we can validate use-before-declaration. This
matches how the Validator processed nodes.

Fixed all tests that failed after this change mainly because of
variables declared after usage.

Bug: tint:642
Change-Id: I01a9575dcfff545b0a056195ec5266283552da38
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45383
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
This commit is contained in:
Antonio Maiorano 2021-03-19 18:45:30 +00:00 committed by Commit Bot service account
parent 1b51be56f0
commit bb5617f21a
9 changed files with 65 additions and 66 deletions

View File

@ -173,52 +173,52 @@ bool Resolver::IsValidAssignment(type::Type* lhs, type::Type* rhs) {
}
bool Resolver::ResolveInternal() {
// Process non-user-defined types (arrays). The rest will be processed in
// order of declaration below.
for (auto* ty : builder_->Types()) {
if (auto* str = ty->As<type::Struct>()) {
if (!Structure(str)) {
return false;
}
continue;
}
if (auto* arr = ty->As<type::Array>()) {
if (!Array(arr)) {
return false;
}
continue;
}
}
for (auto* var : builder_->AST().GlobalVariables()) {
variable_stack_.set_global(var->symbol(), CreateVariableInfo(var));
// Process everything else in the order they appear in the module. This is
// necessary for validation of use-before-declaration.
for (auto* decl : builder_->AST().GlobalDeclarations()) {
if (decl->Is<type::Type>()) {
if (auto* str = decl->As<type::Struct>()) {
if (!Structure(str)) {
return false;
}
} else if (auto* arr = decl->As<type::Array>()) {
if (!Array(arr)) {
return false;
}
}
} else if (auto* func = decl->As<ast::Function>()) {
if (!Function(func)) {
return false;
}
} else if (auto* var = decl->As<ast::Variable>()) {
variable_stack_.set_global(var->symbol(), CreateVariableInfo(var));
if (var->has_constructor()) {
if (!Expression(var->constructor())) {
if (var->has_constructor()) {
if (!Expression(var->constructor())) {
return false;
}
}
if (!ApplyStorageClassUsageToType(var->declared_storage_class(),
var->type(), var->source())) {
diagnostics_.add_note("while instantiating variable " +
builder_->Symbols().NameFor(var->symbol()),
var->source());
return false;
}
}
if (!ApplyStorageClassUsageToType(var->declared_storage_class(),
var->type(), var->source())) {
diagnostics_.add_note("while instantiating variable " +
builder_->Symbols().NameFor(var->symbol()),
var->source());
return false;
}
}
if (!Functions(builder_->AST().Functions())) {
return false;
}
return true;
}
bool Resolver::Functions(const ast::FunctionList& funcs) {
for (auto* func : funcs) {
if (!Function(func)) {
return false;
}
}
return true;
}

View File

@ -211,7 +211,6 @@ class Resolver {
bool Expression(ast::Expression*);
bool Expressions(const ast::ExpressionList&);
bool Function(ast::Function*);
bool Functions(const ast::FunctionList&);
bool Identifier(ast::IdentifierExpression*);
bool IfStatement(ast::IfStatement*);
bool IntrinsicCall(ast::CallExpression*, semantic::IntrinsicType);

View File

@ -516,11 +516,11 @@ TEST_F(ResolverTest, Expr_ArrayAccessor_Vector) {
}
TEST_F(ResolverTest, Expr_Bitcast) {
Global("name", ty.f32(), ast::StorageClass::kPrivate);
auto* bitcast = create<ast::BitcastExpression>(ty.f32(), Expr("name"));
WrapInFunction(bitcast);
Global("name", ty.f32(), ast::StorageClass::kPrivate);
EXPECT_TRUE(r()->Resolve()) << r()->error();
ASSERT_NE(TypeOf(bitcast), nullptr);
@ -833,6 +833,7 @@ TEST_F(ResolverTest, Function_RegisterInputOutputVariables_SubFunction) {
TEST_F(ResolverTest, Function_NotRegisterFunctionVariable) {
auto* var = Var("in_var", ty.f32(), ast::StorageClass::kFunction);
Global("var", ty.f32(), ast::StorageClass::kFunction);
auto* func =
Func("my_func", ast::VariableList{}, ty.void_(),
@ -842,8 +843,6 @@ TEST_F(ResolverTest, Function_NotRegisterFunctionVariable) {
},
ast::DecorationList{});
Global("var", ty.f32(), ast::StorageClass::kFunction);
EXPECT_TRUE(r()->Resolve()) << r()->error();
auto* func_sem = Sem().Get(func);
@ -1309,6 +1308,12 @@ TEST_F(ResolverTest, Function_EntryPoints_StageDecoration) {
// ep_1 -> {}
// ep_2 -> {}
Global("first", ty.f32(), ast::StorageClass::kPrivate);
Global("second", ty.f32(), ast::StorageClass::kPrivate);
Global("call_a", ty.f32(), ast::StorageClass::kPrivate);
Global("call_b", ty.f32(), ast::StorageClass::kPrivate);
Global("call_c", ty.f32(), ast::StorageClass::kPrivate);
ast::VariableList params;
auto* func_b =
Func("b", params, ty.f32(), ast::StatementList{Return(Expr(0.0f))},
@ -1344,12 +1349,6 @@ TEST_F(ResolverTest, Function_EntryPoints_StageDecoration) {
create<ast::StageDecoration>(ast::PipelineStage::kVertex),
});
Global("first", ty.f32(), ast::StorageClass::kPrivate);
Global("second", ty.f32(), ast::StorageClass::kPrivate);
Global("call_a", ty.f32(), ast::StorageClass::kPrivate);
Global("call_b", ty.f32(), ast::StorageClass::kPrivate);
Global("call_c", ty.f32(), ast::StorageClass::kPrivate);
ASSERT_TRUE(r()->Resolve()) << r()->error();
auto* func_b_sem = Sem().Get(func_b);

View File

@ -204,10 +204,11 @@ TEST_F(ValidatorTest, UsingUndefinedVariableGlobalVariableAfter_Fail) {
Global("global_var", ty.f32(), ast::StorageClass::kPrivate, Expr(2.1f));
ValidatorImpl& v = Build();
// TODO(amaiorano): Move to resolver tests. Program is invalid now because
// Resolver catches this. ValidatorImpl& v = Build();
EXPECT_FALSE(v.Validate());
EXPECT_EQ(v.error(), "12:34 v-0006: 'global_var' is not declared");
// EXPECT_FALSE(v.Validate());
// EXPECT_EQ(v.error(), "12:34 v-0006: 'global_var' is not declared");
}
TEST_F(ValidatorTest, UsingUndefinedVariableGlobalVariable_Pass) {

View File

@ -153,16 +153,16 @@ using HlslIntrinsicTest = TestParamHelper<IntrinsicData>;
TEST_P(HlslIntrinsicTest, Emit) {
auto param = GetParam();
auto* call = GenerateCall(param.intrinsic, param.type, this);
ASSERT_NE(nullptr, call) << "Unhandled intrinsic";
WrapInFunction(call);
Global("f2", ty.vec2<float>(), ast::StorageClass::kFunction);
Global("f3", ty.vec3<float>(), ast::StorageClass::kFunction);
Global("u2", ty.vec2<unsigned int>(), ast::StorageClass::kFunction);
Global("b2", ty.vec2<bool>(), ast::StorageClass::kFunction);
Global("m2x2", ty.mat2x2<float>(), ast::StorageClass::kFunction);
auto* call = GenerateCall(param.intrinsic, param.type, this);
ASSERT_NE(nullptr, call) << "Unhandled intrinsic";
WrapInFunction(call);
GeneratorImpl& gen = Build();
auto* sem = program->Sem().Get(call);

View File

@ -162,10 +162,6 @@ using MslIntrinsicTest = TestParamHelper<IntrinsicData>;
TEST_P(MslIntrinsicTest, Emit) {
auto param = GetParam();
auto* call = GenerateCall(param.intrinsic, param.type, this);
ASSERT_NE(nullptr, call) << "Unhandled intrinsic";
WrapInFunction(call);
Global("f2", ty.vec2<float>(), ast::StorageClass::kFunction);
Global("f3", ty.vec3<float>(), ast::StorageClass::kFunction);
Global("f4", ty.vec4<float>(), ast::StorageClass::kFunction);
@ -174,6 +170,10 @@ TEST_P(MslIntrinsicTest, Emit) {
Global("b2", ty.vec2<bool>(), ast::StorageClass::kFunction);
Global("m2x2", ty.mat2x2<float>(), ast::StorageClass::kFunction);
auto* call = GenerateCall(param.intrinsic, param.type, this);
ASSERT_NE(nullptr, call) << "Unhandled intrinsic";
WrapInFunction(call);
GeneratorImpl& gen = Build();
auto* sem = program->Sem().Get(call);

View File

@ -1490,13 +1490,13 @@ TEST_F(SpvBuilderConstructorTest,
TEST_F(SpvBuilderConstructorTest, IsConstructorConst_Vector_WithIdent) {
// vec3<f32>(a, b, c) -> false
auto* t = vec3<f32>("a", "b", "c");
WrapInFunction(t);
Global("a", ty.f32(), ast::StorageClass::kPrivate);
Global("b", ty.f32(), ast::StorageClass::kPrivate);
Global("c", ty.f32(), ast::StorageClass::kPrivate);
auto* t = vec3<f32>("a", "b", "c");
WrapInFunction(t);
spirv::Builder& b = Build();
EXPECT_FALSE(b.is_constructor_const(t, false));
@ -1565,12 +1565,12 @@ TEST_F(SpvBuilderConstructorTest,
Member("b", ty.vec3<f32>()),
});
auto* t = Construct(s, 2.f, "a", 2.f);
WrapInFunction(t);
Global("a", ty.f32(), ast::StorageClass::kPrivate);
Global("b", ty.f32(), ast::StorageClass::kPrivate);
auto* t = Construct(s, 2.f, "a", 2.f);
WrapInFunction(t);
spirv::Builder& b = Build();
EXPECT_FALSE(b.is_constructor_const(t, false));

View File

@ -118,6 +118,10 @@ OpName %11 "main"
}
TEST_F(BuilderTest, Decoration_Stage_WithUsedInterfaceIds) {
auto* v_in = Global("my_in", ty.f32(), ast::StorageClass::kInput);
auto* v_out = Global("my_out", ty.f32(), ast::StorageClass::kOutput);
auto* v_wg = Global("my_wg", ty.f32(), ast::StorageClass::kWorkgroup);
auto* func =
Func("main", {}, ty.void_(),
ast::StatementList{
@ -130,10 +134,6 @@ TEST_F(BuilderTest, Decoration_Stage_WithUsedInterfaceIds) {
create<ast::StageDecoration>(ast::PipelineStage::kVertex),
});
auto* v_in = Global("my_in", ty.f32(), ast::StorageClass::kInput);
auto* v_out = Global("my_out", ty.f32(), ast::StorageClass::kOutput);
auto* v_wg = Global("my_wg", ty.f32(), ast::StorageClass::kWorkgroup);
spirv::Builder& b = Build();
EXPECT_TRUE(b.GenerateGlobalVariable(v_in)) << b.error();

View File

@ -260,14 +260,14 @@ TEST_F(WgslGeneratorImplTest, EmitType_Struct_WithEntryPointDecorations) {
ast::DecorationList decos;
decos.push_back(create<ast::StructBlockDecoration>());
auto* str = create<ast::Struct>(
auto* s = Structure(
"S",
ast::StructMemberList{
Member("a", ty.u32(),
{create<ast::BuiltinDecoration>(ast::Builtin::kVertexIndex)}),
Member("b", ty.f32(), {create<ast::LocationDecoration>(2u)})},
decos);
auto* s = ty.struct_("S", str);
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitStructType(s)) << gen.error();