From 97382d96f5b21e17be317ca048b69dfc11be33ce Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Tue, 14 Mar 2023 11:32:54 +0000 Subject: [PATCH] tint: Validate no template args with functions / builtin calls This was already handled when resolving regular identifiers, but calls special case this resolving, and failed to check for template arguments. Bug: chromium:1424273 Change-Id: Id756c7fbca93afcd9fd3792466471aa43d3dff04 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/123980 Kokoro: Kokoro Commit-Queue: Ben Clayton Reviewed-by: Corentin Wallez --- src/tint/resolver/call_validation_test.cc | 29 ++++++++++++++ src/tint/resolver/resolver.cc | 47 +++++++++++++---------- src/tint/resolver/resolver.h | 5 +++ 3 files changed, 61 insertions(+), 20 deletions(-) diff --git a/src/tint/resolver/call_validation_test.cc b/src/tint/resolver/call_validation_test.cc index b749df9e2c..b00db681d4 100644 --- a/src/tint/resolver/call_validation_test.cc +++ b/src/tint/resolver/call_validation_test.cc @@ -478,5 +478,34 @@ TEST_F(ResolverCallValidationTest, MustUseBuiltin) { EXPECT_EQ(r()->error(), "12:34 error: ignoring return value of builtin 'max'"); } +TEST_F(ResolverCallValidationTest, UnexpectedFunctionTemplateArgs) { + // fn a() {} + // fn b() { + // a(); + // } + Func(Source{{56, 78}}, "a", utils::Empty, ty.void_(), utils::Empty); + Func("b", utils::Empty, ty.void_(), + utils::Vector{ + CallStmt(Call(Ident(Source{{12, 34}}, "a", "i32"))), + }); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), R"(12:34 error: function 'a' does not take template arguments +56:78 note: function 'a' declared here)"); +} + +TEST_F(ResolverCallValidationTest, UnexpectedBuiltinTemplateArgs) { + // fn f() { + // min(1, 2); + // } + Func("f", utils::Empty, ty.void_(), + utils::Vector{ + Decl(Var("v", Call(Ident(Source{{12, 34}}, "min", "i32"), 1_a, 2_a))), + }); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), R"(12:34 error: builtin 'min' does not take template arguments)"); +} + } // namespace } // namespace tint::resolver diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc index 74b32bbbce..f6f1876ae1 100644 --- a/src/tint/resolver/resolver.cc +++ b/src/tint/resolver/resolver.cc @@ -2156,7 +2156,12 @@ sem::Call* Resolver::Call(const ast::CallExpression* expr) { return Switch( sem_.Get(ast_node), // [&](type::Type* t) { return ty_init_or_conv(t); }, - [&](sem::Function* f) { return FunctionCall(expr, f, args, arg_behaviors); }, + [&](sem::Function* f) -> sem::Call* { + if (!TINT_LIKELY(CheckNotTemplated("function", ident))) { + return nullptr; + } + return FunctionCall(expr, f, args, arg_behaviors); + }, [&](sem::Expression* e) { sem_.ErrorUnexpectedExprKind(e, "call target"); return nullptr; @@ -2168,6 +2173,9 @@ sem::Call* Resolver::Call(const ast::CallExpression* expr) { } if (auto f = resolved->BuiltinFunction(); f != builtin::Function::kNone) { + if (!TINT_LIKELY(CheckNotTemplated("builtin", ident))) { + return nullptr; + } return BuiltinCall(expr, f, args); } @@ -2331,13 +2339,7 @@ type::Type* Resolver::BuiltinType(builtin::Builtin builtin_ty, const ast::Identi auto& b = *builder_; auto check_no_tmpl_args = [&](type::Type* ty) -> type::Type* { - if (TINT_UNLIKELY(ident->Is())) { - AddError("type '" + b.Symbols().NameFor(ident->symbol) + - "' does not take template arguments", - ident->source); - return nullptr; - } - return ty; + return TINT_LIKELY(CheckNotTemplated("type", ident)) ? ty : nullptr; }; auto f32 = [&] { return b.create(); }; auto i32 = [&] { return b.create(); }; @@ -3019,25 +3021,15 @@ sem::Expression* Resolver::Identifier(const ast::IdentifierExpression* expr) { return user; }, [&](const type::Type* ty) -> sem::TypeExpression* { - if (TINT_UNLIKELY(ident->Is())) { - AddError("type '" + builder_->Symbols().NameFor(ident->symbol) + - "' does not take template arguments", - ident->source); - sem_.NoteDeclarationSource(ast_node); + if (!TINT_LIKELY(CheckNotTemplated("type", ident))) { return nullptr; } - return builder_->create(expr, current_statement_, ty); }, [&](const sem::Function* fn) -> sem::FunctionExpression* { - if (TINT_UNLIKELY(ident->Is())) { - AddError("function '" + builder_->Symbols().NameFor(ident->symbol) + - "' does not take template arguments", - ident->source); - sem_.NoteDeclarationSource(ast_node); + if (!TINT_LIKELY(CheckNotTemplated("function", ident))) { return nullptr; } - return builder_->create(expr, current_statement_, fn); }); } @@ -4329,6 +4321,21 @@ void Resolver::ApplyDiagnosticSeverities(NODE* node) { } } +bool Resolver::CheckNotTemplated(const char* use, const ast::Identifier* ident) { + if (TINT_UNLIKELY(ident->Is())) { + AddError(std::string(use) + " '" + builder_->Symbols().NameFor(ident->symbol) + + "' does not take template arguments", + ident->source); + if (auto resolved = dependencies_.resolved_identifiers.Get(ident)) { + if (auto* ast_node = resolved->Node()) { + sem_.NoteDeclarationSource(ast_node); + } + } + return false; + } + return true; +} + void Resolver::ErrorMismatchedResolvedIdentifier(const Source& source, const ResolvedIdentifier& resolved, std::string_view wanted) { diff --git a/src/tint/resolver/resolver.h b/src/tint/resolver/resolver.h index cab7b37aaa..266f3a2dd9 100644 --- a/src/tint/resolver/resolver.h +++ b/src/tint/resolver/resolver.h @@ -480,6 +480,11 @@ class Resolver { template void ApplyDiagnosticSeverities(NODE* node); + /// Checks @p ident is not an ast::TemplatedIdentifier. + /// If @p ident is a ast::TemplatedIdentifier, then an error diagnostic is raised. + /// @returns true if @p ident is not a ast::TemplatedIdentifier. + bool CheckNotTemplated(const char* use, const ast::Identifier* ident); + /// Raises an error diagnostic that the resolved identifier @p resolved was not of the expected /// kind. /// @param source the source of the error diagnostic