From f10a57908aeaccbe5f3348183752dc2519e40b39 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Thu, 13 Oct 2022 13:47:39 +0000 Subject: [PATCH] tint: Use `const-expression` and `override-expression` terms Fixed: tint:1601 Change-Id: I72827df7c83dbb8f5dc69a8803fbe955b1a2421d Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/105622 Auto-Submit: Ben Clayton Kokoro: Kokoro Reviewed-by: Dan Sinclair Commit-Queue: Dan Sinclair --- src/tint/resolver/builtin_test.cc | 2 +- src/tint/resolver/builtin_validation_test.cc | 2 +- src/tint/resolver/const_eval.h | 2 +- src/tint/resolver/function_validation_test.cc | 12 ++++++------ src/tint/resolver/resolver.cc | 14 +++++++------- src/tint/resolver/static_assert_test.cc | 2 +- src/tint/resolver/validator.cc | 8 ++++---- src/tint/resolver/variable_validation_test.cc | 6 +++--- src/tint/sem/array.h | 6 +++--- src/tint/sem/function.h | 5 +++-- src/tint/transform/substitute_override.cc | 2 +- src/tint/writer/glsl/generator_impl.cc | 4 ++-- .../writer/glsl/generator_impl_function_test.cc | 2 +- src/tint/writer/hlsl/generator_impl.cc | 4 ++-- .../writer/hlsl/generator_impl_function_test.cc | 2 +- src/tint/writer/msl/generator_impl.cc | 2 +- src/tint/writer/spirv/builder.cc | 2 +- .../spirv/builder_function_attribute_test.cc | 4 ++-- 18 files changed, 41 insertions(+), 40 deletions(-) diff --git a/src/tint/resolver/builtin_test.cc b/src/tint/resolver/builtin_test.cc index 0883aed0ed..2917f4d918 100644 --- a/src/tint/resolver/builtin_test.cc +++ b/src/tint/resolver/builtin_test.cc @@ -67,7 +67,7 @@ TEST_F(ResolverBuiltinTest, ModuleScopeUsage) { // TODO(crbug.com/tint/1581): Once 'abs' is implemented as @const, this will no longer be an // error. - EXPECT_EQ(r()->error(), R"(12:34 error: 'const' initializer must be constant expression)"); + EXPECT_EQ(r()->error(), R"(12:34 error: 'const' initializer must be const-expression)"); } // Tests for Logical builtins diff --git a/src/tint/resolver/builtin_validation_test.cc b/src/tint/resolver/builtin_validation_test.cc index 6ba48de999..95a4dbbb94 100644 --- a/src/tint/resolver/builtin_validation_test.cc +++ b/src/tint/resolver/builtin_validation_test.cc @@ -327,7 +327,7 @@ TEST_P(BuiltinTextureConstExprArgValidationTest, GlobalConst) { EXPECT_FALSE(r()->Resolve()); std::stringstream err; - err << "12:34 error: the " << param.name << " argument must be a const_expression"; + err << "12:34 error: the " << param.name << " argument must be a const-expression"; EXPECT_EQ(r()->error(), err.str()); } INSTANTIATE_TEST_SUITE_P( diff --git a/src/tint/resolver/const_eval.h b/src/tint/resolver/const_eval.h index b58ed41ba1..065cca5427 100644 --- a/src/tint/resolver/const_eval.h +++ b/src/tint/resolver/const_eval.h @@ -38,7 +38,7 @@ class Type; namespace tint::resolver { -/// ConstEval performs shader creation-time (constant expression) expression evaluation. +/// ConstEval performs shader creation-time (const-expression) expression evaluation. /// Methods are called from the resolver, either directly or via member-function pointers indexed by /// the IntrinsicTable. All child-expression nodes are guaranteed to have been already resolved /// before calling a method to evaluate an expression's value. diff --git a/src/tint/resolver/function_validation_test.cc b/src/tint/resolver/function_validation_test.cc index 107fe64cc5..9fb0da42a2 100644 --- a/src/tint/resolver/function_validation_test.cc +++ b/src/tint/resolver/function_validation_test.cc @@ -672,7 +672,7 @@ TEST_F(ResolverFunctionValidationTest, WorkgroupSize_Literal_BadType) { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ( r()->error(), - "12:34 error: workgroup_size argument must be a constant or override expression of type " + "12:34 error: workgroup_size argument must be a constant or override-expression of type " "abstract-integer, i32 or u32"); } @@ -718,7 +718,7 @@ TEST_F(ResolverFunctionValidationTest, WorkgroupSize_Const_BadType) { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ( r()->error(), - "12:34 error: workgroup_size argument must be a constant or override expression of type " + "12:34 error: workgroup_size argument must be a constant or override-expression of type " "abstract-integer, i32 or u32"); } @@ -851,7 +851,7 @@ TEST_F(ResolverFunctionValidationTest, WorkgroupSize_NonConst) { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: workgroup_size argument must be a constant or override expression of " + "12:34 error: workgroup_size argument must be a constant or override-expression of " "type abstract-integer, i32 or u32"); } @@ -866,7 +866,7 @@ TEST_F(ResolverFunctionValidationTest, WorkgroupSize_InvalidExpr_x) { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: workgroup_size argument must be a constant or override expression of " + "12:34 error: workgroup_size argument must be a constant or override-expression of " "type abstract-integer, i32 or u32"); } @@ -881,7 +881,7 @@ TEST_F(ResolverFunctionValidationTest, WorkgroupSize_InvalidExpr_y) { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: workgroup_size argument must be a constant or override expression of " + "12:34 error: workgroup_size argument must be a constant or override-expression of " "type abstract-integer, i32 or u32"); } @@ -896,7 +896,7 @@ TEST_F(ResolverFunctionValidationTest, WorkgroupSize_InvalidExpr_z) { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: workgroup_size argument must be a constant or override expression of " + "12:34 error: workgroup_size argument must be a constant or override-expression of " "type abstract-integer, i32 or u32"); } diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc index bc87b9993a..85adaf8246 100644 --- a/src/tint/resolver/resolver.cc +++ b/src/tint/resolver/resolver.cc @@ -511,7 +511,7 @@ sem::Variable* Resolver::Const(const ast::Const* c, bool is_global) { const auto value = rhs->ConstantValue(); if (!value) { - AddError("'const' initializer must be constant expression", c->constructor->source); + AddError("'const' initializer must be const-expression", c->constructor->source); return nullptr; } @@ -864,7 +864,7 @@ sem::Statement* Resolver::StaticAssert(const ast::StaticAssert* assertion) { } auto* cond = expr->ConstantValue(); if (!cond) { - AddError("static assertion condition must be a constant expression", + AddError("static assertion condition must be a const-expression", assertion->condition->source); return nullptr; } @@ -1068,12 +1068,12 @@ bool Resolver::WorkgroupSize(const ast::Function* func) { utils::Vector arg_tys; constexpr const char* kErrBadExpr = - "workgroup_size argument must be a constant or override expression of type " + "workgroup_size argument must be a constant or override-expression of type " "abstract-integer, i32 or u32"; for (size_t i = 0; i < 3; i++) { // Each argument to this attribute can either be a literal, an identifier for a module-scope - // constants, a constant expression, or nullptr if not specified. + // constants, a const-expression, or nullptr if not specified. auto* value = values[i]; if (!value) { break; @@ -2824,7 +2824,7 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) { } auto const_value = materialized->ConstantValue(); if (!const_value) { - AddError("'offset' must be constant expression", o->expr->source); + AddError("'offset' must be const-expression", o->expr->source); return nullptr; } offset = const_value->As(); @@ -2847,7 +2847,7 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) { auto const_value = materialized->ConstantValue(); if (!const_value) { - AddError("'align' must be constant expression", a->source); + AddError("'align' must be const-expression", a->source); return nullptr; } auto value = const_value->As(); @@ -2865,7 +2865,7 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) { } auto const_value = materialized->ConstantValue(); if (!const_value) { - AddError("'size' must be constant expression", s->expr->source); + AddError("'size' must be const-expression", s->expr->source); return nullptr; } auto value = const_value->As(); diff --git a/src/tint/resolver/static_assert_test.cc b/src/tint/resolver/static_assert_test.cc index ea7396d91e..6d8423dbe0 100644 --- a/src/tint/resolver/static_assert_test.cc +++ b/src/tint/resolver/static_assert_test.cc @@ -88,7 +88,7 @@ TEST_F(ResolverStaticAssertTest, Local_NonConst) { WrapInFunction(StaticAssert(Expr(Source{{12, 34}}, "V"))); EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: static assertion condition must be a constant expression"); + "12:34 error: static assertion condition must be a const-expression"); } TEST_F(ResolverStaticAssertTest, Local_LessThan_Pass) { diff --git a/src/tint/resolver/validator.cc b/src/tint/resolver/validator.cc index 3f58e855f1..fccb50f843 100644 --- a/src/tint/resolver/validator.cc +++ b/src/tint/resolver/validator.cc @@ -593,7 +593,7 @@ bool Validator::GlobalVariable( [&](const ast::Var* var) { if (auto* init = global->Constructor(); init && init->Stage() > sem::EvaluationStage::kOverride) { - AddError("module-scope 'var' initializer must be a constant or override expression", + AddError("module-scope 'var' initializer must be a constant or override-expression", init->Declaration()->source); return false; } @@ -795,7 +795,7 @@ bool Validator::Override( auto* storage_ty = v->Type()->UnwrapRef(); if (auto* init = v->Constructor(); init && init->Stage() > sem::EvaluationStage::kOverride) { - AddError("'override' initializer must be an override expression", + AddError("'override' initializer must be an override-expression", init->Declaration()->source); return false; } @@ -1708,7 +1708,7 @@ bool Validator::TextureBuiltinFunction(const sem::Call* call) const { return true; } } - AddError("the " + name + " argument must be a const_expression", + AddError("the " + name + " argument must be a const-expression", arg->Declaration()->source); return false; }; @@ -1902,7 +1902,7 @@ bool Validator::ArrayConstructor(const ast::CallExpression* ctor, } if (array_type->IsOverrideSized()) { - AddError("cannot construct an array that has an override expression count", ctor->source); + AddError("cannot construct an array that has an override-expression count", ctor->source); return false; } diff --git a/src/tint/resolver/variable_validation_test.cc b/src/tint/resolver/variable_validation_test.cc index 72d6a6d5ee..b1c7bac205 100644 --- a/src/tint/resolver/variable_validation_test.cc +++ b/src/tint/resolver/variable_validation_test.cc @@ -423,7 +423,7 @@ TEST_F(ResolverVariableValidationTest, ConstInitWithVar) { WrapInFunction(v, c); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), R"(12:34 error: 'const' initializer must be constant expression)"); + EXPECT_EQ(r()->error(), R"(12:34 error: 'const' initializer must be const-expression)"); } TEST_F(ResolverVariableValidationTest, ConstInitWithOverride) { @@ -432,7 +432,7 @@ TEST_F(ResolverVariableValidationTest, ConstInitWithOverride) { WrapInFunction(c); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), R"(12:34 error: 'const' initializer must be constant expression)"); + EXPECT_EQ(r()->error(), R"(12:34 error: 'const' initializer must be const-expression)"); } TEST_F(ResolverVariableValidationTest, ConstInitWithLet) { @@ -441,7 +441,7 @@ TEST_F(ResolverVariableValidationTest, ConstInitWithLet) { WrapInFunction(l, c); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), R"(12:34 error: 'const' initializer must be constant expression)"); + EXPECT_EQ(r()->error(), R"(12:34 error: 'const' initializer must be const-expression)"); } } // namespace diff --git a/src/tint/sem/array.h b/src/tint/sem/array.h index d41cd98156..693549d9cd 100644 --- a/src/tint/sem/array.h +++ b/src/tint/sem/array.h @@ -31,7 +31,7 @@ class GlobalVariable; namespace tint::sem { -/// The variant of an ArrayCount when the array is a constant expression. +/// The variant of an ArrayCount when the array is a const-expression. /// Example: /// ``` /// const N = 123; @@ -144,7 +144,7 @@ class Array final : public Castable { /// @returns the number of elements in the array. const ArrayCount& Count() const { return count_; } - /// @returns the array count if the count is a constant expression, otherwise returns nullopt. + /// @returns the array count if the count is a const-expression, otherwise returns nullopt. inline std::optional ConstantCount() const { if (auto* count = std::get_if(&count_)) { return count->value; @@ -175,7 +175,7 @@ class Array final : public Castable { /// natural stride bool IsStrideImplicit() const { return stride_ == implicit_stride_; } - /// @returns true if this array is sized using an constant expression + /// @returns true if this array is sized using an const-expression bool IsConstantSized() const { return std::holds_alternative(count_); } /// @returns true if this array is sized using an override variable diff --git a/src/tint/sem/function.h b/src/tint/sem/function.h index 50d853c73f..9ef99ab6d5 100644 --- a/src/tint/sem/function.h +++ b/src/tint/sem/function.h @@ -40,8 +40,9 @@ class Variable; namespace tint::sem { /// WorkgroupSize is a three-dimensional array of WorkgroupDimensions. -/// Each dimension is a std::optional as a workgroup size can be a constant or override expression. -/// Override expressions are not known at compilation time, so these will be std::nullopt. +/// Each dimension is a std::optional as a workgroup size can be a const-expression or +/// override-expression. Override expressions are not known at compilation time, so these will be +/// std::nullopt. using WorkgroupSize = std::array, 3>; /// Function holds the semantic information for function nodes. diff --git a/src/tint/transform/substitute_override.cc b/src/tint/transform/substitute_override.cc index c189490248..bdaa45b8a4 100644 --- a/src/tint/transform/substitute_override.cc +++ b/src/tint/transform/substitute_override.cc @@ -77,7 +77,7 @@ void SubstituteOverride::Run(CloneContext& ctx, const DataMap& config, DataMap&) if (!ctor) { ctx.dst->Diagnostics().add_error(diag::System::Transform, - "Failed to create override expression"); + "Failed to create override-expression"); return nullptr; } diff --git a/src/tint/writer/glsl/generator_impl.cc b/src/tint/writer/glsl/generator_impl.cc index 25c094bc65..49da7d6686 100644 --- a/src/tint/writer/glsl/generator_impl.cc +++ b/src/tint/writer/glsl/generator_impl.cc @@ -1887,7 +1887,7 @@ bool GeneratorImpl::EmitGlobalVariable(const ast::Variable* global) { [&](const ast::Override*) { // Override is removed with SubstituteOverride diagnostics_.add_error(diag::System::Writer, - "override expressions should have been removed with the " + "override-expressions should have been removed with the " "SubstituteOverride transform"); return false; }, @@ -2109,7 +2109,7 @@ bool GeneratorImpl::EmitEntryPointFunction(const ast::Function* func) { if (!wgsize[i].has_value()) { diagnostics_.add_error( diag::System::Writer, - "override expressions should have been removed with the SubstituteOverride " + "override-expressions should have been removed with the SubstituteOverride " "transform"); return false; } diff --git a/src/tint/writer/glsl/generator_impl_function_test.cc b/src/tint/writer/glsl/generator_impl_function_test.cc index 13579859e3..56beefc8f8 100644 --- a/src/tint/writer/glsl/generator_impl_function_test.cc +++ b/src/tint/writer/glsl/generator_impl_function_test.cc @@ -799,7 +799,7 @@ TEST_F(GlslGeneratorImplTest_Function, EXPECT_FALSE(gen.Generate()) << gen.error(); EXPECT_EQ( gen.error(), - R"(error: override expressions should have been removed with the SubstituteOverride transform)"); + R"(error: override-expressions should have been removed with the SubstituteOverride transform)"); } TEST_F(GlslGeneratorImplTest_Function, Emit_Function_WithArrayParams) { diff --git a/src/tint/writer/hlsl/generator_impl.cc b/src/tint/writer/hlsl/generator_impl.cc index b683e1abd3..a51b954355 100644 --- a/src/tint/writer/hlsl/generator_impl.cc +++ b/src/tint/writer/hlsl/generator_impl.cc @@ -2837,7 +2837,7 @@ bool GeneratorImpl::EmitGlobalVariable(const ast::Variable* global) { [&](const ast::Override*) { // Override is removed with SubstituteOverride diagnostics_.add_error(diag::System::Writer, - "override expressions should have been removed with the " + "override-expressions should have been removed with the " "SubstituteOverride transform"); return false; }, @@ -3044,7 +3044,7 @@ bool GeneratorImpl::EmitEntryPointFunction(const ast::Function* func) { if (!wgsize[i].has_value()) { diagnostics_.add_error( diag::System::Writer, - "override expressions should have been removed with the SubstituteOverride " + "override-expressions should have been removed with the SubstituteOverride " "transform"); return false; } diff --git a/src/tint/writer/hlsl/generator_impl_function_test.cc b/src/tint/writer/hlsl/generator_impl_function_test.cc index 1adb8522a9..56465d30a5 100644 --- a/src/tint/writer/hlsl/generator_impl_function_test.cc +++ b/src/tint/writer/hlsl/generator_impl_function_test.cc @@ -728,7 +728,7 @@ TEST_F(HlslGeneratorImplTest_Function, EXPECT_FALSE(gen.Generate()) << gen.error(); EXPECT_EQ( gen.error(), - R"(error: override expressions should have been removed with the SubstituteOverride transform)"); + R"(error: override-expressions should have been removed with the SubstituteOverride transform)"); } TEST_F(HlslGeneratorImplTest_Function, Emit_Function_WithArrayParams) { diff --git a/src/tint/writer/msl/generator_impl.cc b/src/tint/writer/msl/generator_impl.cc index 57d5bf089c..792b5b5a35 100644 --- a/src/tint/writer/msl/generator_impl.cc +++ b/src/tint/writer/msl/generator_impl.cc @@ -301,7 +301,7 @@ bool GeneratorImpl::Generate() { [&](const ast::Override*) { // Override is removed with SubstituteOverride diagnostics_.add_error(diag::System::Writer, - "override expressions should have been removed with the " + "override-expressions should have been removed with the " "SubstituteOverride transform."); return false; }, diff --git a/src/tint/writer/spirv/builder.cc b/src/tint/writer/spirv/builder.cc index ab88ca5df0..83587f1853 100644 --- a/src/tint/writer/spirv/builder.cc +++ b/src/tint/writer/spirv/builder.cc @@ -509,7 +509,7 @@ bool Builder::GenerateExecutionModes(const ast::Function* func, uint32_t id) { // Check if the workgroup_size uses pipeline-overridable constants. if (!wgsize[0].has_value() || !wgsize[1].has_value() || !wgsize[2].has_value()) { error_ = - "override expressions should have been removed with the SubstituteOverride " + "override-expressions should have been removed with the SubstituteOverride " "transform"; return false; } diff --git a/src/tint/writer/spirv/builder_function_attribute_test.cc b/src/tint/writer/spirv/builder_function_attribute_test.cc index 60bf062a29..b905d8c9ae 100644 --- a/src/tint/writer/spirv/builder_function_attribute_test.cc +++ b/src/tint/writer/spirv/builder_function_attribute_test.cc @@ -164,7 +164,7 @@ TEST_F(BuilderTest, Decoration_ExecutionMode_WorkgroupSize_OverridableConst) { EXPECT_FALSE(b.GenerateExecutionModes(func, 3)) << b.error(); EXPECT_EQ( b.error(), - R"(override expressions should have been removed with the SubstituteOverride transform)"); + R"(override-expressions should have been removed with the SubstituteOverride transform)"); } TEST_F(BuilderTest, Decoration_ExecutionMode_WorkgroupSize_LiteralAndConst) { @@ -181,7 +181,7 @@ TEST_F(BuilderTest, Decoration_ExecutionMode_WorkgroupSize_LiteralAndConst) { EXPECT_FALSE(b.GenerateExecutionModes(func, 3)) << b.error(); EXPECT_EQ( b.error(), - R"(override expressions should have been removed with the SubstituteOverride transform)"); + R"(override-expressions should have been removed with the SubstituteOverride transform)"); } TEST_F(BuilderTest, Decoration_ExecutionMode_MultipleFragment) {