Skip to content

Commit

Permalink
Rollup merge of rust-lang#129493 - cjgillot:early-opaque-def, r=petro…
Browse files Browse the repository at this point in the history
…chenkov

Create opaque definitions in resolver.

Implementing rust-lang#129023 (comment)

That was easier than I expected.

r? `@petrochenkov`
  • Loading branch information
matthiaskrgr authored Sep 1, 2024
2 parents 1063c0d + f68f665 commit 07d5c25
Show file tree
Hide file tree
Showing 8 changed files with 91 additions and 97 deletions.
16 changes: 10 additions & 6 deletions compiler/rustc_ast/src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,14 @@ pub enum FnKind<'a> {
Fn(FnCtxt, Ident, &'a FnSig, &'a Visibility, &'a Generics, Option<&'a Block>),

/// E.g., `|x, y| body`.
Closure(&'a ClosureBinder, &'a FnDecl, &'a Expr),
Closure(&'a ClosureBinder, &'a Option<CoroutineKind>, &'a FnDecl, &'a Expr),
}

impl<'a> FnKind<'a> {
pub fn header(&self) -> Option<&'a FnHeader> {
match *self {
FnKind::Fn(_, _, sig, _, _, _) => Some(&sig.header),
FnKind::Closure(_, _, _) => None,
FnKind::Closure(..) => None,
}
}

Expand All @@ -90,7 +90,7 @@ impl<'a> FnKind<'a> {
pub fn decl(&self) -> &'a FnDecl {
match self {
FnKind::Fn(_, _, sig, _, _, _) => &sig.decl,
FnKind::Closure(_, decl, _) => decl,
FnKind::Closure(_, _, decl, _) => decl,
}
}

Expand Down Expand Up @@ -839,7 +839,7 @@ pub fn walk_fn<'a, V: Visitor<'a>>(visitor: &mut V, kind: FnKind<'a>) -> V::Resu
try_visit!(walk_fn_decl(visitor, decl));
visit_opt!(visitor, visit_block, body);
}
FnKind::Closure(binder, decl, body) => {
FnKind::Closure(binder, _coroutine_kind, decl, body) => {
try_visit!(visitor.visit_closure_binder(binder));
try_visit!(walk_fn_decl(visitor, decl));
try_visit!(visitor.visit_expr(body));
Expand Down Expand Up @@ -1107,7 +1107,7 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) -> V
ExprKind::Closure(box Closure {
binder,
capture_clause,
coroutine_kind: _,
coroutine_kind,
constness: _,
movability: _,
fn_decl,
Expand All @@ -1116,7 +1116,11 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) -> V
fn_arg_span: _,
}) => {
try_visit!(visitor.visit_capture_by(capture_clause));
try_visit!(visitor.visit_fn(FnKind::Closure(binder, fn_decl, body), *span, *id))
try_visit!(visitor.visit_fn(
FnKind::Closure(binder, coroutine_kind, fn_decl, body),
*span,
*id
))
}
ExprKind::Block(block, opt_label) => {
visit_opt!(visitor, visit_label, opt_label);
Expand Down
30 changes: 4 additions & 26 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ use std::collections::hash_map::Entry;
use rustc_ast::node_id::NodeMap;
use rustc_ast::ptr::P;
use rustc_ast::{self as ast, *};
use rustc_ast_pretty::pprust;
use rustc_data_structures::captures::Captures;
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::fx::FxIndexSet;
Expand Down Expand Up @@ -1399,24 +1398,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
self.tcx.dcx().emit_err(errors::NoPreciseCapturesOnApit { span });
}

let span = t.span;

// HACK: pprust breaks strings with newlines when the type
// gets too long. We don't want these to show up in compiler
// output or built artifacts, so replace them here...
// Perhaps we should instead format APITs more robustly.
let ident = Ident::from_str_and_span(
&pprust::ty_to_string(t).replace('\n', " "),
span,
);

self.create_def(
self.current_hir_id_owner.def_id, // FIXME: should this use self.current_def_id_parent?
*def_node_id,
ident.name,
DefKind::TyParam,
span,
);
let def_id = self.local_def_id(*def_node_id);
let name = self.tcx.item_name(def_id.to_def_id());
let ident = Ident::new(name, span);
let (param, bounds, path) = self.lower_universal_param_and_bounds(
*def_node_id,
span,
Expand Down Expand Up @@ -1618,13 +1602,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
opaque_ty_span: Span,
lower_item_bounds: impl FnOnce(&mut Self) -> &'hir [hir::GenericBound<'hir>],
) -> hir::TyKind<'hir> {
let opaque_ty_def_id = self.create_def(
self.current_hir_id_owner.def_id, // FIXME: should this use self.current_def_id_parent?
opaque_ty_node_id,
kw::Empty,
DefKind::OpaqueTy,
opaque_ty_span,
);
let opaque_ty_def_id = self.local_def_id(opaque_ty_node_id);
debug!(?opaque_ty_def_id);

// Map from captured (old) lifetime to synthetic (new) lifetime.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1485,7 +1485,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {

let disallowed = (!tilde_const_allowed).then(|| match fk {
FnKind::Fn(_, ident, _, _, _, _) => TildeConstReason::Function { ident: ident.span },
FnKind::Closure(_, _, _) => TildeConstReason::Closure,
FnKind::Closure(..) => TildeConstReason::Closure,
});
self.with_tilde_const(disallowed, |this| visit::walk_fn(this, fk));
}
Expand Down
116 changes: 64 additions & 52 deletions compiler/rustc_resolve/src/def_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::mem;

use rustc_ast::visit::FnKind;
use rustc_ast::*;
use rustc_ast_pretty::pprust;
use rustc_expand::expand::AstFragment;
use rustc_hir as hir;
use rustc_hir::def::{CtorKind, CtorOf, DefKind};
Expand Down Expand Up @@ -120,8 +121,6 @@ impl<'a, 'b, 'tcx> DefCollector<'a, 'b, 'tcx> {

impl<'a, 'b, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'b, 'tcx> {
fn visit_item(&mut self, i: &'a Item) {
debug!("visit_item: {:?}", i);

// Pick the def data. This need not be unique, but the more
// information we encapsulate into, the better
let mut opt_macro_data = None;
Expand Down Expand Up @@ -183,38 +182,51 @@ impl<'a, 'b, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'b, 'tcx> {
}

fn visit_fn(&mut self, fn_kind: FnKind<'a>, span: Span, _: NodeId) {
if let FnKind::Fn(_, _, sig, _, generics, body) = fn_kind {
match sig.header.coroutine_kind {
Some(coroutine_kind) => {
self.visit_generics(generics);

// For async functions, we need to create their inner defs inside of a
// closure to match their desugared representation. Besides that,
// we must mirror everything that `visit::walk_fn` below does.
self.visit_fn_header(&sig.header);
for param in &sig.decl.inputs {
self.visit_param(param);
}
self.visit_fn_ret_ty(&sig.decl.output);
// If this async fn has no body (i.e. it's an async fn signature in a trait)
// then the closure_def will never be used, and we should avoid generating a
// def-id for it.
if let Some(body) = body {
let closure_def = self.create_def(
coroutine_kind.closure_id(),
kw::Empty,
DefKind::Closure,
span,
);
self.with_parent(closure_def, |this| this.visit_block(body));
}
return;
match fn_kind {
FnKind::Fn(_ctxt, _ident, FnSig { header, decl, span: _ }, _vis, generics, body)
if let Some(coroutine_kind) = header.coroutine_kind =>
{
self.visit_fn_header(header);
self.visit_generics(generics);

// For async functions, we need to create their inner defs inside of a
// closure to match their desugared representation. Besides that,
// we must mirror everything that `visit::walk_fn` below does.
let FnDecl { inputs, output } = &**decl;
for param in inputs {
self.visit_param(param);
}

let (return_id, return_span) = coroutine_kind.return_id();
let return_def =
self.create_def(return_id, kw::Empty, DefKind::OpaqueTy, return_span);
self.with_parent(return_def, |this| this.visit_fn_ret_ty(output));

// If this async fn has no body (i.e. it's an async fn signature in a trait)
// then the closure_def will never be used, and we should avoid generating a
// def-id for it.
if let Some(body) = body {
let closure_def = self.create_def(
coroutine_kind.closure_id(),
kw::Empty,
DefKind::Closure,
span,
);
self.with_parent(closure_def, |this| this.visit_block(body));
}
None => {}
}
}
FnKind::Closure(binder, Some(coroutine_kind), decl, body) => {
self.visit_closure_binder(binder);
visit::walk_fn_decl(self, decl);

visit::walk_fn(self, fn_kind);
// Async closures desugar to closures inside of closures, so
// we must create two defs.
let coroutine_def =
self.create_def(coroutine_kind.closure_id(), kw::Empty, DefKind::Closure, span);
self.with_parent(coroutine_def, |this| visit::walk_expr(this, body));
}
_ => visit::walk_fn(self, fn_kind),
}
}

fn visit_use_tree(&mut self, use_tree: &'a UseTree, id: NodeId, _nested: bool) {
Expand Down Expand Up @@ -334,27 +346,7 @@ impl<'a, 'b, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'b, 'tcx> {
fn visit_expr(&mut self, expr: &'a Expr) {
let parent_def = match expr.kind {
ExprKind::MacCall(..) => return self.visit_macro_invoc(expr.id),
ExprKind::Closure(ref closure) => {
// Async closures desugar to closures inside of closures, so
// we must create two defs.
let closure_def = self.create_def(expr.id, kw::Empty, DefKind::Closure, expr.span);
match closure.coroutine_kind {
Some(coroutine_kind) => {
self.with_parent(closure_def, |this| {
let coroutine_def = this.create_def(
coroutine_kind.closure_id(),
kw::Empty,
DefKind::Closure,
expr.span,
);
this.with_parent(coroutine_def, |this| visit::walk_expr(this, expr));
});
return;
}
None => closure_def,
}
}
ExprKind::Gen(_, _, _, _) => {
ExprKind::Closure(..) | ExprKind::Gen(..) => {
self.create_def(expr.id, kw::Empty, DefKind::Closure, expr.span)
}
ExprKind::ConstBlock(ref constant) => {
Expand All @@ -381,6 +373,26 @@ impl<'a, 'b, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'b, 'tcx> {
TyKind::MacCall(..) => self.visit_macro_invoc(ty.id),
// Anonymous structs or unions are visited later after defined.
TyKind::AnonStruct(..) | TyKind::AnonUnion(..) => {}
TyKind::ImplTrait(id, _) => {
// HACK: pprust breaks strings with newlines when the type
// gets too long. We don't want these to show up in compiler
// output or built artifacts, so replace them here...
// Perhaps we should instead format APITs more robustly.
let name = Symbol::intern(&pprust::ty_to_string(ty).replace('\n', " "));
let kind = match self.impl_trait_context {
ImplTraitContext::Universal => DefKind::TyParam,
ImplTraitContext::Existential => DefKind::OpaqueTy,
};
let id = self.create_def(*id, name, kind, ty.span);
match self.impl_trait_context {
// Do not nest APIT, as we desugar them as `impl_trait: bounds`,
// so the `impl_trait` node is not a parent to `bounds`.
ImplTraitContext::Universal => visit::walk_ty(self, ty),
ImplTraitContext::Existential => {
self.with_parent(id, |this| visit::walk_ty(this, ty))
}
};
}
_ => visit::walk_ty(self, ty),
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1010,7 +1010,7 @@ impl<'a: 'ast, 'ast, 'tcx> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast,
this.in_func_body = previous_state;
}
}
FnKind::Closure(binder, declaration, body) => {
FnKind::Closure(binder, _, declaration, body) => {
this.visit_closure_binder(binder);

this.with_lifetime_rib(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
field_tys: {
_0: CoroutineSavedTy {
ty: Coroutine(
DefId(0:4 ~ async_await[ccf8]::a::{closure#0}),
DefId(0:5 ~ async_await[ccf8]::a::{closure#0}),
[
(),
std::future::ResumeTy,
(),
(),
CoroutineWitness(
DefId(0:4 ~ async_await[ccf8]::a::{closure#0}),
DefId(0:5 ~ async_await[ccf8]::a::{closure#0}),
[],
),
(),
Expand All @@ -24,14 +24,14 @@
},
_1: CoroutineSavedTy {
ty: Coroutine(
DefId(0:4 ~ async_await[ccf8]::a::{closure#0}),
DefId(0:5 ~ async_await[ccf8]::a::{closure#0}),
[
(),
std::future::ResumeTy,
(),
(),
CoroutineWitness(
DefId(0:4 ~ async_await[ccf8]::a::{closure#0}),
DefId(0:5 ~ async_await[ccf8]::a::{closure#0}),
[],
),
(),
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/coroutine/print/coroutine-print-verbose-1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ note: coroutine is not `Send` as this value is used across a yield
--> $DIR/coroutine-print-verbose-1.rs:35:9
|
LL | let _non_send_gen = make_non_send_coroutine();
| ------------- has type `Opaque(DefId(0:34 ~ coroutine_print_verbose_1[75fb]::make_non_send_coroutine::{opaque#0}), [])` which is not `Send`
| ------------- has type `Opaque(DefId(0:24 ~ coroutine_print_verbose_1[75fb]::make_non_send_coroutine::{opaque#0}), [])` which is not `Send`
LL | yield;
| ^^^^^ yield occurs here, with `_non_send_gen` maybe used later
note: required by a bound in `require_send`
Expand All @@ -33,12 +33,12 @@ note: required because it's used within this coroutine
|
LL | #[coroutine] || {
| ^^
note: required because it appears within the type `Opaque(DefId(0:35 ~ coroutine_print_verbose_1[75fb]::make_gen2::{opaque#0}), [Arc<RefCell<i32>>])`
note: required because it appears within the type `Opaque(DefId(0:29 ~ coroutine_print_verbose_1[75fb]::make_gen2::{opaque#0}), [Arc<RefCell<i32>>])`
--> $DIR/coroutine-print-verbose-1.rs:41:30
|
LL | pub fn make_gen2<T>(t: T) -> impl Coroutine<Return = T> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
note: required because it appears within the type `Opaque(DefId(0:36 ~ coroutine_print_verbose_1[75fb]::make_non_send_coroutine2::{opaque#0}), [])`
note: required because it appears within the type `Opaque(DefId(0:32 ~ coroutine_print_verbose_1[75fb]::make_non_send_coroutine2::{opaque#0}), [])`
--> $DIR/coroutine-print-verbose-1.rs:47:34
|
LL | fn make_non_send_coroutine2() -> impl Coroutine<Return = Arc<RefCell<i32>>> {
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/nll/ty-outlives/impl-trait-captures.stderr
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
error[E0700]: hidden type for `Opaque(DefId(0:13 ~ impl_trait_captures[aeb9]::foo::{opaque#0}), ['a/#0, T, 'a/#0])` captures lifetime that does not appear in bounds
error[E0700]: hidden type for `Opaque(DefId(0:11 ~ impl_trait_captures[aeb9]::foo::{opaque#0}), ['a/#0, T, 'a/#0])` captures lifetime that does not appear in bounds
--> $DIR/impl-trait-captures.rs:11:5
|
LL | fn foo<'a, T>(x: &T) -> impl Foo<'a> {
| -- ------------ opaque type defined here
| |
| hidden type `&ReLateParam(DefId(0:8 ~ impl_trait_captures[aeb9]::foo), BrNamed(DefId(0:12 ~ impl_trait_captures[aeb9]::foo::'_), '_)) T` captures the anonymous lifetime defined here
| hidden type `&ReLateParam(DefId(0:8 ~ impl_trait_captures[aeb9]::foo), BrNamed(DefId(0:13 ~ impl_trait_captures[aeb9]::foo::'_), '_)) T` captures the anonymous lifetime defined here
LL | x
| ^
|
help: add a `use<...>` bound to explicitly capture `ReLateParam(DefId(0:8 ~ impl_trait_captures[aeb9]::foo), BrNamed(DefId(0:12 ~ impl_trait_captures[aeb9]::foo::'_), '_))`
help: add a `use<...>` bound to explicitly capture `ReLateParam(DefId(0:8 ~ impl_trait_captures[aeb9]::foo), BrNamed(DefId(0:13 ~ impl_trait_captures[aeb9]::foo::'_), '_))`
|
LL | fn foo<'a, T>(x: &T) -> impl Foo<'a> + use<'a, ReLateParam(DefId(0:8 ~ impl_trait_captures[aeb9]::foo), BrNamed(DefId(0:12 ~ impl_trait_captures[aeb9]::foo::'_), '_)), T> {
LL | fn foo<'a, T>(x: &T) -> impl Foo<'a> + use<'a, ReLateParam(DefId(0:8 ~ impl_trait_captures[aeb9]::foo), BrNamed(DefId(0:13 ~ impl_trait_captures[aeb9]::foo::'_), '_)), T> {
| +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

error: aborting due to 1 previous error
Expand Down

0 comments on commit 07d5c25

Please sign in to comment.