Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions bench_data/glorious_old_parser
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//- minicore: fn
Copy link
Copy Markdown
Member

@Veykril Veykril Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious where we panic without language items, ideally that wouldn't happen (even if in practice no one will ever write a no_core rust program without the fn traits ...). Just looks a bit odd to add a mini core directive here haha

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should add this directive as enabled by default for tests now

Copy link
Copy Markdown
Contributor Author

@ChayimFriedman2 ChayimFriedman2 Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It panicked in the past as Expected to find a suitable `Fn`/`FnMut`/`FnOnce` implementation for ... in callee.rs, this was copied from rustc and I preferred not to change it. But I was forced to, because it panicked in the sysroot run in CI (it doesn't have a proper sysroot configured), and there I could not inject a minicore 😅

use crate::ast::{AngleBracketedArgs, ParenthesizedArgs, AttrStyle, BareFnTy};
use crate::ast::{GenericBound, TraitBoundModifier};
use crate::ast::Unsafety;
Expand Down
10 changes: 10 additions & 0 deletions crates/hir-def/src/expr_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,9 @@ impl ExpressionStore {
f(*expr);
arms.iter().for_each(|arm| {
f(arm.expr);
if let Some(guard) = arm.guard {
Copy link
Copy Markdown
Member

@Veykril Veykril Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In these kinds of functions we should really enforce restructuring patterns to catch things 😅

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, but it's better to leave that to another PR.

f(guard);
}
self.walk_exprs_in_pat(arm.pat, &mut f);
});
}
Expand Down Expand Up @@ -926,6 +929,13 @@ impl ExpressionStore {
// We keep the async closure exactly one expr before.
ExprId::from_raw(la_arena::RawIdx::from_u32(coroutine_closure.into_raw().into_u32() - 1))
}

/// The opposite of [`Self::coroutine_for_closure()`].
#[inline]
pub fn closure_for_coroutine(coroutine: ExprId) -> ExprId {
// We keep the async closure exactly one expr before.
ExprId::from_raw(la_arena::RawIdx::from_u32(coroutine.into_raw().into_u32() + 1))
}
}

impl Index<ExprId> for ExpressionStore {
Expand Down
4 changes: 2 additions & 2 deletions crates/hir-def/src/expr_store/body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ impl Body {
expr: ExprId,
edition: Edition,
) -> String {
pretty::print_expr_hir(db, self, owner, expr, edition)
pretty::print_expr_hir(db, self, owner.into(), expr, edition)
}

pub fn pretty_print_pat(
Expand All @@ -144,7 +144,7 @@ impl Body {
oneline: bool,
edition: Edition,
) -> String {
pretty::print_pat_hir(db, self, owner, pat, oneline, edition)
pretty::print_pat_hir(db, self, owner.into(), pat, oneline, edition)
}
}

Expand Down
87 changes: 57 additions & 30 deletions crates/hir-def/src/expr_store/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -945,12 +945,19 @@ impl<'db> ExprCollector<'db> {
})
}

/// An `async fn` needs to capture all parameters in the generated `async` block, even if they have
/// non-captured patterns such as wildcards (to ensure consistent drop order).
fn lower_async_fn(&mut self, params: &mut Vec<PatId>, body: ExprId) -> ExprId {
/// Lowers a desugared coroutine body after moving all of the arguments
/// into the body. This is to make sure that the future actually owns the
/// arguments that are passed to the function, and to ensure things like
/// drop order are stable.
fn lower_async_block_with_moved_arguments(
&mut self,
params: &mut [PatId],
body: ExprId,
coroutine_source: CoroutineSource,
) -> ExprId {
let mut statements = Vec::new();
for param in params {
let name = match self.store.pats[*param] {
let (name, hygiene) = match self.store.pats[*param] {
Pat::Bind { id, .. }
if matches!(
self.store.bindings[id].mode,
Expand All @@ -962,14 +969,16 @@ impl<'db> ExprCollector<'db> {
}
Pat::Bind { id, .. } => {
// If this is a `ref` binding, we can't leave it as is but we can at least reuse the name, for better display.
self.store.bindings[id].name.clone()
(self.store.bindings[id].name.clone(), self.store.bindings[id].hygiene)
}
_ => self.generate_new_name(),
_ => (self.generate_new_name(), HygieneId::ROOT),
};
let binding_id =
self.alloc_binding(name.clone(), BindingAnnotation::Mutable, HygieneId::ROOT);
let binding_id = self.alloc_binding(name.clone(), BindingAnnotation::Mutable, hygiene);
let pat_id = self.alloc_pat_desugared(Pat::Bind { id: binding_id, subpat: None });
let expr = self.alloc_expr_desugared(Expr::Path(name.into()));
if !hygiene.is_root() {
self.store.ident_hygiene.insert(expr.into(), hygiene);
}
statements.push(Statement::Let {
pat: *param,
type_ref: None,
Expand All @@ -980,12 +989,17 @@ impl<'db> ExprCollector<'db> {
}

let async_ = self.async_block(
CoroutineSource::Fn,
CaptureBy::Value,
coroutine_source,
// The default capture mode here is by-ref. Later on during upvar analysis,
// we will force the captured arguments to by-move, but for async closures,
// we want to make sure that we avoid unnecessarily moving captures, or else
// all async closures would default to `FnOnce` as their calling mode.
CaptureBy::Ref,
None,
statements.into_boxed_slice(),
Some(body),
);
// It's important that this comes last, see the lowering of async closures for why.
self.alloc_expr_desugared(async_)
}

Expand All @@ -1010,14 +1024,18 @@ impl<'db> ExprCollector<'db> {

fn collect(
&mut self,
params: &mut Vec<PatId>,
params: &mut [PatId],
expr: Option<ast::Expr>,
awaitable: Awaitable,
) -> ExprId {
self.awaitable_context.replace(awaitable);
self.with_label_rib(RibKind::Closure, |this| {
let body = this.collect_expr_opt(expr);
if awaitable == Awaitable::Yes { this.lower_async_fn(params, body) } else { body }
if awaitable == Awaitable::Yes {
this.lower_async_block_with_moved_arguments(params, body, CoroutineSource::Fn)
} else {
body
}
})
}

Expand Down Expand Up @@ -1407,9 +1425,11 @@ impl<'db> ExprCollector<'db> {
}
}
ast::Expr::ClosureExpr(e) => self.with_label_rib(RibKind::Closure, |this| {
this.with_binding_owner(|this| {
this.with_binding_owner_and_return(|this| {
let mut args = Vec::new();
let mut arg_types = Vec::new();
// For coroutine closures, the body, aka. the coroutine is the bindings owner, and not the closure.
let mut body_is_bindings_owner = false;
if let Some(pl) = e.param_list() {
let num_params = pl.params().count();
args.reserve_exact(num_params);
Expand Down Expand Up @@ -1448,18 +1468,12 @@ impl<'db> ExprCollector<'db> {
} else if e.async_token().is_some() {
// It's important that this expr is allocated immediately before the closure.
// We rely on it for `coroutine_for_closure()`.
body = this.alloc_expr_desugared(Expr::Closure {
args: Box::default(),
arg_types: Box::default(),
ret_type: None,
body = this.lower_async_block_with_moved_arguments(
&mut args,
body,
closure_kind: ClosureKind::AsyncBlock {
source: CoroutineSource::Closure,
},
// The block may need to capture by move, but we cannot know it now.
// It will be fixed in capture analysis.
capture_by: CaptureBy::Ref,
});
CoroutineSource::Closure,
);
body_is_bindings_owner = true;

ClosureKind::AsyncClosure
} else {
Expand All @@ -1469,7 +1483,7 @@ impl<'db> ExprCollector<'db> {
if e.move_token().is_some() { CaptureBy::Value } else { CaptureBy::Ref };
this.is_lowering_coroutine = prev_is_lowering_coroutine;
this.current_try_block = prev_try_block;
this.alloc_expr(
let closure = this.alloc_expr(
Expr::Closure {
args: args.into(),
arg_types: arg_types.into(),
Expand All @@ -1479,7 +1493,9 @@ impl<'db> ExprCollector<'db> {
capture_by,
},
syntax_ptr,
)
);

(if body_is_bindings_owner { body } else { closure }, closure)
})
}),
ast::Expr::BinExpr(e) => {
Expand Down Expand Up @@ -1781,13 +1797,24 @@ impl<'db> ExprCollector<'db> {
}
}

fn with_binding_owner(&mut self, create_expr: impl FnOnce(&mut Self) -> ExprId) -> ExprId {
/// The callback should return two exprs: the first is the bindings owner, the second is the expr to return.
fn with_binding_owner_and_return(
&mut self,
create_expr: impl FnOnce(&mut Self) -> (ExprId, ExprId),
) -> ExprId {
let prev_unowned_bindings_len = self.unowned_bindings.len();
let expr_id = create_expr(self);
let (bindings_owner, expr_to_return) = create_expr(self);
for binding in self.unowned_bindings.drain(prev_unowned_bindings_len..) {
self.store.binding_owners.insert(binding, expr_id);
self.store.binding_owners.insert(binding, bindings_owner);
}
expr_id
expr_to_return
}

fn with_binding_owner(&mut self, create_expr: impl FnOnce(&mut Self) -> ExprId) -> ExprId {
self.with_binding_owner_and_return(move |this| {
let expr = create_expr(this);
(expr, expr)
})
}

/// Desugar `try { <stmts>; <expr> }` into `'<new_label>: { <stmts>; ::std::ops::Try::from_output(<expr>) }`,
Expand Down
4 changes: 2 additions & 2 deletions crates/hir-def/src/expr_store/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ fn print_generic_params(db: &dyn DefDatabase, generic_params: &GenericParams, p:
pub fn print_expr_hir(
db: &dyn DefDatabase,
store: &ExpressionStore,
_owner: DefWithBodyId,
_owner: ExpressionStoreOwnerId,
expr: ExprId,
edition: Edition,
) -> String {
Expand All @@ -420,7 +420,7 @@ pub fn print_expr_hir(
pub fn print_pat_hir(
db: &dyn DefDatabase,
store: &ExpressionStore,
_owner: DefWithBodyId,
_owner: ExpressionStoreOwnerId,
pat: PatId,
oneline: bool,
edition: Edition,
Expand Down
2 changes: 1 addition & 1 deletion crates/hir-def/src/expr_store/tests/body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ fn async_fn_weird_param_patterns() {
async fn main(&self, param1: i32, ref mut param2: i32, _: i32, param4 @ _: i32, 123: i32) {}
"#,
expect![[r#"
fn main(self, param1, mut param2, mut <ra@gennew>0, param4 @ _, mut <ra@gennew>1) async move {
fn main(self, param1, mut param2, mut <ra@gennew>0, param4 @ _, mut <ra@gennew>1) async {
let ref mut param2 = param2;
let _ = <ra@gennew>0;
let 123 = <ra@gennew>1;
Expand Down
10 changes: 10 additions & 0 deletions crates/hir-def/src/lang_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ language_item_table! { LangItems =>
/// Trait injected by `#[derive(Eq)]`, (i.e. "Total EQ"; no, I will not apologize).
StructuralTeq, sym::structural_teq, TraitId;
Copy, sym::copy, TraitId;
UseCloned, sym::use_cloned, TraitId;
Clone, sym::clone, TraitId;
TrivialClone, sym::trivial_clone, TraitId;
Sync, sym::sync, TraitId;
Expand All @@ -324,6 +325,7 @@ language_item_table! { LangItems =>

Drop, sym::drop, TraitId;
Destruct, sym::destruct, TraitId;
BikeshedGuaranteedNoDrop,sym::bikeshed_guaranteed_no_drop, TraitId;

CoerceUnsized, sym::coerce_unsized, TraitId;
DispatchFromDyn, sym::dispatch_from_dyn, TraitId;
Expand Down Expand Up @@ -373,6 +375,8 @@ language_item_table! { LangItems =>
AsyncFn, sym::async_fn, TraitId;
AsyncFnMut, sym::async_fn_mut, TraitId;
AsyncFnOnce, sym::async_fn_once, TraitId;
AsyncFnKindHelper, sym::async_fn_kind_helper,TraitId;
AsyncFnKindUpvars, sym::async_fn_kind_upvars,TypeAliasId;

CallRefFuture, sym::call_ref_future, TypeAliasId;
CallOnceFuture, sym::call_once_future, TypeAliasId;
Expand Down Expand Up @@ -489,6 +493,8 @@ language_item_table! { LangItems =>
IntoIterIntoIter, sym::into_iter, FunctionId;
IteratorNext, sym::next, FunctionId;
Iterator, sym::iterator, TraitId;
FusedIterator, sym::fused_iterator, TraitId;
AsyncIterator, sym::async_iterator, TraitId;

PinNewUnchecked, sym::new_unchecked, FunctionId;

Expand All @@ -509,6 +515,10 @@ language_item_table! { LangItems =>
CStr, sym::CStr, StructId;
Ordering, sym::Ordering, EnumId;

Field, sym::field, TraitId;
FieldBase, sym::field_base, TypeAliasId;
FieldType, sym::field_type, TypeAliasId;

@non_lang_core_traits:
core::default, Default;
core::fmt, Debug;
Expand Down
6 changes: 3 additions & 3 deletions crates/hir-ty/src/consteval/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1792,14 +1792,14 @@ const GOAL: i32 = {
fn closure_capture_unsized_type() {
check_number(
r#"
//- minicore: fn, copy, slice, index, coerce_unsized
//- minicore: fn, copy, slice, index, coerce_unsized, sized
fn f<T: A>(x: &<T as A>::Ty) -> &<T as A>::Ty {
let c = || &*x;
c()
}

trait A {
type Ty;
type Ty: ?Sized;
}

impl A for i32 {
Expand All @@ -1810,7 +1810,7 @@ fn closure_capture_unsized_type() {
let k: &[u8] = &[1, 2, 3];
let k = f::<i32>(k);
k[0] + k[1] + k[2]
}
};
"#,
6,
);
Expand Down
3 changes: 1 addition & 2 deletions crates/hir-ty/src/diagnostics/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,7 @@ impl<'db> ExprValidator<'db> {
if (pat_ty == scrut_ty
|| scrut_ty
.as_reference()
.map(|(match_expr_ty, ..)| match_expr_ty == pat_ty)
.unwrap_or(false))
.is_none_or(|(match_expr_ty, ..)| match_expr_ty == pat_ty))
&& types_of_subpatterns_do_match(arm.pat, self.body, self.infer)
{
// If we had a NotUsefulMatchArm diagnostic, we could
Expand Down
6 changes: 6 additions & 0 deletions crates/hir-ty/src/diagnostics/match_check/pat_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ pub(crate) trait EnumerateAndAdjustIterator {
}

impl<T: ExactSizeIterator> EnumerateAndAdjustIterator for T {
/// When there is a list of items with a gap of an unknown length inside, and another list
/// of item it should be zipped against, this operates on the list with the gap and returns,
/// for each item, the index it should match in the other list.
///
/// When compiling Rust, such situation often occurs for tuple structs/tuples with a rest pattern
/// that should be matched against the fields.
fn enumerate_and_adjust(
self,
expected_len: usize,
Expand Down
6 changes: 2 additions & 4 deletions crates/hir-ty/src/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ use span::Edition;
use stdx::never;

use crate::{
CallableDefId, FnAbi, ImplTraitId, InferenceResult, MemoryMap, ParamEnvAndCrate, consteval,
CallableDefId, FnAbi, ImplTraitId, MemoryMap, ParamEnvAndCrate, consteval,
db::{HirDatabase, InternedClosure},
generics::generics,
layout::Layout,
Expand Down Expand Up @@ -1495,9 +1495,7 @@ impl<'db> HirDisplay<'db> for Ty<'db> {
}
let sig = interner.signature_unclosure(substs.as_closure().sig(), Safety::Safe);
let sig = sig.skip_binder();
let InternedClosure(owner, _) = id.loc(db);
let infer = InferenceResult::of(db, owner);
let (_, kind) = infer.closure_info(id);
let kind = substs.as_closure().kind();
match f.closure_style {
ClosureStyle::ImplFn => write!(f, "impl {kind:?}(")?,
ClosureStyle::RANotation => write!(f, "|")?,
Expand Down
Loading