Skip to content

Commit

Permalink
Auto merge of rust-lang#17586 - ShoyuVanilla:tuple-arg-macro-rest, r=…
Browse files Browse the repository at this point in the history
…Veykril

Allow macro expansions into `RestPat` in tuple args work as ellipsis like plain `RestPat`

Fixes rust-lang#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 rust-lang#17292 and this PR allows macros expanded into `..` in a tuple arg position work as ellipsis like that.
  • Loading branch information
bors committed Jul 22, 2024
2 parents 7e52128 + 5a292c9 commit 692a42e
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 5 deletions.
44 changes: 39 additions & 5 deletions src/tools/rust-analyzer/crates/hir-def/src/body/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
use std::mem;

use base_db::CrateId;
use either::Either;
use hir_expand::{
name::{AsName, Name},
ExpandError, InFile,
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,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(
Expand Down

0 comments on commit 692a42e

Please sign in to comment.