Skip to content

Commit

Permalink
Auto merge of rust-lang#14065 - lowr:patch/generate-generic-function,…
Browse files Browse the repository at this point in the history
… r=Veykril

Support generic function in `generate_function` assist

Part of rust-lang#3639

This PR adds support for generic function generation in `generate_function` assist. Now the assist looks for generic parameters and trait bounds in scope, filters out irrelevant ones, and generates new function with them.

See `fn_generic_params()` for the outline of the procedure, and see comments on `filter_unnecessary_bounds()` for criteria for filtering. I think it's good criteria for most cases, but I'm open to opinions and suggestions.

The diff is pretty big, but it should run in linear time w.r.t. the number of nodes we operate on and should be fast enough.

Some notes:
- When we generate function in an existing impl, generic parameters may cause name conflict. While we can detect the conflict and rename conflicting params, I didn't find it worthwhile mainly because it's really easy to resolve on IDE: use Rename functionality.
- I've implemented graph structure myself, because we don't have graph library as a dependency and we only need the simplest one.
  - Although `petgraph` is in our dependency graph and I was initially looking to use it, we don't actually depend on it AFAICT since it's only used in chalk's specialization graph handling, which we don't use. I'd be happy to replace my implementation with `petgraph` if it's okay to use it though.
- There are some caveats that I consider out of scope of this PR. See FIXME notes on added tests.
  • Loading branch information
bors committed Feb 2, 2023
2 parents ccd142c + 493cabb commit eeceba7
Show file tree
Hide file tree
Showing 7 changed files with 980 additions and 64 deletions.
69 changes: 68 additions & 1 deletion crates/hir-ty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,13 @@ use std::sync::Arc;
use chalk_ir::{
fold::{Shift, TypeFoldable},
interner::HasInterner,
NoSolution,
visit::{TypeSuperVisitable, TypeVisitable, TypeVisitor},
NoSolution, TyData,
};
use hir_def::{expr::ExprId, type_ref::Rawness, TypeOrConstParamId};
use hir_expand::name;
use itertools::Either;
use rustc_hash::FxHashSet;
use traits::FnTrait;
use utils::Generics;

Expand Down Expand Up @@ -562,3 +564,68 @@ pub fn callable_sig_from_fnonce(

Some(CallableSig::from_params_and_return(params, ret_ty, false, Safety::Safe))
}

struct PlaceholderCollector<'db> {
db: &'db dyn HirDatabase,
placeholders: FxHashSet<TypeOrConstParamId>,
}

impl PlaceholderCollector<'_> {
fn collect(&mut self, idx: PlaceholderIndex) {
let id = from_placeholder_idx(self.db, idx);
self.placeholders.insert(id);
}
}

impl TypeVisitor<Interner> for PlaceholderCollector<'_> {
type BreakTy = ();

fn as_dyn(&mut self) -> &mut dyn TypeVisitor<Interner, BreakTy = Self::BreakTy> {
self
}

fn interner(&self) -> Interner {
Interner
}

fn visit_ty(
&mut self,
ty: &Ty,
outer_binder: DebruijnIndex,
) -> std::ops::ControlFlow<Self::BreakTy> {
let has_placeholder_bits = TypeFlags::HAS_TY_PLACEHOLDER | TypeFlags::HAS_CT_PLACEHOLDER;
let TyData { kind, flags } = ty.data(Interner);

if let TyKind::Placeholder(idx) = kind {
self.collect(*idx);
} else if flags.intersects(has_placeholder_bits) {
return ty.super_visit_with(self, outer_binder);
} else {
// Fast path: don't visit inner types (e.g. generic arguments) when `flags` indicate
// that there are no placeholders.
}

std::ops::ControlFlow::Continue(())
}

fn visit_const(
&mut self,
constant: &chalk_ir::Const<Interner>,
_outer_binder: DebruijnIndex,
) -> std::ops::ControlFlow<Self::BreakTy> {
if let chalk_ir::ConstValue::Placeholder(idx) = constant.data(Interner).value {
self.collect(idx);
}
std::ops::ControlFlow::Continue(())
}
}

/// Returns unique placeholders for types and consts contained in `value`.
pub fn collect_placeholders<T>(value: &T, db: &dyn HirDatabase) -> Vec<TypeOrConstParamId>
where
T: ?Sized + TypeVisitable<Interner>,
{
let mut collector = PlaceholderCollector { db, placeholders: FxHashSet::default() };
value.visit_with(&mut collector, DebruijnIndex::INNERMOST);
collector.placeholders.into_iter().collect()
}
34 changes: 30 additions & 4 deletions crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2165,6 +2165,16 @@ impl AsAssocItem for ModuleDef {
}
}
}
impl AsAssocItem for DefWithBody {
fn as_assoc_item(self, db: &dyn HirDatabase) -> Option<AssocItem> {
match self {
DefWithBody::Function(it) => it.as_assoc_item(db),
DefWithBody::Const(it) => it.as_assoc_item(db),
DefWithBody::Static(_) | DefWithBody::Variant(_) => None,
}
}
}

fn as_assoc_item<ID, DEF, CTOR, AST>(db: &dyn HirDatabase, ctor: CTOR, id: ID) -> Option<AssocItem>
where
ID: Lookup<Data = AssocItemLoc<AST>>,
Expand Down Expand Up @@ -2565,6 +2575,14 @@ impl GenericParam {
GenericParam::LifetimeParam(it) => it.name(db),
}
}

pub fn parent(self) -> GenericDef {
match self {
GenericParam::TypeParam(it) => it.id.parent().into(),
GenericParam::ConstParam(it) => it.id.parent().into(),
GenericParam::LifetimeParam(it) => it.id.parent.into(),
}
}
}

#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
Expand Down Expand Up @@ -3144,15 +3162,15 @@ impl Type {
}

pub fn is_closure(&self) -> bool {
matches!(&self.ty.kind(Interner), TyKind::Closure { .. })
matches!(self.ty.kind(Interner), TyKind::Closure { .. })
}

pub fn is_fn(&self) -> bool {
matches!(&self.ty.kind(Interner), TyKind::FnDef(..) | TyKind::Function { .. })
matches!(self.ty.kind(Interner), TyKind::FnDef(..) | TyKind::Function { .. })
}

pub fn is_array(&self) -> bool {
matches!(&self.ty.kind(Interner), TyKind::Array(..))
matches!(self.ty.kind(Interner), TyKind::Array(..))
}

pub fn is_packed(&self, db: &dyn HirDatabase) -> bool {
Expand All @@ -3169,7 +3187,7 @@ impl Type {
}

pub fn is_raw_ptr(&self) -> bool {
matches!(&self.ty.kind(Interner), TyKind::Raw(..))
matches!(self.ty.kind(Interner), TyKind::Raw(..))
}

pub fn contains_unknown(&self) -> bool {
Expand Down Expand Up @@ -3604,6 +3622,14 @@ impl Type {
_ => None,
}
}

/// Returns unique `GenericParam`s contained in this type.
pub fn generic_params(&self, db: &dyn HirDatabase) -> FxHashSet<GenericParam> {
hir_ty::collect_placeholders(&self.ty, db)
.into_iter()
.map(|id| TypeOrConstParam { id }.split(db).either_into())
.collect()
}
}

#[derive(Debug)]
Expand Down
7 changes: 2 additions & 5 deletions crates/hir/src/semantics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1319,10 +1319,7 @@ impl<'db> SemanticsImpl<'db> {
let _p = profile::span("Semantics::analyze_impl");
let node = self.find_file(node);

let container = match self.with_ctx(|ctx| ctx.find_container(node)) {
Some(it) => it,
None => return None,
};
let container = self.with_ctx(|ctx| ctx.find_container(node))?;

let resolver = match container {
ChildContainer::DefWithBodyId(def) => {
Expand Down Expand Up @@ -1582,7 +1579,7 @@ fn find_root(node: &SyntaxNode) -> SyntaxNode {
node.ancestors().last().unwrap()
}

/// `SemanticScope` encapsulates the notion of a scope (the set of visible
/// `SemanticsScope` encapsulates the notion of a scope (the set of visible
/// names) at a particular program point.
///
/// It is a bit tricky, as scopes do not really exist inside the compiler.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ pub(crate) fn generate_delegate_methods(acc: &mut Assists, ctx: &AssistContext<'
let tail_expr_finished =
if is_async { make::expr_await(tail_expr) } else { tail_expr };
let body = make::block_expr([], Some(tail_expr_finished));
let f = make::fn_(vis, name, type_params, params, body, ret_type, is_async)
let f = make::fn_(vis, name, type_params, None, params, body, ret_type, is_async)
.indent(ast::edit::IndentLevel(1))
.clone_for_update();

Expand Down
Loading

0 comments on commit eeceba7

Please sign in to comment.