Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #17586 - ShoyuVanilla:tuple-arg-macro-rest, r=Veykril
Allow macro expansions into `RestPat` in tuple args work as ellipsis like plain `RestPat`
Fixes #17292
Currently, Rust Analyzer lowers `ast::Pat::RestPat` into `Pat::Missing` in general cases on the following lines;
https://github.com/rust-lang/rust-analyzer/blob/ffbc5ad993d5cd2f3b8bcf9a511165470944ab91/crates/hir-def/src/body/lower.rs#L1359-L1367
And in some proper positions such as `TupleStruct(..)`, it is specially handed on the following lines;
https://github.com/rust-lang/rust-analyzer/blob/ffbc5ad993d5cd2f3b8bcf9a511165470944ab91/crates/hir-def/src/body/lower.rs#L1429-L1437
This behavior is reasonable because rustc does similar things in
https://github.com/rust-lang/rust/blob/62c068feeafd1f4abbf87243d69cf8862e4dd277/compiler/rustc_ast_lowering/src/pat.rs#L108-L111
and
https://github.com/rust-lang/rust/blob/62c068feeafd1f4abbf87243d69cf8862e4dd277/compiler/rustc_ast_lowering/src/pat.rs#L123-L142
But this sometimes works differently because Rust Analyzer expands macros while ast lowering;
https://github.com/rust-lang/rust-analyzer/blob/ffbc5ad993d5cd2f3b8bcf9a511165470944ab91/crates/hir-def/src/body/lower.rs#L1386-L1398
https://github.com/rust-lang/rust-analyzer/blob/ffbc5ad993d5cd2f3b8bcf9a511165470944ab91/crates/hir-def/src/body/lower.rs#L941-L963
but rustc uses expanded ast in the corresponding tuple-handling process, so it does not have macro patterns there.
https://github.com/rust-lang/rust/blob/62c068feeafd1f4abbf87243d69cf8862e4dd277/compiler/rustc_ast_lowering/src/pat.rs#L114
So, if a macro expansion in a tuple arg results in `..`, rustc permits it like plain `..` pattern, but Rust Analyzer rejects it.
This is the root cause of #17292 and this PR allows macros expanded into `..` in a tuple arg position work as ellipsis like that.
| -rw-r--r-- | crates/hir-def/src/body/lower.rs | 44 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/mismatched_arg_count.rs | 69 |
2 files changed, 108 insertions, 5 deletions
diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index e2a5f35313..e69f4129dd 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -4,6 +4,7 @@ use std::mem; use base_db::CrateId; +use either::Either; use hir_expand::{ name::{AsName, Name}, ExpandError, InFile, @@ -1437,14 +1438,12 @@ impl ExprCollector<'_> { has_leading_comma: bool, binding_list: &mut BindingList, ) -> (Box<[PatId]>, Option<usize>) { + let args: Vec<_> = args.map(|p| self.collect_pat_possibly_rest(p, binding_list)).collect(); // Find the location of the `..`, if there is one. Note that we do not // consider the possibility of there being multiple `..` here. - let ellipsis = args.clone().position(|p| matches!(p, ast::Pat::RestPat(_))); + let ellipsis = args.iter().position(|p| p.is_right()); // We want to skip the `..` pattern here, since we account for it above. - let mut args: Vec<_> = args - .filter(|p| !matches!(p, ast::Pat::RestPat(_))) - .map(|p| self.collect_pat(p, binding_list)) - .collect(); + let mut args: Vec<_> = args.into_iter().filter_map(Either::left).collect(); // if there is a leading comma, the user is most likely to type out a leading pattern // so we insert a missing pattern at the beginning for IDE features if has_leading_comma { @@ -1454,6 +1453,41 @@ impl ExprCollector<'_> { (args.into_boxed_slice(), ellipsis) } + // `collect_pat` rejects `ast::Pat::RestPat`, but it should be handled in some cases that + // it is the macro expansion result of an arg sub-pattern in a slice or tuple pattern. + fn collect_pat_possibly_rest( + &mut self, + pat: ast::Pat, + binding_list: &mut BindingList, + ) -> Either<PatId, ()> { + match &pat { + ast::Pat::RestPat(_) => Either::Right(()), + 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| { + if let Some(expanded_pat) = expanded_pat { + this.collect_pat_possibly_rest(expanded_pat, binding_list) + } else { + Either::Left(this.missing_pat()) + } + }); + if let Some(pat) = pat.left() { + self.source_map.pat_map.insert(src, pat); + } + pat + } + None => { + let ptr = AstPtr::new(&pat); + Either::Left(self.alloc_pat(Pat::Missing, ptr)) + } + }, + _ => Either::Left(self.collect_pat(pat, binding_list)), + } + } + // endregion: patterns /// Returns `None` (and emits diagnostics) when `owner` if `#[cfg]`d out, and `Some(())` when diff --git a/crates/ide-diagnostics/src/handlers/mismatched_arg_count.rs b/crates/ide-diagnostics/src/handlers/mismatched_arg_count.rs index 685761e8ff..56ec45c898 100644 --- a/crates/ide-diagnostics/src/handlers/mismatched_arg_count.rs +++ b/crates/ide-diagnostics/src/handlers/mismatched_arg_count.rs @@ -257,6 +257,75 @@ impl Foo { } #[test] + fn rest_pat_in_macro_expansion() { + check_diagnostics( + r#" +// issue #17292 +#![allow(dead_code)] + +macro_rules! replace_with_2_dots { + ( $( $input:tt )* ) => { + .. + }; +} + +macro_rules! enum_str { + ( + $( + $variant:ident ( + $( $tfield:ty ),* + ) + ) + , + * + ) => { + enum Foo { + $( + $variant ( $( $tfield ),* ), + )* + } + + impl Foo { + fn variant_name_as_str(&self) -> &str { + match self { + $( + Self::$variant ( replace_with_2_dots!( $( $tfield ),* ) ) + => "", + )* + } + } + } + }; +} + +enum_str! { + TupleVariant1(i32), + TupleVariant2(), + TupleVariant3(i8,u8,i128) +} +"#, + ); + + check_diagnostics( + r#" +#![allow(dead_code)] +macro_rules! two_dots1 { + () => { .. }; +} + +macro_rules! two_dots2 { + () => { two_dots1!() }; +} + +fn test() { + let (_, _, two_dots1!()) = ((), 42); + let (_, two_dots2!(), _) = (1, true, 2, false, (), (), 3); +} +"#, + ); + } + + #[test] fn varargs() { check_diagnostics( r#" |