From 511529f082daa0ae7f803024dfbce1ba1e0e9fe3 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Sun, 26 Jun 2022 09:05:00 +0000 Subject: [PATCH] tint/resolver: Clean up workgroup-size error diagnostic Change-Id: I09cc3e15c1f62d7a104e9727a104f3b2bfc9c973 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/94607 Reviewed-by: David Neto Commit-Queue: Ben Clayton --- src/tint/resolver/function_validation_test.cc | 44 +++++++++---------- src/tint/resolver/resolver.cc | 15 +++---- 2 files changed, 28 insertions(+), 31 deletions(-) diff --git a/src/tint/resolver/function_validation_test.cc b/src/tint/resolver/function_validation_test.cc index 0734dd947d..3326d11971 100644 --- a/src/tint/resolver/function_validation_test.cc +++ b/src/tint/resolver/function_validation_test.cc @@ -241,8 +241,8 @@ TEST_F(ResolverFunctionValidationTest, VoidFunctionReturnsAInt) { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: return statement type must match its function return " - "type, returned 'abstract-int', expected 'void'"); + "12:34 error: return statement type must match its function return type, returned " + "'abstract-int', expected 'void'"); } TEST_F(ResolverFunctionValidationTest, VoidFunctionReturnsAFloat) { @@ -251,8 +251,8 @@ TEST_F(ResolverFunctionValidationTest, VoidFunctionReturnsAFloat) { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: return statement type must match its function return " - "type, returned 'abstract-float', expected 'void'"); + "12:34 error: return statement type must match its function return type, returned " + "'abstract-float', expected 'void'"); } TEST_F(ResolverFunctionValidationTest, VoidFunctionReturnsI32) { @@ -261,8 +261,8 @@ TEST_F(ResolverFunctionValidationTest, VoidFunctionReturnsI32) { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: return statement type must match its function return " - "type, returned 'i32', expected 'void'"); + "12:34 error: return statement type must match its function return type, returned " + "'i32', expected 'void'"); } TEST_F(ResolverFunctionValidationTest, FunctionTypeMustMatchReturnStatementType_void_fail) { @@ -287,8 +287,8 @@ TEST_F(ResolverFunctionValidationTest, FunctionTypeMustMatchReturnStatementTypeM EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: return statement type must match its function return " - "type, returned 'void', expected 'f32'"); + "12:34 error: return statement type must match its function return type, returned " + "'void', expected 'f32'"); } TEST_F(ResolverFunctionValidationTest, FunctionTypeMustMatchReturnStatementTypeF32_pass) { @@ -310,8 +310,8 @@ TEST_F(ResolverFunctionValidationTest, FunctionTypeMustMatchReturnStatementTypeF EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: return statement type must match its function return " - "type, returned 'i32', expected 'f32'"); + "12:34 error: return statement type must match its function return type, returned " + "'i32', expected 'f32'"); } TEST_F(ResolverFunctionValidationTest, FunctionTypeMustMatchReturnStatementTypeF32Alias_pass) { @@ -337,8 +337,8 @@ TEST_F(ResolverFunctionValidationTest, FunctionTypeMustMatchReturnStatementTypeF EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: return statement type must match its function return " - "type, returned 'u32', expected 'f32'"); + "12:34 error: return statement type must match its function return type, returned " + "'u32', expected 'f32'"); } TEST_F(ResolverFunctionValidationTest, CannotCallEntryPoint) { @@ -638,8 +638,8 @@ TEST_F(ResolverFunctionValidationTest, WorkgroupSize_Literal_BadType) { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: workgroup_size argument must be either literal or " - "module-scope constant of type i32 or u32"); + "12:34 error: workgroup_size argument must be either a literal, constant, or " + "overridable of type abstract-integer, i32 or u32"); } TEST_F(ResolverFunctionValidationTest, WorkgroupSize_Literal_Negative) { @@ -674,8 +674,8 @@ TEST_F(ResolverFunctionValidationTest, WorkgroupSize_Const_BadType) { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: workgroup_size argument must be either literal or " - "module-scope constant of type i32 or u32"); + "12:34 error: workgroup_size argument must be either a literal, constant, or " + "overridable of type abstract-integer, i32 or u32"); } // TODO(crbug.com/tint/1580): Remove when module-scope 'let' is removed @@ -689,8 +689,8 @@ TEST_F(ResolverFunctionValidationTest, WorkgroupSize_Let_BadType) { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: workgroup_size argument must be either literal or " - "module-scope constant of type i32 or u32"); + "12:34 error: workgroup_size argument must be either a literal, constant, or " + "overridable of type abstract-integer, i32 or u32"); } TEST_F(ResolverFunctionValidationTest, WorkgroupSize_Const_Negative) { @@ -778,8 +778,8 @@ TEST_F(ResolverFunctionValidationTest, WorkgroupSize_NonConst) { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: workgroup_size argument must be either literal or " - "module-scope constant of type i32 or u32"); + "12:34 error: workgroup_size argument must be either a literal, constant, or " + "overridable of type abstract-integer, i32 or u32"); } TEST_F(ResolverFunctionValidationTest, WorkgroupSize_InvalidExpr) { @@ -791,8 +791,8 @@ TEST_F(ResolverFunctionValidationTest, WorkgroupSize_InvalidExpr) { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: workgroup_size argument must be either a literal or " - "a module-scope constant"); + "12:34 error: workgroup_size argument must be either a literal, constant, or " + "overridable of type abstract-integer, i32 or u32"); } TEST_F(ResolverFunctionValidationTest, ReturnIsConstructible_NonPlain) { diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc index b6e5e5b61d..76aaea285b 100644 --- a/src/tint/resolver/resolver.cc +++ b/src/tint/resolver/resolver.cc @@ -885,9 +885,9 @@ bool Resolver::WorkgroupSize(const ast::Function* func) { std::array arg_tys = {}; size_t arg_count = 0; - constexpr const char* kErrBadType = - "workgroup_size argument must be either literal or module-scope constant of type i32 " - "or u32"; + constexpr const char* kErrBadExpr = + "workgroup_size argument must be either a literal, constant, or overridable of type " + "abstract-integer, i32 or u32"; for (int i = 0; i < 3; i++) { // Each argument to this attribute can either be a literal, an identifier for a module-scope @@ -902,7 +902,7 @@ bool Resolver::WorkgroupSize(const ast::Function* func) { } auto* ty = expr->Type(); if (!ty->IsAnyOf()) { - AddError(kErrBadType, value->source); + AddError(kErrBadExpr, value->source); return false; } @@ -935,7 +935,7 @@ bool Resolver::WorkgroupSize(const ast::Function* func) { // We have an variable of a module-scope constant. auto* decl = user->Variable()->Declaration(); if (!decl->IsAnyOf()) { - AddError(kErrBadType, values[i]->source); + AddError(kErrBadExpr, values[i]->source); return false; } // Capture the constant if it is pipeline-overridable. @@ -953,10 +953,7 @@ bool Resolver::WorkgroupSize(const ast::Function* func) { } else if (values[i]->Is()) { value = materialized->ConstantValue(); } else { - AddError( - "workgroup_size argument must be either a literal or a " - "module-scope constant", - values[i]->source); + AddError(kErrBadExpr, values[i]->source); return false; }