From da931e7398c7aa6593908aeb05728dd8e8f7f2da Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Sat, 24 Aug 2024 22:35:49 +0300 Subject: [PATCH 1/9] Fix few bugs in closure capture computation, and add tests Also create a test infrastructure for capture computation. --- crates/hir-ty/src/mir.rs | 8 +- crates/hir-ty/src/tests.rs | 1 + crates/hir-ty/src/tests/closure_captures.rs | 284 ++++++++++++++++++++ crates/hir/src/lib.rs | 1 + 4 files changed, 291 insertions(+), 3 deletions(-) create mode 100644 crates/hir-ty/src/tests/closure_captures.rs diff --git a/crates/hir-ty/src/mir.rs b/crates/hir-ty/src/mir.rs index 9c727cb4ed..be094b1388 100644 --- a/crates/hir-ty/src/mir.rs +++ b/crates/hir-ty/src/mir.rs @@ -636,6 +636,7 @@ pub enum TerminatorKind { }, } +// Order of variants in this enum matter: they are used to compare borrow kinds. #[derive(Debug, PartialEq, Eq, Clone, Copy, PartialOrd, Ord)] pub enum BorrowKind { /// Data must be immutable and is aliasable. @@ -666,15 +667,16 @@ pub enum BorrowKind { Mut { kind: MutBorrowKind }, } +// Order of variants in this enum matter: they are used to compare borrow kinds. #[derive(Debug, PartialEq, Eq, Clone, Copy, PartialOrd, Ord)] pub enum MutBorrowKind { + /// Data must be immutable but not aliasable. This kind of borrow cannot currently + /// be expressed by the user and is used only in implicit closure bindings. + ClosureCapture, Default, /// This borrow arose from method-call auto-ref /// (i.e., adjustment::Adjust::Borrow). TwoPhasedBorrow, - /// Data must be immutable but not aliasable. This kind of borrow cannot currently - /// be expressed by the user and is used only in implicit closure bindings. - ClosureCapture, } impl BorrowKind { diff --git a/crates/hir-ty/src/tests.rs b/crates/hir-ty/src/tests.rs index efd8546216..bcf9d5ceff 100644 --- a/crates/hir-ty/src/tests.rs +++ b/crates/hir-ty/src/tests.rs @@ -1,3 +1,4 @@ +mod closure_captures; mod coercion; mod diagnostics; mod display_source_code; diff --git a/crates/hir-ty/src/tests/closure_captures.rs b/crates/hir-ty/src/tests/closure_captures.rs new file mode 100644 index 0000000000..782331a136 --- /dev/null +++ b/crates/hir-ty/src/tests/closure_captures.rs @@ -0,0 +1,284 @@ +use base_db::salsa::InternKey; +use expect_test::{expect, Expect}; +use hir_def::db::DefDatabase; +use itertools::Itertools; +use test_fixture::WithFixture; + +use crate::db::{HirDatabase, InternedClosureId}; +use crate::display::HirDisplay; +use crate::test_db::TestDB; + +use super::visit_module; + +fn check_closure_captures(ra_fixture: &str, expect: Expect) { + let (db, file_id) = TestDB::with_single_file(ra_fixture); + let module = db.module_for_file(file_id); + let def_map = module.def_map(&db); + + let mut defs = Vec::new(); + visit_module(&db, &def_map, module.local_id, &mut |it| defs.push(it)); + + let mut captures_info = Vec::new(); + for def in defs { + let infer = db.infer(def); + let db = &db; + captures_info.extend(infer.closure_info.iter().flat_map(|(closure_id, (captures, _))| { + let closure = db.lookup_intern_closure(InternedClosureId::from_intern_id(closure_id.0)); + let (_, source_map) = db.body_with_source_map(closure.0); + let closure_text_range = source_map + .expr_syntax(closure.1) + .expect("failed to map closure to SyntaxNode") + .value + .text_range(); + captures.iter().flat_map(move |capture| { + // FIXME: Deduplicate this with hir::Local::sources(). + let (body, source_map) = db.body_with_source_map(closure.0); + let local_text_ranges = match body.self_param.zip(source_map.self_param_syntax()) { + Some((param, source)) if param == capture.local() => { + vec![source.file_syntax(db).text_range()] + } + _ => source_map + .patterns_for_binding(capture.local()) + .iter() + .map(|&definition| { + let src = source_map.pat_syntax(definition).unwrap(); + src.file_syntax(db).text_range() + }) + .collect(), + }; + let place = capture.display_place(closure.0, db); + let capture_ty = capture.ty.skip_binders().display_test(db).to_string(); + local_text_ranges.into_iter().map(move |local_text_range| { + ( + closure_text_range, + local_text_range, + place.clone(), + capture_ty.clone(), + capture.kind(), + ) + }) + }) + })); + } + captures_info.sort_unstable_by_key(|(closure_text_range, local_text_range, ..)| { + (closure_text_range.start(), local_text_range.start()) + }); + + let rendered = captures_info + .iter() + .map(|(closure_text_range, local_text_range, place, capture_ty, capture_kind)| { + format!( + "{closure_text_range:?};{local_text_range:?} {capture_kind:?} {place} {capture_ty}" + ) + }) + .join("\n"); + + expect.assert_eq(&rendered); +} + +#[test] +fn deref_in_let() { + check_closure_captures( + r#" +//- minicore:copy +fn main() { + let a = &mut true; + let closure = || { let b = *a; }; +} +"#, + expect!["53..71;0..75 ByRef(Shared) *a &'? bool"], + ); +} + +#[test] +fn deref_then_ref_pattern() { + check_closure_captures( + r#" +//- minicore:copy +fn main() { + let a = &mut true; + let closure = || { let &mut ref b = a; }; +} +"#, + expect!["53..79;0..83 ByRef(Shared) *a &'? bool"], + ); + check_closure_captures( + r#" +//- minicore:copy +fn main() { + let a = &mut true; + let closure = || { let &mut ref mut b = a; }; +} +"#, + expect!["53..83;0..87 ByRef(Mut { kind: Default }) *a &'? mut bool"], + ); +} + +#[test] +fn unique_borrow() { + check_closure_captures( + r#" +//- minicore:copy +fn main() { + let a = &mut true; + let closure = || { *a = false; }; +} +"#, + expect!["53..71;0..75 ByRef(Mut { kind: Default }) *a &'? mut bool"], + ); +} + +#[test] +fn deref_ref_mut() { + check_closure_captures( + r#" +//- minicore:copy +fn main() { + let a = &mut true; + let closure = || { let ref mut b = *a; }; +} +"#, + expect!["53..79;0..83 ByRef(Mut { kind: Default }) *a &'? mut bool"], + ); +} + +#[test] +fn let_else_not_consuming() { + check_closure_captures( + r#" +//- minicore:copy +fn main() { + let a = &mut true; + let closure = || { let _ = *a else { return; }; }; +} +"#, + expect!["53..88;0..92 ByRef(Shared) *a &'? bool"], + ); +} + +#[test] +fn consume() { + check_closure_captures( + r#" +//- minicore:copy +struct NonCopy; +fn main() { + let a = NonCopy; + let closure = || { let b = a; }; +} +"#, + expect!["67..84;0..88 ByValue a NonCopy"], + ); +} + +#[test] +fn ref_to_upvar() { + check_closure_captures( + r#" +//- minicore:copy +struct NonCopy; +fn main() { + let mut a = NonCopy; + let closure = || { let b = &a; }; + let closure = || { let c = &mut a; }; +} +"#, + expect![[r#" + 71..89;0..135 ByRef(Shared) a &'? NonCopy + 109..131;0..135 ByRef(Mut { kind: Default }) a &'? mut NonCopy"#]], + ); +} + +#[test] +fn field() { + check_closure_captures( + r#" +//- minicore:copy +struct Foo { a: i32, b: i32 } +fn main() { + let a = Foo { a: 0, b: 0 }; + let closure = || { let b = a.a; }; +} +"#, + expect!["92..111;0..115 ByRef(Shared) a.a &'? i32"], + ); +} + +#[test] +fn fields_different_mode() { + check_closure_captures( + r#" +//- minicore:copy +struct NonCopy; +struct Foo { a: i32, b: i32, c: NonCopy, d: bool } +fn main() { + let mut a = Foo { a: 0, b: 0 }; + let closure = || { + let b = &a.a; + let c = &mut a.b; + let d = a.c; + }; +} +"#, + expect![[r#" + 133..212;0..216 ByRef(Shared) a.a &'? i32 + 133..212;0..216 ByRef(Mut { kind: Default }) a.b &'? mut i32 + 133..212;0..216 ByValue a.c NonCopy"#]], + ); +} + +#[test] +fn autoref() { + check_closure_captures( + r#" +//- minicore:copy +struct Foo; +impl Foo { + fn imm(&self) {} + fn mut_(&mut self) {} +} +fn main() { + let mut a = Foo; + let closure = || a.imm(); + let closure = || a.mut_(); +} +"#, + expect![[r#" + 123..133;0..168 ByRef(Shared) a &'? Foo + 153..164;0..168 ByRef(Mut { kind: Default }) a &'? mut Foo"#]], + ); +} + +#[test] +fn captures_priority() { + check_closure_captures( + r#" +//- minicore:copy +struct NonCopy; +fn main() { + let mut a = &mut true; + // Max ByRef(Mut { kind: Default }) + let closure = || { + *a = false; + let b = &mut a; + }; + // Max ByRef(Mut { kind: ClosureCapture }) + let closure = || { + let b = *a; + let c = &mut *a; + }; + // Max ByValue + let mut a = NonCopy; + let closure = || { + let b = a; + let c = &mut a; + let d = &a; + }; +} +"#, + expect![[r#" + 113..167;0..430 ByRef(Mut { kind: Default }) a &'? mut &'? mut bool + 234..289;0..430 ByRef(Mut { kind: Default }) *a &'? mut bool + 353..426;0..430 ByValue a NonCopy"#]], + ); +} diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 6e26af55af..1e8127161d 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -4133,6 +4133,7 @@ impl ClosureCapture { } } +#[derive(Clone, Copy, PartialEq, Eq)] pub enum CaptureKind { SharedRef, UniqueSharedRef, From 506b9663bf8168fc10b5ace8a2b974fa65535ef2 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Sat, 24 Aug 2024 22:46:45 +0300 Subject: [PATCH 2/9] Preserve all spans for closure captures, not just one This is important for the "convert closure to fn" assist, as it needs to find and modify the places the captures are used. --- crates/hir-ty/src/infer.rs | 8 + crates/hir-ty/src/infer/closure.rs | 387 +++++++++++++------- crates/hir-ty/src/mir/lower.rs | 11 +- crates/hir-ty/src/tests/closure_captures.rs | 231 +++++++++--- 4 files changed, 457 insertions(+), 180 deletions(-) diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index 8b4e98c550..062ea27815 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -59,6 +59,7 @@ use crate::{ generics::Generics, infer::{coerce::CoerceMany, unify::InferenceTable}, lower::ImplTraitLoweringMode, + mir::MirSpan, to_assoc_type_id, traits::FnTrait, utils::{InTypeConstIdMetadata, UnevaluatedConstEvaluatorFolder}, @@ -553,6 +554,12 @@ pub(crate) struct InferenceContext<'a> { // fields related to closure capture current_captures: Vec, + /// A stack that has an entry for each projection in the current capture. + /// + /// For example, in `a.b.c`, we capture the spans of `a`, `a.b`, and `a.b.c`. + /// We do that because sometimes we truncate projections (when a closure captures + /// both `a.b` and `a.b.c`), and we want to provide accurate spans in this case. + current_capture_span_stack: Vec, current_closure: Option, /// Stores the list of closure ids that need to be analyzed before this closure. See the /// comment on `InferenceContext::sort_closures` @@ -634,6 +641,7 @@ impl<'a> InferenceContext<'a> { breakables: Vec::new(), deferred_cast_checks: Vec::new(), current_captures: Vec::new(), + current_capture_span_stack: Vec::new(), current_closure: None, deferred_closures: FxHashMap::default(), closure_dependencies: FxHashMap::default(), diff --git a/crates/hir-ty/src/infer/closure.rs b/crates/hir-ty/src/infer/closure.rs index 910e19dc58..68e1a6ee82 100644 --- a/crates/hir-ty/src/infer/closure.rs +++ b/crates/hir-ty/src/infer/closure.rs @@ -18,7 +18,7 @@ use hir_def::{ use hir_expand::name::Name; use intern::sym; use rustc_hash::FxHashMap; -use smallvec::SmallVec; +use smallvec::{smallvec, SmallVec}; use stdx::never; use crate::{ @@ -236,7 +236,13 @@ pub enum CaptureKind { pub struct CapturedItem { pub(crate) place: HirPlace, pub(crate) kind: CaptureKind, - pub(crate) span: MirSpan, + /// The inner vec is the stacks; the outer vec is for each capture reference. + /// + /// Even though we always report only the last span (i.e. the most inclusive span), + /// we need to keep them all, since when a closure occurs inside a closure, we + /// copy all captures of the inner closure to the outer closure, and then we may + /// truncate them, and we want the correct span to be reported. + span_stacks: SmallVec<[SmallVec<[MirSpan; 3]>; 3]>, pub(crate) ty: Binders, } @@ -253,6 +259,10 @@ impl CapturedItem { self.kind } + pub fn spans(&self) -> SmallVec<[MirSpan; 3]> { + self.span_stacks.iter().map(|stack| *stack.last().expect("empty span stack")).collect() + } + pub fn display_place(&self, owner: DefWithBodyId, db: &dyn HirDatabase) -> String { let body = db.body(owner); let krate = owner.krate(db.upcast()); @@ -314,7 +324,8 @@ impl CapturedItem { pub(crate) struct CapturedItemWithoutTy { pub(crate) place: HirPlace, pub(crate) kind: CaptureKind, - pub(crate) span: MirSpan, + /// The inner vec is the stacks; the outer vec is for each capture reference. + pub(crate) span_stacks: SmallVec<[SmallVec<[MirSpan; 3]>; 3]>, } impl CapturedItemWithoutTy { @@ -333,7 +344,7 @@ impl CapturedItemWithoutTy { return CapturedItem { place: self.place, kind: self.kind, - span: self.span, + span_stacks: self.span_stacks, ty: replace_placeholder_with_binder(ctx, ty), }; @@ -393,22 +404,26 @@ impl InferenceContext<'_> { let r = self.place_of_expr_without_adjust(tgt_expr)?; let default = vec![]; let adjustments = self.result.expr_adjustments.get(&tgt_expr).unwrap_or(&default); - apply_adjusts_to_place(r, adjustments) + apply_adjusts_to_place(&mut self.current_capture_span_stack, r, adjustments) } + /// Changes `current_capture_span_stack` to contain the stack of spans for this expr. fn place_of_expr_without_adjust(&mut self, tgt_expr: ExprId) -> Option { + self.current_capture_span_stack.clear(); match &self.body[tgt_expr] { Expr::Path(p) => { let resolver = resolver_for_expr(self.db.upcast(), self.owner, tgt_expr); if let Some(ResolveValueResult::ValueNs(ValueNs::LocalBinding(b), _)) = resolver.resolve_path_in_value_ns(self.db.upcast(), p) { + self.current_capture_span_stack.push(MirSpan::ExprId(tgt_expr)); return Some(HirPlace { local: b, projections: vec![] }); } } Expr::Field { expr, name: _ } => { let mut place = self.place_of_expr(*expr)?; let field = self.result.field_resolution(tgt_expr)?; + self.current_capture_span_stack.push(MirSpan::ExprId(tgt_expr)); place.projections.push(ProjectionElem::Field(field)); return Some(place); } @@ -418,6 +433,7 @@ impl InferenceContext<'_> { TyKind::Ref(..) | TyKind::Raw(..) ) { let mut place = self.place_of_expr(*expr)?; + self.current_capture_span_stack.push(MirSpan::ExprId(tgt_expr)); place.projections.push(ProjectionElem::Deref); return Some(place); } @@ -427,29 +443,65 @@ impl InferenceContext<'_> { None } - fn push_capture(&mut self, capture: CapturedItemWithoutTy) { - self.current_captures.push(capture); + fn push_capture(&mut self, place: HirPlace, kind: CaptureKind) { + self.current_captures.push(CapturedItemWithoutTy { + place, + kind, + span_stacks: smallvec![self.current_capture_span_stack.iter().copied().collect()], + }); } - fn ref_expr(&mut self, expr: ExprId) { - if let Some(place) = self.place_of_expr(expr) { - self.add_capture(place, CaptureKind::ByRef(BorrowKind::Shared), expr.into()); + fn is_ref_span(&self, span: MirSpan) -> bool { + match span { + MirSpan::ExprId(expr) => matches!(self.body[expr], Expr::Ref { .. }), + MirSpan::BindingId(_) => true, + MirSpan::PatId(_) | MirSpan::SelfParam | MirSpan::Unknown => false, + } + } + + fn truncate_capture_spans(&self, capture: &mut CapturedItemWithoutTy, mut truncate_to: usize) { + // The first span is the identifier, and it must always remain. + truncate_to += 1; + for span_stack in &mut capture.span_stacks { + let mut remained = truncate_to; + let mut actual_truncate_to = 0; + for &span in &*span_stack { + actual_truncate_to += 1; + if !self.is_ref_span(span) { + remained -= 1; + if remained == 0 { + break; + } + } + } + if actual_truncate_to < span_stack.len() + && self.is_ref_span(span_stack[actual_truncate_to]) + { + // Include the ref operator if there is one, we will fix it later (in `strip_captures_ref_span()`) if it's incorrect. + actual_truncate_to += 1; + } + span_stack.truncate(actual_truncate_to); + } + } + + fn ref_expr(&mut self, expr: ExprId, place: Option) { + if let Some(place) = place { + self.add_capture(place, CaptureKind::ByRef(BorrowKind::Shared)); } self.walk_expr(expr); } - fn add_capture(&mut self, place: HirPlace, kind: CaptureKind, span: MirSpan) { + fn add_capture(&mut self, place: HirPlace, kind: CaptureKind) { if self.is_upvar(&place) { - self.push_capture(CapturedItemWithoutTy { place, kind, span }); + self.push_capture(place, kind); } } - fn mutate_expr(&mut self, expr: ExprId) { - if let Some(place) = self.place_of_expr(expr) { + fn mutate_expr(&mut self, expr: ExprId, place: Option) { + if let Some(place) = place { self.add_capture( place, CaptureKind::ByRef(BorrowKind::Mut { kind: MutBorrowKind::Default }), - expr.into(), ); } self.walk_expr(expr); @@ -457,12 +509,12 @@ impl InferenceContext<'_> { fn consume_expr(&mut self, expr: ExprId) { if let Some(place) = self.place_of_expr(expr) { - self.consume_place(place, expr.into()); + self.consume_place(place); } self.walk_expr(expr); } - fn consume_place(&mut self, place: HirPlace, span: MirSpan) { + fn consume_place(&mut self, place: HirPlace) { if self.is_upvar(&place) { let ty = place.ty(self); let kind = if self.is_ty_copy(ty) { @@ -470,7 +522,7 @@ impl InferenceContext<'_> { } else { CaptureKind::ByValue }; - self.push_capture(CapturedItemWithoutTy { place, kind, span }); + self.push_capture(place, kind); } } @@ -501,8 +553,10 @@ impl InferenceContext<'_> { Mutability::Not => CaptureKind::ByRef(BorrowKind::Shared), }; if let Some(place) = self.place_of_expr_without_adjust(tgt_expr) { - if let Some(place) = apply_adjusts_to_place(place, rest) { - self.add_capture(place, capture_kind, tgt_expr.into()); + if let Some(place) = + apply_adjusts_to_place(&mut self.current_capture_span_stack, place, rest) + { + self.add_capture(place, capture_kind); } } self.walk_expr_with_adjust(tgt_expr, rest); @@ -584,11 +638,7 @@ impl InferenceContext<'_> { self.walk_pat(&mut capture_mode, arm.pat); } if let Some(c) = capture_mode { - self.push_capture(CapturedItemWithoutTy { - place: discr_place, - kind: c, - span: (*expr).into(), - }) + self.push_capture(discr_place, c); } } } @@ -632,10 +682,11 @@ impl InferenceContext<'_> { } false }; + let place = self.place_of_expr(*expr); if mutability { - self.mutate_expr(*expr); + self.mutate_expr(*expr, place); } else { - self.ref_expr(*expr); + self.ref_expr(*expr, place); } } else { self.select_from_expr(*expr); @@ -650,16 +701,22 @@ impl InferenceContext<'_> { | Expr::Cast { expr, type_ref: _ } => { self.consume_expr(*expr); } - Expr::Ref { expr, rawness: _, mutability } => match mutability { - hir_def::type_ref::Mutability::Shared => self.ref_expr(*expr), - hir_def::type_ref::Mutability::Mut => self.mutate_expr(*expr), - }, + Expr::Ref { expr, rawness: _, mutability } => { + // We need to do this before we push the span so the order will be correct. + let place = self.place_of_expr(*expr); + self.current_capture_span_stack.push(MirSpan::ExprId(tgt_expr)); + match mutability { + hir_def::type_ref::Mutability::Shared => self.ref_expr(*expr, place), + hir_def::type_ref::Mutability::Mut => self.mutate_expr(*expr, place), + } + } Expr::BinaryOp { lhs, rhs, op } => { let Some(op) = op else { return; }; if matches!(op, BinaryOp::Assignment { .. }) { - self.mutate_expr(*lhs); + let place = self.place_of_expr(*lhs); + self.mutate_expr(*lhs, place); self.consume_expr(*rhs); return; } @@ -690,7 +747,11 @@ impl InferenceContext<'_> { ); let mut cc = mem::take(&mut self.current_captures); cc.extend(captures.iter().filter(|it| self.is_upvar(&it.place)).map(|it| { - CapturedItemWithoutTy { place: it.place.clone(), kind: it.kind, span: it.span } + CapturedItemWithoutTy { + place: it.place.clone(), + kind: it.kind, + span_stacks: it.span_stacks.clone(), + } })); self.current_captures = cc; } @@ -812,10 +873,13 @@ impl InferenceContext<'_> { } fn restrict_precision_for_unsafe(&mut self) { - for capture in &mut self.current_captures { + // FIXME: Borrow checker problems without this. + let mut current_captures = std::mem::take(&mut self.current_captures); + for capture in &mut current_captures { let mut ty = self.table.resolve_completely(self.result[capture.place.local].clone()); if ty.as_raw_ptr().is_some() || ty.is_union() { capture.kind = CaptureKind::ByRef(BorrowKind::Shared); + self.truncate_capture_spans(capture, 0); capture.place.projections.truncate(0); continue; } @@ -830,29 +894,35 @@ impl InferenceContext<'_> { ); if ty.as_raw_ptr().is_some() || ty.is_union() { capture.kind = CaptureKind::ByRef(BorrowKind::Shared); + self.truncate_capture_spans(capture, i + 1); capture.place.projections.truncate(i + 1); break; } } } + self.current_captures = current_captures; } fn adjust_for_move_closure(&mut self) { - for capture in &mut self.current_captures { + // FIXME: Borrow checker won't allow without this. + let mut current_captures = std::mem::take(&mut self.current_captures); + for capture in &mut current_captures { if let Some(first_deref) = capture.place.projections.iter().position(|proj| *proj == ProjectionElem::Deref) { + self.truncate_capture_spans(capture, first_deref); capture.place.projections.truncate(first_deref); } capture.kind = CaptureKind::ByValue; } + self.current_captures = current_captures; } fn minimize_captures(&mut self) { - self.current_captures.sort_by_key(|it| it.place.projections.len()); + self.current_captures.sort_unstable_by_key(|it| it.place.projections.len()); let mut hash_map = FxHashMap::::default(); let result = mem::take(&mut self.current_captures); - for item in result { + for mut item in result { let mut lookup_place = HirPlace { local: item.place.local, projections: vec![] }; let mut it = item.place.projections.iter(); let prev_index = loop { @@ -860,12 +930,17 @@ impl InferenceContext<'_> { break Some(*k); } match it.next() { - Some(it) => lookup_place.projections.push(it.clone()), + Some(it) => { + lookup_place.projections.push(it.clone()); + } None => break None, } }; match prev_index { Some(p) => { + let prev_projections_len = self.current_captures[p].place.projections.len(); + self.truncate_capture_spans(&mut item, prev_projections_len); + self.current_captures[p].span_stacks.extend(item.span_stacks); let len = self.current_captures[p].place.projections.len(); let kind_after_truncate = item.place.capture_kind_of_truncated_place(item.kind, len); @@ -880,113 +955,128 @@ impl InferenceContext<'_> { } } - fn consume_with_pat(&mut self, mut place: HirPlace, pat: PatId) { - let cnt = self.result.pat_adjustments.get(&pat).map(|it| it.len()).unwrap_or_default(); - place.projections = place - .projections - .iter() - .cloned() - .chain((0..cnt).map(|_| ProjectionElem::Deref)) - .collect::>(); - match &self.body[pat] { - Pat::Missing | Pat::Wild => (), - Pat::Tuple { args, ellipsis } => { - let (al, ar) = args.split_at(ellipsis.map_or(args.len(), |it| it as usize)); - let field_count = match self.result[pat].kind(Interner) { - TyKind::Tuple(_, s) => s.len(Interner), - _ => return, - }; - let fields = 0..field_count; - let it = al.iter().zip(fields.clone()).chain(ar.iter().rev().zip(fields.rev())); - for (arg, i) in it { - let mut p = place.clone(); - p.projections.push(ProjectionElem::Field(Either::Right(TupleFieldId { - tuple: TupleId(!0), // dummy this, as its unused anyways - index: i as u32, - }))); - self.consume_with_pat(p, *arg); - } - } - Pat::Or(pats) => { - for pat in pats.iter() { - self.consume_with_pat(place.clone(), *pat); - } - } - Pat::Record { args, .. } => { - let Some(variant) = self.result.variant_resolution_for_pat(pat) else { - return; - }; - match variant { - VariantId::EnumVariantId(_) | VariantId::UnionId(_) => { - self.consume_place(place, pat.into()) + fn consume_with_pat(&mut self, mut place: HirPlace, tgt_pat: PatId) { + let adjustments_count = + self.result.pat_adjustments.get(&tgt_pat).map(|it| it.len()).unwrap_or_default(); + place.projections.extend((0..adjustments_count).map(|_| ProjectionElem::Deref)); + self.current_capture_span_stack + .extend((0..adjustments_count).map(|_| MirSpan::PatId(tgt_pat))); + 'reset_span_stack: { + match &self.body[tgt_pat] { + Pat::Missing | Pat::Wild => (), + Pat::Tuple { args, ellipsis } => { + let (al, ar) = args.split_at(ellipsis.map_or(args.len(), |it| it as usize)); + let field_count = match self.result[tgt_pat].kind(Interner) { + TyKind::Tuple(_, s) => s.len(Interner), + _ => break 'reset_span_stack, + }; + let fields = 0..field_count; + let it = al.iter().zip(fields.clone()).chain(ar.iter().rev().zip(fields.rev())); + for (&arg, i) in it { + let mut p = place.clone(); + self.current_capture_span_stack.push(MirSpan::PatId(arg)); + p.projections.push(ProjectionElem::Field(Either::Right(TupleFieldId { + tuple: TupleId(!0), // dummy this, as its unused anyways + index: i as u32, + }))); + self.consume_with_pat(p, arg); + self.current_capture_span_stack.pop(); } - VariantId::StructId(s) => { - let vd = &*self.db.struct_data(s).variant_data; - for field_pat in args.iter() { - let arg = field_pat.pat; - let Some(local_id) = vd.field(&field_pat.name) else { - continue; - }; - let mut p = place.clone(); - p.projections.push(ProjectionElem::Field(Either::Left(FieldId { - parent: variant, - local_id, - }))); - self.consume_with_pat(p, arg); + } + Pat::Or(pats) => { + for pat in pats.iter() { + self.consume_with_pat(place.clone(), *pat); + } + } + Pat::Record { args, .. } => { + let Some(variant) = self.result.variant_resolution_for_pat(tgt_pat) else { + break 'reset_span_stack; + }; + match variant { + VariantId::EnumVariantId(_) | VariantId::UnionId(_) => { + self.consume_place(place) + } + VariantId::StructId(s) => { + let vd = &*self.db.struct_data(s).variant_data; + for field_pat in args.iter() { + let arg = field_pat.pat; + let Some(local_id) = vd.field(&field_pat.name) else { + continue; + }; + let mut p = place.clone(); + self.current_capture_span_stack.push(MirSpan::PatId(arg)); + p.projections.push(ProjectionElem::Field(Either::Left(FieldId { + parent: variant, + local_id, + }))); + self.consume_with_pat(p, arg); + self.current_capture_span_stack.pop(); + } } } } - } - Pat::Range { .. } - | Pat::Slice { .. } - | Pat::ConstBlock(_) - | Pat::Path(_) - | Pat::Lit(_) => self.consume_place(place, pat.into()), - Pat::Bind { id: _, subpat: _ } => { - let mode = self.result.binding_modes[pat]; - let capture_kind = match mode { - BindingMode::Move => { - self.consume_place(place, pat.into()); - return; - } - BindingMode::Ref(Mutability::Not) => BorrowKind::Shared, - BindingMode::Ref(Mutability::Mut) => { - BorrowKind::Mut { kind: MutBorrowKind::Default } - } - }; - self.add_capture(place, CaptureKind::ByRef(capture_kind), pat.into()); - } - Pat::TupleStruct { path: _, args, ellipsis } => { - let Some(variant) = self.result.variant_resolution_for_pat(pat) else { - return; - }; - match variant { - VariantId::EnumVariantId(_) | VariantId::UnionId(_) => { - self.consume_place(place, pat.into()) - } - VariantId::StructId(s) => { - let vd = &*self.db.struct_data(s).variant_data; - let (al, ar) = args.split_at(ellipsis.map_or(args.len(), |it| it as usize)); - let fields = vd.fields().iter(); - let it = - al.iter().zip(fields.clone()).chain(ar.iter().rev().zip(fields.rev())); - for (arg, (i, _)) in it { - let mut p = place.clone(); - p.projections.push(ProjectionElem::Field(Either::Left(FieldId { - parent: variant, - local_id: i, - }))); - self.consume_with_pat(p, *arg); + Pat::Range { .. } + | Pat::Slice { .. } + | Pat::ConstBlock(_) + | Pat::Path(_) + | Pat::Lit(_) => self.consume_place(place), + &Pat::Bind { id, subpat: _ } => { + let mode = self.result.binding_modes[tgt_pat]; + let capture_kind = match mode { + BindingMode::Move => { + self.consume_place(place); + break 'reset_span_stack; + } + BindingMode::Ref(Mutability::Not) => BorrowKind::Shared, + BindingMode::Ref(Mutability::Mut) => { + BorrowKind::Mut { kind: MutBorrowKind::Default } + } + }; + self.current_capture_span_stack.push(MirSpan::BindingId(id)); + self.add_capture(place, CaptureKind::ByRef(capture_kind)); + self.current_capture_span_stack.pop(); + } + Pat::TupleStruct { path: _, args, ellipsis } => { + let Some(variant) = self.result.variant_resolution_for_pat(tgt_pat) else { + break 'reset_span_stack; + }; + match variant { + VariantId::EnumVariantId(_) | VariantId::UnionId(_) => { + self.consume_place(place) + } + VariantId::StructId(s) => { + let vd = &*self.db.struct_data(s).variant_data; + let (al, ar) = + args.split_at(ellipsis.map_or(args.len(), |it| it as usize)); + let fields = vd.fields().iter(); + let it = al + .iter() + .zip(fields.clone()) + .chain(ar.iter().rev().zip(fields.rev())); + for (&arg, (i, _)) in it { + let mut p = place.clone(); + self.current_capture_span_stack.push(MirSpan::PatId(arg)); + p.projections.push(ProjectionElem::Field(Either::Left(FieldId { + parent: variant, + local_id: i, + }))); + self.consume_with_pat(p, arg); + self.current_capture_span_stack.pop(); + } } } } + Pat::Ref { pat, mutability: _ } => { + self.current_capture_span_stack.push(MirSpan::PatId(tgt_pat)); + place.projections.push(ProjectionElem::Deref); + self.consume_with_pat(place, *pat); + self.current_capture_span_stack.pop(); + } + Pat::Box { .. } => (), // not supported } - Pat::Ref { pat, mutability: _ } => { - place.projections.push(ProjectionElem::Deref); - self.consume_with_pat(place, *pat) - } - Pat::Box { .. } => (), // not supported } + self.current_capture_span_stack + .truncate(self.current_capture_span_stack.len() - adjustments_count); } fn consume_exprs(&mut self, exprs: impl Iterator) { @@ -1044,12 +1134,28 @@ impl InferenceContext<'_> { CaptureBy::Ref => (), } self.minimize_captures(); + self.strip_captures_ref_span(); let result = mem::take(&mut self.current_captures); let captures = result.into_iter().map(|it| it.with_ty(self)).collect::>(); self.result.closure_info.insert(closure, (captures, closure_kind)); closure_kind } + fn strip_captures_ref_span(&mut self) { + // FIXME: Borrow checker won't allow without this. + let mut captures = std::mem::take(&mut self.current_captures); + for capture in &mut captures { + if matches!(capture.kind, CaptureKind::ByValue) { + for span_stack in &mut capture.span_stacks { + if self.is_ref_span(span_stack[span_stack.len() - 1]) { + span_stack.truncate(span_stack.len() - 1); + } + } + } + } + self.current_captures = captures; + } + pub(crate) fn infer_closures(&mut self) { let deferred_closures = self.sort_closures(); for (closure, exprs) in deferred_closures.into_iter().rev() { @@ -1110,10 +1216,17 @@ impl InferenceContext<'_> { } } -fn apply_adjusts_to_place(mut r: HirPlace, adjustments: &[Adjustment]) -> Option { +/// Call this only when the last span in the stack isn't a split. +fn apply_adjusts_to_place( + current_capture_span_stack: &mut Vec, + mut r: HirPlace, + adjustments: &[Adjustment], +) -> Option { + let span = *current_capture_span_stack.last().expect("empty capture span stack"); for adj in adjustments { match &adj.kind { Adjust::Deref(None) => { + current_capture_span_stack.push(span); r.projections.push(ProjectionElem::Deref); } _ => return None, diff --git a/crates/hir-ty/src/mir/lower.rs b/crates/hir-ty/src/mir/lower.rs index 3a0d0c933f..9e23550451 100644 --- a/crates/hir-ty/src/mir/lower.rs +++ b/crates/hir-ty/src/mir/lower.rs @@ -1180,8 +1180,15 @@ impl<'ctx> MirLowerCtx<'ctx> { let placeholder_subst = self.placeholder_subst(); let tmp_ty = capture.ty.clone().substitute(Interner, &placeholder_subst); - let tmp: Place = self.temp(tmp_ty, current, capture.span)?.into(); - self.push_assignment(current, tmp, Rvalue::Ref(*bk, p), capture.span); + // FIXME: Handle more than one span. + let capture_spans = capture.spans(); + let tmp: Place = self.temp(tmp_ty, current, capture_spans[0])?.into(); + self.push_assignment( + current, + tmp, + Rvalue::Ref(*bk, p), + capture_spans[0], + ); operands.push(Operand::Move(tmp)); } CaptureKind::ByValue => operands.push(Operand::Move(p)), diff --git a/crates/hir-ty/src/tests/closure_captures.rs b/crates/hir-ty/src/tests/closure_captures.rs index 782331a136..22cef3505b 100644 --- a/crates/hir-ty/src/tests/closure_captures.rs +++ b/crates/hir-ty/src/tests/closure_captures.rs @@ -1,11 +1,15 @@ use base_db::salsa::InternKey; use expect_test::{expect, Expect}; use hir_def::db::DefDatabase; +use hir_expand::files::InFileWrapper; use itertools::Itertools; +use span::{HirFileId, TextRange}; +use syntax::{AstNode, AstPtr}; use test_fixture::WithFixture; use crate::db::{HirDatabase, InternedClosureId}; use crate::display::HirDisplay; +use crate::mir::MirSpan; use crate::test_db::TestDB; use super::visit_module; @@ -30,45 +34,69 @@ fn check_closure_captures(ra_fixture: &str, expect: Expect) { .expect("failed to map closure to SyntaxNode") .value .text_range(); - captures.iter().flat_map(move |capture| { + captures.iter().map(move |capture| { + fn text_range( + db: &TestDB, + syntax: InFileWrapper>, + ) -> TextRange { + let root = syntax.file_syntax(db); + syntax.value.to_node(&root).syntax().text_range() + } + // FIXME: Deduplicate this with hir::Local::sources(). let (body, source_map) = db.body_with_source_map(closure.0); - let local_text_ranges = match body.self_param.zip(source_map.self_param_syntax()) { + let local_text_range = match body.self_param.zip(source_map.self_param_syntax()) { Some((param, source)) if param == capture.local() => { - vec![source.file_syntax(db).text_range()] + format!("{:?}", text_range(db, source)) } _ => source_map .patterns_for_binding(capture.local()) .iter() .map(|&definition| { - let src = source_map.pat_syntax(definition).unwrap(); - src.file_syntax(db).text_range() + text_range(db, source_map.pat_syntax(definition).unwrap()) }) - .collect(), + .map(|it| format!("{it:?}")) + .join(", "), }; let place = capture.display_place(closure.0, db); let capture_ty = capture.ty.skip_binders().display_test(db).to_string(); - local_text_ranges.into_iter().map(move |local_text_range| { - ( - closure_text_range, - local_text_range, - place.clone(), - capture_ty.clone(), - capture.kind(), - ) - }) + let spans = capture + .spans() + .iter() + .flat_map(|span| match *span { + MirSpan::ExprId(expr) => { + vec![text_range(db, source_map.expr_syntax(expr).unwrap())] + } + MirSpan::PatId(pat) => { + vec![text_range(db, source_map.pat_syntax(pat).unwrap())] + } + MirSpan::BindingId(binding) => source_map + .patterns_for_binding(binding) + .iter() + .map(|pat| text_range(db, source_map.pat_syntax(*pat).unwrap())) + .collect(), + MirSpan::SelfParam => { + vec![text_range(db, source_map.self_param_syntax().unwrap())] + } + MirSpan::Unknown => Vec::new(), + }) + .sorted_by_key(|it| it.start()) + .map(|it| format!("{it:?}")) + .join(","); + + (closure_text_range, local_text_range, spans, place, capture_ty, capture.kind()) }) })); } captures_info.sort_unstable_by_key(|(closure_text_range, local_text_range, ..)| { - (closure_text_range.start(), local_text_range.start()) + (closure_text_range.start(), local_text_range.clone()) }); let rendered = captures_info .iter() - .map(|(closure_text_range, local_text_range, place, capture_ty, capture_kind)| { + .map(|(closure_text_range, local_text_range, spans, place, capture_ty, capture_kind)| { format!( - "{closure_text_range:?};{local_text_range:?} {capture_kind:?} {place} {capture_ty}" + "{closure_text_range:?};{local_text_range};{spans} {capture_kind:?} {place} {capture_ty}" ) }) .join("\n"); @@ -86,7 +114,7 @@ fn main() { let closure = || { let b = *a; }; } "#, - expect!["53..71;0..75 ByRef(Shared) *a &'? bool"], + expect!["53..71;20..21;66..68 ByRef(Shared) *a &'? bool"], ); } @@ -100,7 +128,7 @@ fn main() { let closure = || { let &mut ref b = a; }; } "#, - expect!["53..79;0..83 ByRef(Shared) *a &'? bool"], + expect!["53..79;20..21;67..72 ByRef(Shared) *a &'? bool"], ); check_closure_captures( r#" @@ -110,7 +138,7 @@ fn main() { let closure = || { let &mut ref mut b = a; }; } "#, - expect!["53..83;0..87 ByRef(Mut { kind: Default }) *a &'? mut bool"], + expect!["53..83;20..21;67..76 ByRef(Mut { kind: Default }) *a &'? mut bool"], ); } @@ -124,7 +152,7 @@ fn main() { let closure = || { *a = false; }; } "#, - expect!["53..71;0..75 ByRef(Mut { kind: Default }) *a &'? mut bool"], + expect!["53..71;20..21;58..60 ByRef(Mut { kind: Default }) *a &'? mut bool"], ); } @@ -138,7 +166,7 @@ fn main() { let closure = || { let ref mut b = *a; }; } "#, - expect!["53..79;0..83 ByRef(Mut { kind: Default }) *a &'? mut bool"], + expect!["53..79;20..21;62..71 ByRef(Mut { kind: Default }) *a &'? mut bool"], ); } @@ -152,7 +180,7 @@ fn main() { let closure = || { let _ = *a else { return; }; }; } "#, - expect!["53..88;0..92 ByRef(Shared) *a &'? bool"], + expect!["53..88;20..21;66..68 ByRef(Shared) *a &'? bool"], ); } @@ -167,7 +195,7 @@ fn main() { let closure = || { let b = a; }; } "#, - expect!["67..84;0..88 ByValue a NonCopy"], + expect!["67..84;36..37;80..81 ByValue a NonCopy"], ); } @@ -184,8 +212,8 @@ fn main() { } "#, expect![[r#" - 71..89;0..135 ByRef(Shared) a &'? NonCopy - 109..131;0..135 ByRef(Mut { kind: Default }) a &'? mut NonCopy"#]], + 71..89;36..41;84..86 ByRef(Shared) a &'? NonCopy + 109..131;36..41;122..128 ByRef(Mut { kind: Default }) a &'? mut NonCopy"#]], ); } @@ -200,7 +228,7 @@ fn main() { let closure = || { let b = a.a; }; } "#, - expect!["92..111;0..115 ByRef(Shared) a.a &'? i32"], + expect!["92..111;50..51;105..108 ByRef(Shared) a.a &'? i32"], ); } @@ -221,9 +249,9 @@ fn main() { } "#, expect![[r#" - 133..212;0..216 ByRef(Shared) a.a &'? i32 - 133..212;0..216 ByRef(Mut { kind: Default }) a.b &'? mut i32 - 133..212;0..216 ByValue a.c NonCopy"#]], + 133..212;87..92;154..158 ByRef(Shared) a.a &'? i32 + 133..212;87..92;176..184 ByRef(Mut { kind: Default }) a.b &'? mut i32 + 133..212;87..92;202..205 ByValue a.c NonCopy"#]], ); } @@ -244,8 +272,8 @@ fn main() { } "#, expect![[r#" - 123..133;0..168 ByRef(Shared) a &'? Foo - 153..164;0..168 ByRef(Mut { kind: Default }) a &'? mut Foo"#]], + 123..133;92..97;126..127 ByRef(Shared) a &'? Foo + 153..164;92..97;156..157 ByRef(Mut { kind: Default }) a &'? mut Foo"#]], ); } @@ -262,11 +290,6 @@ fn main() { *a = false; let b = &mut a; }; - // Max ByRef(Mut { kind: ClosureCapture }) - let closure = || { - let b = *a; - let c = &mut *a; - }; // Max ByValue let mut a = NonCopy; let closure = || { @@ -277,8 +300,134 @@ fn main() { } "#, expect![[r#" - 113..167;0..430 ByRef(Mut { kind: Default }) a &'? mut &'? mut bool - 234..289;0..430 ByRef(Mut { kind: Default }) *a &'? mut bool - 353..426;0..430 ByValue a NonCopy"#]], + 113..167;36..41;127..128,154..160 ByRef(Mut { kind: Default }) a &'? mut &'? mut bool + 231..304;196..201;252..253,276..277,296..297 ByValue a NonCopy"#]], + ); +} + +#[test] +fn let_underscore() { + check_closure_captures( + r#" +//- minicore:copy +fn main() { + let mut a = true; + let closure = || { let _ = a; }; +} +"#, + expect![""], + ); +} + +#[test] +fn match_wildcard() { + check_closure_captures( + r#" +//- minicore:copy +struct NonCopy; +fn main() { + let mut a = NonCopy; + let closure = || match a { + _ => {} + }; + let closure = || match a { + ref b => {} + }; + let closure = || match a { + ref mut b => {} + }; +} +"#, + expect![[r#" + 125..163;36..41;134..135 ByRef(Shared) a &'? NonCopy + 183..225;36..41;192..193 ByRef(Mut { kind: Default }) a &'? mut NonCopy"#]], + ); +} + +#[test] +fn multiple_bindings() { + check_closure_captures( + r#" +//- minicore:copy +fn main() { + let mut a = false; + let mut closure = || { let (b | b) = a; }; +} +"#, + expect!["57..80;20..25;76..77,76..77 ByRef(Shared) a &'? bool"], + ); +} + +#[test] +fn multiple_usages() { + check_closure_captures( + r#" +//- minicore:copy +fn main() { + let mut a = false; + let mut closure = || { + let b = &a; + let c = &a; + let d = &mut a; + a = true; + }; +} +"#, + expect!["57..149;20..25;78..80,98..100,118..124,134..135 ByRef(Mut { kind: Default }) a &'? mut bool"], + ); +} + +#[test] +fn ref_then_deref() { + check_closure_captures( + r#" +//- minicore:copy +fn main() { + let mut a = false; + let mut closure = || { let b = *&mut a; }; +} +"#, + expect!["57..80;20..25;71..77 ByRef(Mut { kind: Default }) a &'? mut bool"], + ); +} + +#[test] +fn ref_of_ref() { + check_closure_captures( + r#" +//- minicore:copy +fn main() { + let mut a = &false; + let closure = || { let b = &a; }; + let closure = || { let b = &mut a; }; + let a = &mut false; + let closure = || { let b = &a; }; + let closure = || { let b = &mut a; }; +} +"#, + expect![[r#" + 54..72;20..25;67..69 ByRef(Shared) a &'? &'? bool + 92..114;20..25;105..111 ByRef(Mut { kind: Default }) a &'? mut &'? bool + 158..176;124..125;171..173 ByRef(Shared) a &'? &'? mut bool + 196..218;124..125;209..215 ByRef(Mut { kind: Default }) a &'? mut &'? mut bool"#]], + ); +} + +#[test] +fn multiple_capture_usages() { + check_closure_captures( + r#" +//- minicore:copy +struct A { a: i32, b: bool } +fn main() { + let mut a = A { a: 123, b: false }; + let closure = |$0| { + let b = a.b; + a = A { a: 456, b: true }; + }; + closure(); +} +"#, + expect!["99..165;49..54;120..121,133..134 ByRef(Mut { kind: Default }) a &'? mut A"], ); } From cc07652be5c3663e9dcb9a08dff3f9d253d60f5f Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Thu, 22 Aug 2024 14:27:35 +0300 Subject: [PATCH 3/9] Add `gen` modifier to functions We don't yet lower or maybe even parse them, but blocks already have `gen`, so why not. --- .../src/handlers/extract_function.rs | 1 + .../src/handlers/generate_delegate_methods.rs | 2 ++ .../src/handlers/generate_delegate_trait.rs | 1 + .../src/handlers/generate_function.rs | 1 + .../src/handlers/generate_getter_or_setter.rs | 28 +++++++++++++++++-- .../ide-assists/src/handlers/generate_new.rs | 1 + crates/syntax/rust.ungram | 2 +- crates/syntax/src/ast/generated/nodes.rs | 2 ++ crates/syntax/src/ast/make.rs | 4 ++- 9 files changed, 38 insertions(+), 4 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_function.rs b/crates/ide-assists/src/handlers/extract_function.rs index fd379cb88d..fd0b11f6dd 100644 --- a/crates/ide-assists/src/handlers/extract_function.rs +++ b/crates/ide-assists/src/handlers/extract_function.rs @@ -1617,6 +1617,7 @@ fn format_function( fun.control_flow.is_async, fun.mods.is_const, fun.control_flow.is_unsafe, + false, ) } diff --git a/crates/ide-assists/src/handlers/generate_delegate_methods.rs b/crates/ide-assists/src/handlers/generate_delegate_methods.rs index bbb902f8a2..081e36b4ff 100644 --- a/crates/ide-assists/src/handlers/generate_delegate_methods.rs +++ b/crates/ide-assists/src/handlers/generate_delegate_methods.rs @@ -122,6 +122,7 @@ pub(crate) fn generate_delegate_methods(acc: &mut Assists, ctx: &AssistContext<' let is_async = method_source.async_token().is_some(); let is_const = method_source.const_token().is_some(); let is_unsafe = method_source.unsafe_token().is_some(); + let is_gen = method_source.gen_token().is_some(); let fn_name = make::name(&name); @@ -154,6 +155,7 @@ pub(crate) fn generate_delegate_methods(acc: &mut Assists, ctx: &AssistContext<' is_async, is_const, is_unsafe, + is_gen, ) .clone_for_update(); diff --git a/crates/ide-assists/src/handlers/generate_delegate_trait.rs b/crates/ide-assists/src/handlers/generate_delegate_trait.rs index 933ab2058e..bf4ce5c907 100644 --- a/crates/ide-assists/src/handlers/generate_delegate_trait.rs +++ b/crates/ide-assists/src/handlers/generate_delegate_trait.rs @@ -740,6 +740,7 @@ fn func_assoc_item( item.async_token().is_some(), item.const_token().is_some(), item.unsafe_token().is_some(), + item.gen_token().is_some(), ) .clone_for_update(); diff --git a/crates/ide-assists/src/handlers/generate_function.rs b/crates/ide-assists/src/handlers/generate_function.rs index cd29f77f0c..76a647807c 100644 --- a/crates/ide-assists/src/handlers/generate_function.rs +++ b/crates/ide-assists/src/handlers/generate_function.rs @@ -365,6 +365,7 @@ impl FunctionBuilder { self.is_async, false, // FIXME : const and unsafe are not handled yet. false, + false, ) .clone_for_update(); diff --git a/crates/ide-assists/src/handlers/generate_getter_or_setter.rs b/crates/ide-assists/src/handlers/generate_getter_or_setter.rs index 8e349e84a9..c879a4a3d9 100644 --- a/crates/ide-assists/src/handlers/generate_getter_or_setter.rs +++ b/crates/ide-assists/src/handlers/generate_getter_or_setter.rs @@ -261,7 +261,19 @@ fn generate_getter_from_info( let ret_type = Some(make::ret_type(ty)); let body = make::block_expr([], Some(body)); - make::fn_(strukt.visibility(), fn_name, None, None, params, body, ret_type, false, false, false) + make::fn_( + strukt.visibility(), + fn_name, + None, + None, + params, + body, + ret_type, + false, + false, + false, + false, + ) } fn generate_setter_from_info(info: &AssistInfo, record_field_info: &RecordFieldInfo) -> ast::Fn { @@ -285,7 +297,19 @@ fn generate_setter_from_info(info: &AssistInfo, record_field_info: &RecordFieldI let body = make::block_expr([assign_stmt.into()], None); // Make the setter fn - make::fn_(strukt.visibility(), fn_name, None, None, params, body, None, false, false, false) + make::fn_( + strukt.visibility(), + fn_name, + None, + None, + params, + body, + None, + false, + false, + false, + false, + ) } fn extract_and_parse( diff --git a/crates/ide-assists/src/handlers/generate_new.rs b/crates/ide-assists/src/handlers/generate_new.rs index b9dede7cbd..70d14d6b95 100644 --- a/crates/ide-assists/src/handlers/generate_new.rs +++ b/crates/ide-assists/src/handlers/generate_new.rs @@ -115,6 +115,7 @@ pub(crate) fn generate_new(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option false, false, false, + false, ) .clone_for_update(); fn_.indent(1.into()); diff --git a/crates/syntax/rust.ungram b/crates/syntax/rust.ungram index 069be2df3a..7e2c88ff00 100644 --- a/crates/syntax/rust.ungram +++ b/crates/syntax/rust.ungram @@ -186,7 +186,7 @@ UseTreeList = Fn = Attr* Visibility? - 'default'? 'const'? 'async'? 'unsafe'? Abi? + 'default'? 'const'? 'async'? 'gen'? 'unsafe'? Abi? 'fn' Name GenericParamList? ParamList RetType? WhereClause? (body:BlockExpr | ';') diff --git a/crates/syntax/src/ast/generated/nodes.rs b/crates/syntax/src/ast/generated/nodes.rs index 01886d119d..25e49d3d65 100644 --- a/crates/syntax/src/ast/generated/nodes.rs +++ b/crates/syntax/src/ast/generated/nodes.rs @@ -484,6 +484,8 @@ impl Fn { #[inline] pub fn fn_token(&self) -> Option { support::token(&self.syntax, T![fn]) } #[inline] + pub fn gen_token(&self) -> Option { support::token(&self.syntax, T![gen]) } + #[inline] pub fn unsafe_token(&self) -> Option { support::token(&self.syntax, T![unsafe]) } } diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 00ce5c37cc..abf1a1f382 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -1035,6 +1035,7 @@ pub fn fn_( is_async: bool, is_const: bool, is_unsafe: bool, + is_gen: bool, ) -> ast::Fn { let type_params = match type_params { Some(type_params) => format!("{type_params}"), @@ -1056,9 +1057,10 @@ pub fn fn_( let async_literal = if is_async { "async " } else { "" }; let const_literal = if is_const { "const " } else { "" }; let unsafe_literal = if is_unsafe { "unsafe " } else { "" }; + let gen_literal = if is_gen { "gen " } else { "" }; ast_from_text(&format!( - "{visibility}{async_literal}{const_literal}{unsafe_literal}fn {fn_name}{type_params}{params} {ret_type}{where_clause}{body}", + "{visibility}{const_literal}{async_literal}{gen_literal}{unsafe_literal}fn {fn_name}{type_params}{params} {ret_type}{where_clause}{body}", )) } pub fn struct_( From 3712bb7f6aac7e9b761c48aba9c848cfc108e0a8 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Thu, 22 Aug 2024 14:30:54 +0300 Subject: [PATCH 4/9] Handle associated types that are lang items Previously we were ignoring them. --- crates/hir-def/src/lang_item.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/crates/hir-def/src/lang_item.rs b/crates/hir-def/src/lang_item.rs index a09fd658ae..f7c7f72139 100644 --- a/crates/hir-def/src/lang_item.rs +++ b/crates/hir-def/src/lang_item.rs @@ -117,11 +117,19 @@ impl LangItems { match def { ModuleDefId::TraitId(trait_) => { lang_items.collect_lang_item(db, trait_, LangItemTarget::Trait); - db.trait_data(trait_).items.iter().for_each(|&(_, assoc_id)| { - if let AssocItemId::FunctionId(f) = assoc_id { - lang_items.collect_lang_item(db, f, LangItemTarget::Function); - } - }); + db.trait_data(trait_).items.iter().for_each( + |&(_, assoc_id)| match assoc_id { + AssocItemId::FunctionId(f) => { + lang_items.collect_lang_item(db, f, LangItemTarget::Function); + } + AssocItemId::TypeAliasId(alias) => lang_items.collect_lang_item( + db, + alias, + LangItemTarget::TypeAlias, + ), + AssocItemId::ConstId(_) => {} + }, + ); } ModuleDefId::AdtId(AdtId::EnumId(e)) => { lang_items.collect_lang_item(db, e, LangItemTarget::EnumId); From 34920dde8d3f2d1ed1228785a8c42f9684eec8de Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Thu, 22 Aug 2024 14:33:35 +0300 Subject: [PATCH 5/9] Provide `Future::Output` and `Iterator` lang items --- crates/hir-def/src/lang_item.rs | 2 ++ crates/ide/src/hover/tests.rs | 6 +++--- crates/intern/src/symbol/symbols.rs | 2 ++ crates/test-utils/src/minicore.rs | 2 ++ 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/hir-def/src/lang_item.rs b/crates/hir-def/src/lang_item.rs index f7c7f72139..cdcd7208f3 100644 --- a/crates/hir-def/src/lang_item.rs +++ b/crates/hir-def/src/lang_item.rs @@ -461,6 +461,7 @@ language_item_table! { Context, sym::Context, context, Target::Struct, GenericRequirement::None; FuturePoll, sym::poll, future_poll_fn, Target::Method(MethodKind::Trait { body: false }), GenericRequirement::None; + FutureOutput, sym::future_output, future_output, Target::TypeAlias, GenericRequirement::None; Option, sym::Option, option_type, Target::Enum, GenericRequirement::None; OptionSome, sym::Some, option_some_variant, Target::Variant, GenericRequirement::None; @@ -475,6 +476,7 @@ language_item_table! { IntoFutureIntoFuture, sym::into_future, into_future_fn, Target::Method(MethodKind::Trait { body: false }), GenericRequirement::None; IntoIterIntoIter, sym::into_iter, into_iter_fn, Target::Method(MethodKind::Trait { body: false }), GenericRequirement::None; IteratorNext, sym::next, next_fn, Target::Method(MethodKind::Trait { body: false}), GenericRequirement::None; + Iterator, sym::iterator, iterator, Target::Trait, GenericRequirement::None; PinNewUnchecked, sym::new_unchecked, new_unchecked_fn, Target::Method(MethodKind::Inherent), GenericRequirement::None; diff --git a/crates/ide/src/hover/tests.rs b/crates/ide/src/hover/tests.rs index 8b60e562d7..9585bdbe4c 100644 --- a/crates/ide/src/hover/tests.rs +++ b/crates/ide/src/hover/tests.rs @@ -8465,7 +8465,7 @@ impl Iterator for S { file_id: FileId( 1, ), - full_range: 7800..8008, + full_range: 7800..8042, focus_range: 7865..7871, name: "Future", kind: Trait, @@ -8479,8 +8479,8 @@ impl Iterator for S { file_id: FileId( 1, ), - full_range: 8638..9104, - focus_range: 8682..8690, + full_range: 8672..9171, + focus_range: 8749..8757, name: "Iterator", kind: Trait, container_name: "iterator", diff --git a/crates/intern/src/symbol/symbols.rs b/crates/intern/src/symbol/symbols.rs index 2feca32ff8..7eb8e4a5e2 100644 --- a/crates/intern/src/symbol/symbols.rs +++ b/crates/intern/src/symbol/symbols.rs @@ -239,6 +239,7 @@ define_symbols! { fundamental, future_trait, future, + future_output, Future, ge, get_context, @@ -273,6 +274,7 @@ define_symbols! { iter_mut, iter, Iterator, + iterator, keyword, lang, le, diff --git a/crates/test-utils/src/minicore.rs b/crates/test-utils/src/minicore.rs index 2d615c34a3..7dbc498ead 100644 --- a/crates/test-utils/src/minicore.rs +++ b/crates/test-utils/src/minicore.rs @@ -1195,6 +1195,7 @@ pub mod future { #[doc(notable_trait)] #[lang = "future_trait"] pub trait Future { + #[lang = "future_output"] type Output; #[lang = "poll"] fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll; @@ -1292,6 +1293,7 @@ pub mod iter { mod traits { mod iterator { #[doc(notable_trait)] + #[lang = "iterator"] pub trait Iterator { type Item; #[lang = "next"] From e6d59e65ec354cba3a918d5761262821fe8e6d67 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Thu, 22 Aug 2024 14:35:39 +0300 Subject: [PATCH 6/9] Add helper methods to retrieve `Future::Output` and `Iterator::Item` --- crates/hir-def/src/lang_item.rs | 7 +++++++ crates/hir/src/lib.rs | 18 +++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/crates/hir-def/src/lang_item.rs b/crates/hir-def/src/lang_item.rs index cdcd7208f3..166c965d14 100644 --- a/crates/hir-def/src/lang_item.rs +++ b/crates/hir-def/src/lang_item.rs @@ -74,6 +74,13 @@ impl LangItemTarget { _ => None, } } + + pub fn as_type_alias(self) -> Option { + match self { + LangItemTarget::TypeAlias(id) => Some(id), + _ => None, + } + } } #[derive(Default, Debug, Clone, PartialEq, Eq)] diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 1e8127161d..2ff5f0d538 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -78,7 +78,7 @@ use hir_ty::{ use itertools::Itertools; use nameres::diagnostics::DefDiagnosticKind; use rustc_hash::FxHashSet; -use span::{Edition, EditionedFileId, FileId, MacroCallId}; +use span::{Edition, EditionedFileId, FileId, MacroCallId, SyntaxContextId}; use stdx::{impl_from, never}; use syntax::{ ast::{self, HasAttrs as _, HasGenericParams, HasName}, @@ -4379,6 +4379,22 @@ impl Type { method_resolution::implements_trait(&canonical_ty, db, &self.env, trait_) } + /// This does **not** resolve `IntoFuture`, only `Future`. + pub fn future_output(self, db: &dyn HirDatabase) -> Option { + let future_output = + db.lang_item(self.env.krate, LangItem::FutureOutput)?.as_type_alias()?; + self.normalize_trait_assoc_type(db, &[], future_output.into()) + } + + /// This does **not** resolve `IntoIterator`, only `Iterator`. + pub fn iterator_item(self, db: &dyn HirDatabase) -> Option { + let iterator_trait = db.lang_item(self.env.krate, LangItem::Iterator)?.as_trait()?; + let iterator_item = db + .trait_data(iterator_trait) + .associated_type_by_name(&Name::new_symbol(sym::Item.clone(), SyntaxContextId::ROOT))?; + self.normalize_trait_assoc_type(db, &[], iterator_item.into()) + } + /// Checks that particular type `ty` implements `std::ops::FnOnce`. /// /// This function can be used to check if a particular type is callable, since FnOnce is a From ddbb28daa0882306b383a2fb7c400a64c28e5d56 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Thu, 22 Aug 2024 14:39:59 +0300 Subject: [PATCH 7/9] Modify `hacks::parse_expr_from_str()` to take an edition too This will be needed as we parse unknown identifiers and want to insert them into source code. --- crates/ide-assists/src/handlers/remove_dbg.rs | 4 ++-- crates/ide-completion/src/completions/attribute.rs | 6 ++++-- crates/ide-db/src/syntax_helpers/node_ext.rs | 10 ++++++---- crates/syntax/src/hacks.rs | 4 ++-- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/crates/ide-assists/src/handlers/remove_dbg.rs b/crates/ide-assists/src/handlers/remove_dbg.rs index cffa3f55c9..0e9c463e02 100644 --- a/crates/ide-assists/src/handlers/remove_dbg.rs +++ b/crates/ide-assists/src/handlers/remove_dbg.rs @@ -1,7 +1,7 @@ use itertools::Itertools; use syntax::{ ast::{self, make, AstNode, AstToken}, - match_ast, ted, NodeOrToken, SyntaxElement, TextRange, TextSize, T, + match_ast, ted, Edition, NodeOrToken, SyntaxElement, TextRange, TextSize, T, }; use crate::{AssistContext, AssistId, AssistKind, Assists}; @@ -77,7 +77,7 @@ fn compute_dbg_replacement(macro_expr: ast::MacroExpr) -> Option<(TextRange, Opt let input_expressions = input_expressions .into_iter() .filter_map(|(is_sep, group)| (!is_sep).then_some(group)) - .map(|mut tokens| syntax::hacks::parse_expr_from_str(&tokens.join(""))) + .map(|mut tokens| syntax::hacks::parse_expr_from_str(&tokens.join(""), Edition::CURRENT)) .collect::>>()?; let parent = macro_expr.syntax().parent()?; diff --git a/crates/ide-completion/src/completions/attribute.rs b/crates/ide-completion/src/completions/attribute.rs index 75bd327608..9821fb4a2f 100644 --- a/crates/ide-completion/src/completions/attribute.rs +++ b/crates/ide-completion/src/completions/attribute.rs @@ -14,7 +14,7 @@ use ide_db::{ use itertools::Itertools; use syntax::{ ast::{self, AttrKind}, - AstNode, SyntaxKind, T, + AstNode, Edition, SyntaxKind, T, }; use crate::{ @@ -373,7 +373,9 @@ fn parse_comma_sep_expr(input: ast::TokenTree) -> Option> { input_expressions .into_iter() .filter_map(|(is_sep, group)| (!is_sep).then_some(group)) - .filter_map(|mut tokens| syntax::hacks::parse_expr_from_str(&tokens.join(""))) + .filter_map(|mut tokens| { + syntax::hacks::parse_expr_from_str(&tokens.join(""), Edition::CURRENT) + }) .collect::>(), ) } diff --git a/crates/ide-db/src/syntax_helpers/node_ext.rs b/crates/ide-db/src/syntax_helpers/node_ext.rs index fa82902a74..91e0b4495f 100644 --- a/crates/ide-db/src/syntax_helpers/node_ext.rs +++ b/crates/ide-db/src/syntax_helpers/node_ext.rs @@ -477,10 +477,12 @@ pub fn parse_tt_as_comma_sep_paths( .into_iter() .filter_map(|(is_sep, group)| (!is_sep).then_some(group)) .filter_map(|mut tokens| { - syntax::hacks::parse_expr_from_str(&tokens.join("")).and_then(|expr| match expr { - ast::Expr::PathExpr(it) => it.path(), - _ => None, - }) + syntax::hacks::parse_expr_from_str(&tokens.join(""), Edition::CURRENT).and_then( + |expr| match expr { + ast::Expr::PathExpr(it) => it.path(), + _ => None, + }, + ) }) .collect(); Some(paths) diff --git a/crates/syntax/src/hacks.rs b/crates/syntax/src/hacks.rs index 36615d11d8..9e63448ce9 100644 --- a/crates/syntax/src/hacks.rs +++ b/crates/syntax/src/hacks.rs @@ -6,9 +6,9 @@ use parser::Edition; use crate::{ast, AstNode}; -pub fn parse_expr_from_str(s: &str) -> Option { +pub fn parse_expr_from_str(s: &str, edition: Edition) -> Option { let s = s.trim(); - let file = ast::SourceFile::parse(&format!("const _: () = {s};"), Edition::CURRENT); + let file = ast::SourceFile::parse(&format!("const _: () = {s};"), edition); let expr = file.syntax_node().descendants().find_map(ast::Expr::cast)?; if expr.syntax().text() != s { return None; From 5c59a718c57c502602d048fb9f1447595ecd7960 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Thu, 22 Aug 2024 14:40:55 +0300 Subject: [PATCH 8/9] Provide `impl From for ast::GenericParam` --- crates/syntax/src/ast/node_ext.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/syntax/src/ast/node_ext.rs b/crates/syntax/src/ast/node_ext.rs index 5447906206..693bfe330b 100644 --- a/crates/syntax/src/ast/node_ext.rs +++ b/crates/syntax/src/ast/node_ext.rs @@ -17,7 +17,7 @@ use crate::{ ted, NodeOrToken, SmolStr, SyntaxElement, SyntaxToken, TokenText, T, }; -use super::{RangeItem, RangeOp}; +use super::{GenericParam, RangeItem, RangeOp}; impl ast::Lifetime { pub fn text(&self) -> TokenText<'_> { @@ -822,6 +822,15 @@ pub enum TypeOrConstParam { Const(ast::ConstParam), } +impl From for GenericParam { + fn from(value: TypeOrConstParam) -> Self { + match value { + TypeOrConstParam::Type(it) => GenericParam::TypeParam(it), + TypeOrConstParam::Const(it) => GenericParam::ConstParam(it), + } + } +} + impl TypeOrConstParam { pub fn name(&self) -> Option { match self { From 52462ada22e2938e6670e7c73c18d1b9a41aa4db Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Thu, 22 Aug 2024 14:41:54 +0300 Subject: [PATCH 9/9] Impl PartialEq and Eq for `IndentLevel` We can impl PartialOrd and Ord too, but I didn't need that. --- crates/syntax/src/ast/edit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/syntax/src/ast/edit.rs b/crates/syntax/src/ast/edit.rs index 5bc6b780e4..de40d638be 100644 --- a/crates/syntax/src/ast/edit.rs +++ b/crates/syntax/src/ast/edit.rs @@ -8,7 +8,7 @@ use crate::{ ted, AstToken, NodeOrToken, SyntaxElement, SyntaxNode, SyntaxToken, }; -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct IndentLevel(pub u8); impl From for IndentLevel {