From 02ee4d556c53327e10c00a1f9743c75d7abff4db Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Mon, 17 Aug 2026 03:05:35 +0300 Subject: [PATCH] Fix HIR lowering of params of trait assoc fns It turns out they should be treated as bare idents, not patterns, but only if they have no body. Also allow macro patterns for both them and extern fns, because rustc allows that. --- crates/hir-def/src/expr_store/lower.rs | 81 ++++++++++++++++---------- crates/hir-ty/src/tests/regression.rs | 44 +++++++++++++- 2 files changed, 92 insertions(+), 33 deletions(-) diff --git a/crates/hir-def/src/expr_store/lower.rs b/crates/hir-def/src/expr_store/lower.rs index 91faafaf843e..08cecb52570e 100644 --- a/crates/hir-def/src/expr_store/lower.rs +++ b/crates/hir-def/src/expr_store/lower.rs @@ -135,7 +135,9 @@ pub(super) fn lower_body( } collector.with_expr_root(|collector| { - if let Some(param_list) = parameters { + if let DefWithBodyId::FunctionId(func) = owner + && let Some(param_list) = parameters + { if let Some(self_param_syn) = param_list.self_param().filter(|it| collector.check_cfg(it)) { @@ -155,23 +157,28 @@ pub(super) fn lower_body( Some(collector.expander.in_file(AstPtr::new(&self_param_syn))); } - let is_extern = matches!( - owner, - DefWithBodyId::FunctionId(id) - if matches!(id.loc(db).container, ItemContainerId::ExternBlockId(_)), - ); + let params_are_bare_idents = match func.loc(db).container { + ItemContainerId::ExternBlockId(_) => true, + ItemContainerId::TraitId(_) => body.is_none(), + ItemContainerId::ModuleId(_) | ItemContainerId::ImplId(_) => false, + }; for param in param_list.params() { if collector.check_cfg(¶m) { - let param_pat = if is_extern { - collector.collect_extern_fn_param(param.pat()) - } else { - collector.collect_pat_top(param.pat()) + let param_pat = match param.pat() { + Some(pat) => { + if params_are_bare_idents { + collector.collect_param_as_ident(pat) + } else { + collector.collect_pat_top(Some(pat)) + } + } + None => collector.missing_pat(), }; params.push(Param::new(param_pat)); } } - }; + } collector.collect( &mut self_param, @@ -2792,11 +2799,11 @@ impl<'db> ExprCollector<'db> { } } - fn collect_extern_fn_param(&mut self, pat: Option) -> PatId { - // parameters of functions in `extern` blocks can only be simple identifiers and wildcards. + fn collect_param_as_ident(&mut self, pat: ast::Pat) -> PatId { + // parameters of functions in `extern` blocks and associated trait functions without a body + // can only be simple identifiers and wildcards. // Furthermore, the identifiers in their parameters are always interpreted as bindings, even // if in a normal function they won't be, because they would refer to a path pattern. - let Some(pat) = pat else { return self.missing_pat() }; match &pat { ast::Pat::IdentPat(bp) if bp.is_simple_ident() => { @@ -2812,6 +2819,8 @@ impl<'db> ExprCollector<'db> { pat } ast::Pat::WildcardPat(_) => self.alloc_pat(Pat::Wild, AstPtr::new(&pat)), + ast::Pat::MacroPat(mac) => self + .collect_macro_pat_with(mac.clone(), |this, pat| this.collect_param_as_ident(pat)), _ => { self.store.diagnostics.push(ExpressionStoreDiagnostics::PatternArgInExternFn { node: self.expander.in_file(AstPtr::new(&pat)), @@ -3017,19 +3026,11 @@ impl<'db> ExprCollector<'db> { Pat::Missing } } - ast::Pat::MacroPat(mac) => match mac.macro_call() { - Some(call) => { - let macro_ptr = AstPtr::new(&call); - let src = self.expander.in_file(AstPtr::new(&pat)); - let pat = - self.collect_macro_call(call, macro_ptr, true, |this, expanded_pat| { - this.collect_pat_opt(expanded_pat, binding_list) - }); - self.store.pat_map.insert(src, pat.into()); - return pat; - } - None => Pat::Missing, - }, + ast::Pat::MacroPat(mac) => { + return self.collect_macro_pat_with(mac.clone(), |this, expanded_pat| { + this.collect_pat(expanded_pat, binding_list) + }); + } ast::Pat::RangePat(p) => { let mut range_part_lower = |p: Option| -> Option { p.and_then(|it| { @@ -3068,6 +3069,28 @@ impl<'db> ExprCollector<'db> { self.alloc_pat(pattern, ptr) } + fn collect_macro_pat_with( + &mut self, + mac: ast::MacroPat, + callback: impl FnOnce(&mut Self, ast::Pat) -> PatId, + ) -> PatId { + match mac.macro_call() { + Some(call) => { + let macro_ptr = AstPtr::new(&call); + let src = self.expander.in_file(AstPtr::new(&mac.into())); + let pat = self.collect_macro_call(call, macro_ptr, true, |this, expanded_pat| { + match expanded_pat { + Some(pat) => callback(this, pat), + None => this.missing_pat(), + } + }); + self.store.pat_map.insert(src, pat.into()); + pat + } + None => self.missing_pat(), + } + } + fn collect_pat_opt(&mut self, pat: Option, binding_list: &mut BindingList) -> PatId { match pat { Some(pat) => self.collect_pat(pat, binding_list), @@ -3172,9 +3195,7 @@ impl<'db> ExprCollector<'db> { ) } ast::Pat::MacroPat(pat) => { - let Some(call) = pat.macro_call() else { return self.missing_pat() }; - let ptr = AstPtr::new(&call); - self.collect_macro_call(call, ptr, true, |this, pat| this.collect_ty_pat_opt(pat)) + self.collect_macro_pat_with(pat, |this, pat| this.collect_ty_pat(pat)) } _ => { // FIXME: Emit an error. diff --git a/crates/hir-ty/src/tests/regression.rs b/crates/hir-ty/src/tests/regression.rs index cdc0a8ab9e36..ba418558e913 100644 --- a/crates/hir-ty/src/tests/regression.rs +++ b/crates/hir-ty/src/tests/regression.rs @@ -2796,10 +2796,48 @@ where fn extern_fns_cannot_have_param_patterns() { check_no_mismatches( r#" -pub(crate) struct Builder<'a>(&'a ()); +macro_rules! m { + () => { Builder }; +} + +pub(crate) struct Builder; + +unsafe extern "C" { + pub(crate) fn foo(Builder: (), m!(): ()); +} + "#, + ); +} + +#[test] +fn trait_assoc_fns_cannot_have_param_patterns() { + check_no_mismatches( + r#" +macro_rules! m { + () => { Builder }; +} -unsafe extern "C" { - pub(crate) fn foo<'a>(Builder: &Builder<'a>); +pub(crate) struct Builder; + +trait Trait { + fn foo(Builder: (), m!(): ()); +} + "#, + ); + // But assoc fns with bodies do have patterns: + check( + r#" +macro_rules! m { + () => { Builder }; +} + +pub(crate) struct Builder; + +trait Trait { + fn foo(Builder: (), + // ^^^^^^^ expected (), got Builder + m!(): ()) {} + // ^^ expected (), got Builder } "#, );