From ad7c2b06609c0da21ebdab6e45135cf07290f585 Mon Sep 17 00:00:00 2001 From: Oliver Killane Date: Sun, 5 May 2024 13:54:33 +0100 Subject: [PATCH 01/25] Updated error code explanation --- .../src/error_codes/E0582.md | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/compiler/rustc_error_codes/src/error_codes/E0582.md b/compiler/rustc_error_codes/src/error_codes/E0582.md index e50cc60ea3302..ea32e4f9f33f1 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0582.md +++ b/compiler/rustc_error_codes/src/error_codes/E0582.md @@ -27,6 +27,40 @@ fn bar(t: F, u: G) fn main() { } ``` +This error also includes the use of associated types with lifetime parameters. +```compile_fail,E0582 +trait Foo { + type Assoc<'a>; +} + +struct Bar +where + X: Foo, + F: for<'a> Fn(X::Assoc<'a>) -> &'a i32 +{ + x: X, + f: F +} +``` +This is as `Foo::Assoc<'a>` could be implemented by a type that does not use +the `'a` parameter, so there is no guarentee that `X::Assoc<'a>` actually uses +`'a`. + +To fix this we can pass a dummy parameter: +``` +# trait Foo { +# type Assoc<'a>; +# } +struct Bar +where + X: Foo, + F: for<'a> Fn(X::Assoc<'a>, /* dummy */ &'a ()) -> &'a i32 +{ + x: X, + f: F +} +``` + Note: The examples above used to be (erroneously) accepted by the compiler, but this was since corrected. See [issue #33685] for more details. From f3dcf65985dd956595a3f14dcb2a3052daceeb73 Mon Sep 17 00:00:00 2001 From: Oliver Killane Date: Sun, 5 May 2024 14:55:16 +0100 Subject: [PATCH 02/25] fix whitespace --- compiler/rustc_error_codes/src/error_codes/E0582.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_error_codes/src/error_codes/E0582.md b/compiler/rustc_error_codes/src/error_codes/E0582.md index ea32e4f9f33f1..31927cd5fe34f 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0582.md +++ b/compiler/rustc_error_codes/src/error_codes/E0582.md @@ -42,8 +42,8 @@ where f: F } ``` -This is as `Foo::Assoc<'a>` could be implemented by a type that does not use -the `'a` parameter, so there is no guarentee that `X::Assoc<'a>` actually uses +This is as `Foo::Assoc<'a>` could be implemented by a type that does not use +the `'a` parameter, so there is no guarentee that `X::Assoc<'a>` actually uses `'a`. To fix this we can pass a dummy parameter: From a5d0988a88a7557d4ae507fecd4df3d9ed8ad839 Mon Sep 17 00:00:00 2001 From: Oliver Killane <44177991+OliverKillane@users.noreply.github.com> Date: Wed, 15 May 2024 00:07:21 +0100 Subject: [PATCH 03/25] Update compiler/rustc_error_codes/src/error_codes/E0582.md Co-authored-by: Felix S Klock II --- compiler/rustc_error_codes/src/error_codes/E0582.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_error_codes/src/error_codes/E0582.md b/compiler/rustc_error_codes/src/error_codes/E0582.md index 31927cd5fe34f..26b65b12bc912 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0582.md +++ b/compiler/rustc_error_codes/src/error_codes/E0582.md @@ -42,7 +42,7 @@ where f: F } ``` -This is as `Foo::Assoc<'a>` could be implemented by a type that does not use +The latter scenario encounters this error because `Foo::Assoc<'a>` could be implemented by a type that does not use the `'a` parameter, so there is no guarentee that `X::Assoc<'a>` actually uses `'a`. From 012288b922e5db0ca6a8bbb2d1704c86c0fd79ff Mon Sep 17 00:00:00 2001 From: Oliver Killane Date: Wed, 15 May 2024 01:15:36 +0100 Subject: [PATCH 04/25] tidy fix from suggestion --- compiler/rustc_error_codes/src/error_codes/E0582.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_error_codes/src/error_codes/E0582.md b/compiler/rustc_error_codes/src/error_codes/E0582.md index 26b65b12bc912..b2cdb509c95c0 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0582.md +++ b/compiler/rustc_error_codes/src/error_codes/E0582.md @@ -42,9 +42,9 @@ where f: F } ``` -The latter scenario encounters this error because `Foo::Assoc<'a>` could be implemented by a type that does not use -the `'a` parameter, so there is no guarentee that `X::Assoc<'a>` actually uses -`'a`. +The latter scenario encounters this error because `Foo::Assoc<'a>` could be +implemented by a type that does not use the `'a` parameter, so there is no +guarentee that `X::Assoc<'a>` actually uses `'a`. To fix this we can pass a dummy parameter: ``` From d6e4fe569c84c6a9d20690f139ad15a3d3da9550 Mon Sep 17 00:00:00 2001 From: Michael Baikov Date: Wed, 22 May 2024 09:26:02 -0400 Subject: [PATCH 05/25] A custom error message for lending iterators --- compiler/rustc_resolve/messages.ftl | 4 ++ compiler/rustc_resolve/src/errors.rs | 9 +++++ compiler/rustc_resolve/src/late.rs | 40 ++++++++++++++++--- tests/ui/lifetimes/no_lending_iterators.rs | 12 ++++++ .../ui/lifetimes/no_lending_iterators.stderr | 14 +++++++ 5 files changed, 74 insertions(+), 5 deletions(-) create mode 100644 tests/ui/lifetimes/no_lending_iterators.rs create mode 100644 tests/ui/lifetimes/no_lending_iterators.stderr diff --git a/compiler/rustc_resolve/messages.ftl b/compiler/rustc_resolve/messages.ftl index f824e4faf5d06..90339b2498b4b 100644 --- a/compiler/rustc_resolve/messages.ftl +++ b/compiler/rustc_resolve/messages.ftl @@ -234,6 +234,10 @@ resolve_items_in_traits_are_not_importable = resolve_label_with_similar_name_reachable = a label with a similar name is reachable +resolve_lending_iterator_report_error = + associated type `Iterator::Item` is declared without lifetime parameters, so using a borrowed type for them requires that lifetime to come from the implemented type. + .note = you can't create an `Iterator` that borrows each `Item` from itself, but you can instead create a new type that borrows your existing type and implement `Iterator` for that new type. + resolve_lifetime_param_in_enum_discriminant = lifetime parameters may not be used in enum discriminant values diff --git a/compiler/rustc_resolve/src/errors.rs b/compiler/rustc_resolve/src/errors.rs index edfeacec7e3b7..e9989b803e8a3 100644 --- a/compiler/rustc_resolve/src/errors.rs +++ b/compiler/rustc_resolve/src/errors.rs @@ -882,6 +882,15 @@ pub(crate) struct ElidedAnonymousLivetimeReportError { pub(crate) suggestion: Option, } +#[derive(Diagnostic)] +#[diag(resolve_lending_iterator_report_error)] +pub(crate) struct LendingIteratorReportError { + #[primary_span] + pub(crate) lifetime: Span, + #[note] + pub(crate) ty: Span, +} + #[derive(Subdiagnostic)] #[multipart_suggestion( resolve_elided_anonymous_lifetime_report_error_suggestion, diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 63d0d6c260dbe..08196619befad 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -9,7 +9,7 @@ use crate::{errors, path_names_to_string, rustdoc, BindingError, Finalize, LexicalScopeBinding}; use crate::{BindingKey, Used}; use crate::{Module, ModuleOrUniformRoot, NameBinding, ParentScope, PathResult}; -use crate::{ResolutionError, Resolver, Segment, UseError}; +use crate::{ResolutionError, Resolver, Segment, TyCtxt, UseError}; use rustc_ast::ptr::P; use rustc_ast::visit::{visit_opt, walk_list, AssocCtxt, BoundKind, FnCtxt, FnKind, Visitor}; @@ -1703,10 +1703,28 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { break; } } - self.r.dcx().emit_err(errors::ElidedAnonymousLivetimeReportError { - span: lifetime.ident.span, - suggestion, - }); + + // Is it caused by user trying to implement a lending iterator? + if !self.in_func_body + && let Some((module, _)) = &self.current_trait_ref + && let Some(ty) = &self.diag_metadata.current_self_type + && let crate::ModuleKind::Def(DefKind::Trait, trait_id, _) = module.kind + && def_id_matches_path( + self.r.tcx, + trait_id, + &["core", "iter", "traits", "iterator", "Iterator"], + ) + { + self.r.dcx().emit_err(errors::LendingIteratorReportError { + lifetime: lifetime.ident.span, + ty: ty.span(), + }); + } else { + self.r.dcx().emit_err(errors::ElidedAnonymousLivetimeReportError { + span: lifetime.ident.span, + suggestion, + }); + } } else { self.r.dcx().emit_err(errors::ExplicitAnonymousLivetimeReportError { span: lifetime.ident.span, @@ -4824,3 +4842,15 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } } } + +/// Check if definition matches a path +fn def_id_matches_path(tcx: TyCtxt<'_>, mut def_id: DefId, expected_path: &[&str]) -> bool { + let mut path = expected_path.iter().rev(); + while let (Some(parent), Some(next_step)) = (tcx.opt_parent(def_id), path.next()) { + if !tcx.opt_item_name(def_id).map_or(false, |n| n.as_str() == *next_step) { + return false; + } + def_id = parent; + } + return true; +} diff --git a/tests/ui/lifetimes/no_lending_iterators.rs b/tests/ui/lifetimes/no_lending_iterators.rs new file mode 100644 index 0000000000000..1f83406721822 --- /dev/null +++ b/tests/ui/lifetimes/no_lending_iterators.rs @@ -0,0 +1,12 @@ +struct Data(String); + +impl Iterator for Data { + type Item = &str; + //~^ ERROR 4:17: 4:18: associated type `Iterator::Item` is declared without lifetime parameters, so using a borrowed type for them requires that lifetime to come from the implemented type. + + fn next(&mut self) -> Option { + Some(&self.0) + } +} + +fn main() {} diff --git a/tests/ui/lifetimes/no_lending_iterators.stderr b/tests/ui/lifetimes/no_lending_iterators.stderr new file mode 100644 index 0000000000000..d052bfac38405 --- /dev/null +++ b/tests/ui/lifetimes/no_lending_iterators.stderr @@ -0,0 +1,14 @@ +error: associated type `Iterator::Item` is declared without lifetime parameters, so using a borrowed type for them requires that lifetime to come from the implemented type. + --> $DIR/no_lending_iterators.rs:4:17 + | +LL | type Item = &str; + | ^ + | +note: you can't create an `Iterator` that borrows each `Item` from itself, but you can instead create a new type that borrows your existing type and implement `Iterator` for that new type. + --> $DIR/no_lending_iterators.rs:3:19 + | +LL | impl Iterator for Data { + | ^^^^ + +error: aborting due to 1 previous error + From b70fb4159be7ea3e549a242cf7a6e4ebcb41f168 Mon Sep 17 00:00:00 2001 From: Michael Baikov Date: Fri, 24 May 2024 11:20:33 -0400 Subject: [PATCH 06/25] And more general error --- compiler/rustc_resolve/messages.ftl | 6 +++- compiler/rustc_resolve/src/errors.rs | 8 ++++++ compiler/rustc_resolve/src/late.rs | 28 +++++++++++++------ .../assoc-type.rs | 2 +- .../assoc-type.stderr | 4 +-- tests/ui/lifetimes/no_lending_iterators.rs | 25 ++++++++++++++++- .../ui/lifetimes/no_lending_iterators.stderr | 20 +++++++++++-- 7 files changed, 78 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_resolve/messages.ftl b/compiler/rustc_resolve/messages.ftl index 90339b2498b4b..358f25e233434 100644 --- a/compiler/rustc_resolve/messages.ftl +++ b/compiler/rustc_resolve/messages.ftl @@ -11,6 +11,10 @@ resolve_added_macro_use = resolve_ancestor_only = visibilities can only be restricted to ancestor modules +resolve_anonymous_livetime_non_gat_report_error = + in the trait associated type is declared without lifetime parameters, so using a borrowed type for them requires that lifetime to come from the implemented type + .label = this lifetime must come from the implemented type + resolve_arguments_macro_use_not_allowed = arguments to `macro_use` are not allowed here resolve_associated_const_with_similar_name_exists = @@ -235,7 +239,7 @@ resolve_label_with_similar_name_reachable = a label with a similar name is reachable resolve_lending_iterator_report_error = - associated type `Iterator::Item` is declared without lifetime parameters, so using a borrowed type for them requires that lifetime to come from the implemented type. + associated type `Iterator::Item` is declared without lifetime parameters, so using a borrowed type for them requires that lifetime to come from the implemented type .note = you can't create an `Iterator` that borrows each `Item` from itself, but you can instead create a new type that borrows your existing type and implement `Iterator` for that new type. resolve_lifetime_param_in_enum_discriminant = diff --git a/compiler/rustc_resolve/src/errors.rs b/compiler/rustc_resolve/src/errors.rs index e9989b803e8a3..0620f3d709eb2 100644 --- a/compiler/rustc_resolve/src/errors.rs +++ b/compiler/rustc_resolve/src/errors.rs @@ -891,6 +891,14 @@ pub(crate) struct LendingIteratorReportError { pub(crate) ty: Span, } +#[derive(Diagnostic)] +#[diag(resolve_anonymous_livetime_non_gat_report_error)] +pub(crate) struct AnonymousLivetimeNonGatReportError { + #[primary_span] + #[label] + pub(crate) lifetime: Span, +} + #[derive(Subdiagnostic)] #[multipart_suggestion( resolve_elided_anonymous_lifetime_report_error_suggestion, diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 08196619befad..8f80b9ad5fdca 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -629,6 +629,9 @@ struct DiagMetadata<'ast> { in_assignment: Option<&'ast Expr>, is_assign_rhs: bool, + /// If we are setting an associated type in trait impl, is it a non-GAT type? + in_non_gat_assoc_type: Option, + /// Used to detect possible `.` -> `..` typo when calling methods. in_range: Option<(&'ast Expr, &'ast Expr)>, @@ -1704,21 +1707,28 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { } } - // Is it caused by user trying to implement a lending iterator? + // are we trying to use an anonymous lifetime + // on a non GAT associated trait type? if !self.in_func_body && let Some((module, _)) = &self.current_trait_ref && let Some(ty) = &self.diag_metadata.current_self_type + && Some(true) == self.diag_metadata.in_non_gat_assoc_type && let crate::ModuleKind::Def(DefKind::Trait, trait_id, _) = module.kind - && def_id_matches_path( + { + if def_id_matches_path( self.r.tcx, trait_id, &["core", "iter", "traits", "iterator", "Iterator"], - ) - { - self.r.dcx().emit_err(errors::LendingIteratorReportError { - lifetime: lifetime.ident.span, - ty: ty.span(), - }); + ) { + self.r.dcx().emit_err(errors::LendingIteratorReportError { + lifetime: lifetime.ident.span, + ty: ty.span(), + }); + } else { + self.r.dcx().emit_err(errors::AnonymousLivetimeNonGatReportError { + lifetime: lifetime.ident.span, + }); + } } else { self.r.dcx().emit_err(errors::ElidedAnonymousLivetimeReportError { span: lifetime.ident.span, @@ -3076,6 +3086,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { ); } AssocItemKind::Type(box TyAlias { generics, .. }) => { + self.diag_metadata.in_non_gat_assoc_type = Some(generics.params.is_empty()); debug!("resolve_implementation AssocItemKind::Type"); // We also need a new scope for the impl item type parameters. self.with_generic_param_rib( @@ -3104,6 +3115,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { }); }, ); + self.diag_metadata.in_non_gat_assoc_type = None; } AssocItemKind::Delegation(box delegation) => { debug!("resolve_implementation AssocItemKind::Delegation"); diff --git a/tests/ui/impl-header-lifetime-elision/assoc-type.rs b/tests/ui/impl-header-lifetime-elision/assoc-type.rs index b0089a37aa05f..db3c416540fcd 100644 --- a/tests/ui/impl-header-lifetime-elision/assoc-type.rs +++ b/tests/ui/impl-header-lifetime-elision/assoc-type.rs @@ -9,7 +9,7 @@ trait MyTrait { impl MyTrait for &i32 { type Output = &i32; - //~^ ERROR `&` without an explicit lifetime name cannot be used here + //~^ ERROR 11:19: 11:20: in the trait associated type is declared without lifetime parameters, so using a borrowed type for them requires that lifetime to come from the implemented type } impl MyTrait for &u32 { diff --git a/tests/ui/impl-header-lifetime-elision/assoc-type.stderr b/tests/ui/impl-header-lifetime-elision/assoc-type.stderr index c4f27e0b80e41..e650eeca48ad9 100644 --- a/tests/ui/impl-header-lifetime-elision/assoc-type.stderr +++ b/tests/ui/impl-header-lifetime-elision/assoc-type.stderr @@ -1,8 +1,8 @@ -error[E0637]: `&` without an explicit lifetime name cannot be used here +error: in the trait associated type is declared without lifetime parameters, so using a borrowed type for them requires that lifetime to come from the implemented type --> $DIR/assoc-type.rs:11:19 | LL | type Output = &i32; - | ^ explicit lifetime name needed here + | ^ this lifetime must come from the implemented type error[E0637]: `'_` cannot be used here --> $DIR/assoc-type.rs:16:20 diff --git a/tests/ui/lifetimes/no_lending_iterators.rs b/tests/ui/lifetimes/no_lending_iterators.rs index 1f83406721822..21395475fb3dc 100644 --- a/tests/ui/lifetimes/no_lending_iterators.rs +++ b/tests/ui/lifetimes/no_lending_iterators.rs @@ -2,11 +2,34 @@ struct Data(String); impl Iterator for Data { type Item = &str; - //~^ ERROR 4:17: 4:18: associated type `Iterator::Item` is declared without lifetime parameters, so using a borrowed type for them requires that lifetime to come from the implemented type. + //~^ ERROR 4:17: 4:18: associated type `Iterator::Item` is declared without lifetime parameters, so using a borrowed type for them requires that lifetime to come from the implemented type fn next(&mut self) -> Option { Some(&self.0) } } +trait Bar { + type Item; + fn poke(&mut self, item: Self::Item); +} + +impl Bar for usize { + type Item = &usize; + //~^ ERROR 18:17: 18:18: in the trait associated type is declared without lifetime parameters, so using a borrowed type for them requires that lifetime to come from the implemented type + + fn poke(&mut self, item: Self::Item) { + self += *item; + } +} + +impl Bar for isize { + type Item<'a> = &'a isize; + //~^ ERROR 27:14: 27:18: lifetime parameters or bounds on type `Item` do not match the trait declaration [E0195] + + fn poke(&mut self, item: Self::Item) { + self += *item; + } +} + fn main() {} diff --git a/tests/ui/lifetimes/no_lending_iterators.stderr b/tests/ui/lifetimes/no_lending_iterators.stderr index d052bfac38405..c3784770d7926 100644 --- a/tests/ui/lifetimes/no_lending_iterators.stderr +++ b/tests/ui/lifetimes/no_lending_iterators.stderr @@ -1,4 +1,4 @@ -error: associated type `Iterator::Item` is declared without lifetime parameters, so using a borrowed type for them requires that lifetime to come from the implemented type. +error: associated type `Iterator::Item` is declared without lifetime parameters, so using a borrowed type for them requires that lifetime to come from the implemented type --> $DIR/no_lending_iterators.rs:4:17 | LL | type Item = &str; @@ -10,5 +10,21 @@ note: you can't create an `Iterator` that borrows each `Item` from itself, but y LL | impl Iterator for Data { | ^^^^ -error: aborting due to 1 previous error +error: in the trait associated type is declared without lifetime parameters, so using a borrowed type for them requires that lifetime to come from the implemented type + --> $DIR/no_lending_iterators.rs:18:17 + | +LL | type Item = &usize; + | ^ this lifetime must come from the implemented type + +error[E0195]: lifetime parameters or bounds on type `Item` do not match the trait declaration + --> $DIR/no_lending_iterators.rs:27:14 + | +LL | type Item; + | - lifetimes in impl do not match this type in trait +... +LL | type Item<'a> = &'a isize; + | ^^^^ lifetimes do not match type in trait + +error: aborting due to 3 previous errors +For more information about this error, try `rustc --explain E0195`. From 9f8b1caf2957f281f88577ff62b20811d5fcb419 Mon Sep 17 00:00:00 2001 From: Alona Enraght-Moony Date: Fri, 24 May 2024 17:17:25 +0000 Subject: [PATCH 07/25] Add intra-doc-links to rustc_middle crate-level docs. --- compiler/rustc_middle/src/lib.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_middle/src/lib.rs b/compiler/rustc_middle/src/lib.rs index e52a5863fd03d..ea7b8baf3f2c5 100644 --- a/compiler/rustc_middle/src/lib.rs +++ b/compiler/rustc_middle/src/lib.rs @@ -4,15 +4,16 @@ //! has their own README with further details). //! //! - **HIR.** The "high-level (H) intermediate representation (IR)" is -//! defined in the `hir` module. +//! defined in the [`hir`] module. //! - **MIR.** The "mid-level (M) intermediate representation (IR)" is -//! defined in the `mir` module. This module contains only the +//! defined in the [`mir`] module. This module contains only the //! *definition* of the MIR; the passes that transform and operate //! on MIR are found in `rustc_const_eval` crate. //! - **Types.** The internal representation of types used in rustc is -//! defined in the `ty` module. This includes the **type context** -//! (or `tcx`), which is the central context during most of -//! compilation, containing the interners and other things. +//! defined in the [`ty`] module. This includes the +//! [**type context**][ty::TyCtxt] (or `tcx`), which is the central +//! context during most of compilation, containing the interners and +//! other things. //! //! For more information about how rustc works, see the [rustc dev guide]. //! From e485b193d07a77495dcdcb3f52f5976c9c9ca1f4 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 30 May 2024 19:45:27 -0400 Subject: [PATCH 08/25] Don't drop Upcast candidate in intercrate mode --- .../src/traits/select/candidate_assembly.rs | 6 ++++++ .../ui/specialization/dont-drop-upcast-candidate.rs | 13 +++++++++++++ .../dont-drop-upcast-candidate.stderr | 11 +++++++++++ 3 files changed, 30 insertions(+) create mode 100644 tests/ui/specialization/dont-drop-upcast-candidate.rs create mode 100644 tests/ui/specialization/dont-drop-upcast-candidate.stderr diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs index fd7c47ad6fb3d..f0ecbd833410d 100644 --- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs +++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs @@ -921,6 +921,12 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { param_env: ty::ParamEnv<'tcx>, cause: &ObligationCause<'tcx>, ) -> Option> { + // Don't drop any candidates in intercrate mode, as it's incomplete. + // (Not that it matters, since `Unsize` is not a stable trait.) + if self.infcx.intercrate { + return None; + } + let tcx = self.tcx(); if tcx.features().trait_upcasting { return None; diff --git a/tests/ui/specialization/dont-drop-upcast-candidate.rs b/tests/ui/specialization/dont-drop-upcast-candidate.rs new file mode 100644 index 0000000000000..98d8cad7c1fbd --- /dev/null +++ b/tests/ui/specialization/dont-drop-upcast-candidate.rs @@ -0,0 +1,13 @@ +#![feature(unsize)] + +use std::marker::Unsize; +use std::ops::Deref; + +trait Foo: Bar {} +trait Bar {} + +impl Bar for T where dyn Foo: Unsize {} +impl Bar for () {} +//~^ ERROR conflicting implementations of trait `Bar` for type `()` + +fn main() {} diff --git a/tests/ui/specialization/dont-drop-upcast-candidate.stderr b/tests/ui/specialization/dont-drop-upcast-candidate.stderr new file mode 100644 index 0000000000000..dc0c54f9aa82f --- /dev/null +++ b/tests/ui/specialization/dont-drop-upcast-candidate.stderr @@ -0,0 +1,11 @@ +error[E0119]: conflicting implementations of trait `Bar` for type `()` + --> $DIR/dont-drop-upcast-candidate.rs:10:1 + | +LL | impl Bar for T where dyn Foo: Unsize {} + | ------------------------------------------------ first implementation here +LL | impl Bar for () {} + | ^^^^^^^^^^^^^^^ conflicting implementation for `()` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0119`. From 5e802f07bae5a078c241ed1c2a122e756cbcad30 Mon Sep 17 00:00:00 2001 From: Hai-Hsin Date: Sat, 1 Jun 2024 23:13:33 +0800 Subject: [PATCH 09/25] rustc_codegen_ssa: fix get_rpath_relative_to_output panic when lib only contains file name --- compiler/rustc_codegen_ssa/src/back/rpath.rs | 5 +++++ .../rustc_codegen_ssa/src/back/rpath/tests.rs | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/compiler/rustc_codegen_ssa/src/back/rpath.rs b/compiler/rustc_codegen_ssa/src/back/rpath.rs index 3114f1c38ae71..f499bbcf85339 100644 --- a/compiler/rustc_codegen_ssa/src/back/rpath.rs +++ b/compiler/rustc_codegen_ssa/src/back/rpath.rs @@ -85,6 +85,11 @@ fn get_rpath_relative_to_output(config: &RPathConfig<'_>, lib: &Path) -> OsStrin // Strip filenames let lib = lib.parent().unwrap(); let output = config.out_filename.parent().unwrap(); + + // If output or lib is empty, just assume it locates in current path + let lib = if lib == Path::new("") { Path::new(".") } else { lib }; + let output = if output == Path::new("") { Path::new(".") } else { output }; + let lib = try_canonicalize(lib).unwrap(); let output = try_canonicalize(output).unwrap(); let relative = path_relative_from(&lib, &output) diff --git a/compiler/rustc_codegen_ssa/src/back/rpath/tests.rs b/compiler/rustc_codegen_ssa/src/back/rpath/tests.rs index ac2e54072c416..0de90a1036ec5 100644 --- a/compiler/rustc_codegen_ssa/src/back/rpath/tests.rs +++ b/compiler/rustc_codegen_ssa/src/back/rpath/tests.rs @@ -57,6 +57,22 @@ fn test_rpath_relative() { } } +#[test] +fn test_rpath_relative_issue_119571() { + let config = &mut RPathConfig { + libs: &[], + out_filename: PathBuf::from("rustc"), + has_rpath: true, + is_like_osx: false, + linker_is_gnu: true, + }; + // Should not panic when out_filename only contains filename. + // Issue 119571 + let _ = get_rpath_relative_to_output(config, Path::new("lib/libstd.so")); + // Should not panic when lib only contains filename. + let _ = get_rpath_relative_to_output(config, Path::new("libstd.so")); +} + #[test] fn test_xlinker() { let args = rpaths_to_flags(vec!["a/normal/path".into(), "a,comma,path".into()]); From 92f85f12a82ac23b794dcdd58a76b0ae7519951c Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Mon, 3 Jun 2024 06:58:30 +0300 Subject: [PATCH 10/25] wipe bootstrap build before switching to bumped rustc Technically, wiping bootstrap builds can increase the build time. But in practice, trying to manually resolve post-bump issues and even accidentally removing the entire build directory will result in a much greater loss of time. After all, the bootstrap build process is not a particularly lengthy operation. Signed-off-by: onur-ozkan --- src/bootstrap/bootstrap.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py index e60e8f0aa1f70..9861121aac0a3 100644 --- a/src/bootstrap/bootstrap.py +++ b/src/bootstrap/bootstrap.py @@ -599,6 +599,12 @@ def download_toolchain(self): print('Choosing a pool size of', pool_size, 'for the unpacking of the tarballs') p = Pool(pool_size) try: + # FIXME: A cheap workaround for https://github.com/rust-lang/rust/issues/125578, + # remove this once the issue is closed. + bootstrap_out = self.bootstrap_out() + if os.path.exists(bootstrap_out): + shutil.rmtree(bootstrap_out) + p.map(unpack_component, tarballs_download_info) finally: p.close() @@ -864,6 +870,16 @@ def get_string(line): return line[start + 1:end] return None + def bootstrap_out(self): + """Return the path of the bootstrap build artifacts + + >>> rb = RustBuild() + >>> rb.build_dir = "build" + >>> rb.bootstrap_binary() == os.path.join("build", "bootstrap") + True + """ + return os.path.join(self.build_dir, "bootstrap") + def bootstrap_binary(self): """Return the path of the bootstrap binary @@ -873,7 +889,7 @@ def bootstrap_binary(self): ... "debug", "bootstrap") True """ - return os.path.join(self.build_dir, "bootstrap", "debug", "bootstrap") + return os.path.join(self.bootstrap_out(), "debug", "bootstrap") def build_bootstrap(self): """Build bootstrap""" From e609c9b2541fe337872b26af849223ec37aed50d Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 3 Jun 2024 16:57:17 +1000 Subject: [PATCH 11/25] Add unit tests for `Span::trim_start` --- compiler/rustc_span/src/tests.rs | 38 ++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/compiler/rustc_span/src/tests.rs b/compiler/rustc_span/src/tests.rs index cb88fa89058dc..5c9dcc2a80fcd 100644 --- a/compiler/rustc_span/src/tests.rs +++ b/compiler/rustc_span/src/tests.rs @@ -42,3 +42,41 @@ fn test_normalize_newlines() { check("\r\r\n", "\r\n", &[2]); check("hello\rworld", "hello\rworld", &[]); } + +#[test] +fn test_trim() { + let span = |lo: usize, hi: usize| { + Span::new(BytePos::from_usize(lo), BytePos::from_usize(hi), SyntaxContext::root(), None) + }; + + // Various positions, named for their relation to `start` and `end`. + let well_before = 1; + let before = 3; + let start = 5; + let mid = 7; + let end = 9; + let after = 11; + let well_after = 13; + + // The resulting span's context should be that of `self`, not `other`. + let other = span(start, end).with_ctxt(SyntaxContext::from_u32(999)); + + // Test cases for `trim_start`. + + assert_eq!(span(after, well_after).trim_start(other), Some(span(after, well_after))); + assert_eq!(span(end, well_after).trim_start(other), Some(span(end, well_after))); + assert_eq!(span(mid, well_after).trim_start(other), Some(span(end, well_after))); + assert_eq!(span(start, well_after).trim_start(other), Some(span(end, well_after))); + assert_eq!(span(before, well_after).trim_start(other), Some(span(end, well_after))); + + assert_eq!(span(mid, end).trim_start(other), None); + assert_eq!(span(start, end).trim_start(other), None); + assert_eq!(span(before, end).trim_start(other), None); + + assert_eq!(span(start, mid).trim_start(other), None); + assert_eq!(span(before, mid).trim_start(other), None); + + assert_eq!(span(before, start).trim_start(other), None); + + assert_eq!(span(well_before, before).trim_start(other), None); +} From df96cba432d9e869765385d6474b505494533435 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 3 Jun 2024 17:00:10 +1000 Subject: [PATCH 12/25] Add `Span::trim_end` This is the counterpart of `Span::trim_start`. --- compiler/rustc_span/src/lib.rs | 7 +++++++ compiler/rustc_span/src/tests.rs | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index b2ca01fe3b94c..82179a4a05867 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -682,6 +682,13 @@ impl Span { if span.hi > other.hi { Some(span.with_lo(cmp::max(span.lo, other.hi))) } else { None } } + /// Returns `Some(span)`, where the end is trimmed by the start of `other`. + pub fn trim_end(self, other: Span) -> Option { + let span = self.data(); + let other = other.data(); + if span.lo < other.lo { Some(span.with_hi(cmp::min(span.hi, other.lo))) } else { None } + } + /// Returns the source span -- this is either the supplied span, or the span for /// the macro callsite that expanded to it. pub fn source_callsite(self) -> Span { diff --git a/compiler/rustc_span/src/tests.rs b/compiler/rustc_span/src/tests.rs index 5c9dcc2a80fcd..48fa786fb1c80 100644 --- a/compiler/rustc_span/src/tests.rs +++ b/compiler/rustc_span/src/tests.rs @@ -61,6 +61,25 @@ fn test_trim() { // The resulting span's context should be that of `self`, not `other`. let other = span(start, end).with_ctxt(SyntaxContext::from_u32(999)); + // Test cases for `trim_end`. + + assert_eq!(span(well_before, before).trim_end(other), Some(span(well_before, before))); + assert_eq!(span(well_before, start).trim_end(other), Some(span(well_before, start))); + assert_eq!(span(well_before, mid).trim_end(other), Some(span(well_before, start))); + assert_eq!(span(well_before, end).trim_end(other), Some(span(well_before, start))); + assert_eq!(span(well_before, after).trim_end(other), Some(span(well_before, start))); + + assert_eq!(span(start, mid).trim_end(other), None); + assert_eq!(span(start, end).trim_end(other), None); + assert_eq!(span(start, after).trim_end(other), None); + + assert_eq!(span(mid, end).trim_end(other), None); + assert_eq!(span(mid, after).trim_end(other), None); + + assert_eq!(span(end, after).trim_end(other), None); + + assert_eq!(span(after, well_after).trim_end(other), None); + // Test cases for `trim_start`. assert_eq!(span(after, well_after).trim_start(other), Some(span(after, well_after))); From 9c931c01f70d12d7296fead28b1c3dc417bb9372 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 3 Jun 2024 15:27:30 +1000 Subject: [PATCH 13/25] coverage: Return a nested vector from initial span extraction This will allow the span extractor to produce multiple separate buckets, instead of just one flat list of spans. --- .../rustc_mir_transform/src/coverage/spans.rs | 15 +++++++++------ .../src/coverage/spans/from_mir.rs | 4 ++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index f2f76ac70c20d..b0bd514f69039 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -20,13 +20,16 @@ pub(super) fn extract_refined_covspans( basic_coverage_blocks: &CoverageGraph, code_mappings: &mut impl Extend, ) { - let sorted_spans = + let sorted_span_buckets = from_mir::mir_to_initial_sorted_coverage_spans(mir_body, hir_info, basic_coverage_blocks); - let coverage_spans = SpansRefiner::refine_sorted_spans(sorted_spans); - code_mappings.extend(coverage_spans.into_iter().map(|RefinedCovspan { bcb, span, .. }| { - // Each span produced by the generator represents an ordinary code region. - mappings::CodeMapping { span, bcb } - })); + for bucket in sorted_span_buckets { + let refined_spans = SpansRefiner::refine_sorted_spans(bucket); + code_mappings.extend(refined_spans.into_iter().map(|covspan| { + let RefinedCovspan { span, bcb, is_hole: _ } = covspan; + // Each span produced by the refiner represents an ordinary code region. + mappings::CodeMapping { span, bcb } + })); + } } #[derive(Debug)] diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index d1727a94a35e3..8e4466a99718e 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -23,7 +23,7 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( mir_body: &mir::Body<'_>, hir_info: &ExtractedHirInfo, basic_coverage_blocks: &CoverageGraph, -) -> Vec { +) -> Vec> { let &ExtractedHirInfo { body_span, .. } = hir_info; let mut initial_spans = vec![]; @@ -67,7 +67,7 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( // requires a lot more complexity in the span refiner, for little benefit.) initial_spans.dedup_by(|b, a| a.span.source_equal(b.span)); - initial_spans + vec![initial_spans] } /// Macros that expand into branches (e.g. `assert!`, `trace!`) tend to generate From 464dee24ffb1295d8dfb36455452b8658ae37878 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 3 Jun 2024 15:56:09 +1000 Subject: [PATCH 14/25] coverage: Build up initial spans by appending to a vector This is less elegant than returning an iterator, but more flexible. --- .../src/coverage/spans/from_mir.rs | 39 ++++++++++--------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 8e4466a99718e..3fa37198732ea 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -1,4 +1,3 @@ -use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxHashSet; use rustc_middle::bug; use rustc_middle::mir::coverage::CoverageKind; @@ -29,7 +28,7 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( let mut initial_spans = vec![]; for (bcb, bcb_data) in basic_coverage_blocks.iter_enumerated() { - initial_spans.extend(bcb_to_initial_coverage_spans(mir_body, body_span, bcb, bcb_data)); + bcb_to_initial_coverage_spans(mir_body, body_span, bcb, bcb_data, &mut initial_spans); } // Only add the signature span if we found at least one span in the body. @@ -135,8 +134,9 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>( body_span: Span, bcb: BasicCoverageBlock, bcb_data: &'a BasicCoverageBlockData, -) -> impl Iterator + Captures<'a> + Captures<'tcx> { - bcb_data.basic_blocks.iter().flat_map(move |&bb| { + initial_covspans: &mut Vec, +) { + for &bb in &bcb_data.basic_blocks { let data = &mir_body[bb]; let unexpand = move |expn_span| { @@ -146,24 +146,27 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>( .filter(|(span, _)| !span.source_equal(body_span)) }; - let statement_spans = data.statements.iter().filter_map(move |statement| { - let expn_span = filtered_statement_span(statement)?; - let (span, visible_macro) = unexpand(expn_span)?; - - // A statement that looks like the assignment of a closure expression - // is treated as a "hole" span, to be carved out of other spans. - Some(SpanFromMir::new(span, visible_macro, bcb, is_closure_like(statement))) - }); + for statement in data.statements.iter() { + let _: Option<()> = try { + let expn_span = filtered_statement_span(statement)?; + let (span, visible_macro) = unexpand(expn_span)?; + + // A statement that looks like the assignment of a closure expression + // is treated as a "hole" span, to be carved out of other spans. + let covspan = + SpanFromMir::new(span, visible_macro, bcb, is_closure_like(statement)); + initial_covspans.push(covspan); + }; + } - let terminator_span = Some(data.terminator()).into_iter().filter_map(move |terminator| { + let _: Option<()> = try { + let terminator = data.terminator(); let expn_span = filtered_terminator_span(terminator)?; let (span, visible_macro) = unexpand(expn_span)?; - Some(SpanFromMir::new(span, visible_macro, bcb, false)) - }); - - statement_spans.chain(terminator_span) - }) + initial_covspans.push(SpanFromMir::new(span, visible_macro, bcb, false)); + }; + } } fn is_closure_like(statement: &Statement<'_>) -> bool { From 6d1557f268f3dd5e7767ffdf7568856f659a3d57 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 3 Jun 2024 17:15:50 +1000 Subject: [PATCH 15/25] coverage: Use hole spans to carve up coverage spans into separate buckets This performs the same task as the hole-carving code in the main span refiner, but in a separate earlier pass. --- .../rustc_mir_transform/src/coverage/spans.rs | 2 +- .../src/coverage/spans/from_mir.rs | 199 +++++++++++++----- tests/coverage/closure_macro.cov-map | 8 +- tests/coverage/closure_macro_async.cov-map | 8 +- 4 files changed, 152 insertions(+), 65 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index b0bd514f69039..9cda71cdd0770 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -262,7 +262,7 @@ impl SpansRefiner { // a region of code, such as skipping past a hole. debug!(?prev, "prev.span starts after curr.span, so curr will be dropped"); } else { - self.some_curr = Some(CurrCovspan::new(curr.span, curr.bcb, curr.is_hole)); + self.some_curr = Some(CurrCovspan::new(curr.span, curr.bcb, false)); return true; } } diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 3fa37198732ea..b1f71035ddede 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -1,3 +1,6 @@ +use std::collections::VecDeque; + +use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxHashSet; use rustc_middle::bug; use rustc_middle::mir::coverage::CoverageKind; @@ -16,8 +19,11 @@ use crate::coverage::ExtractedHirInfo; /// spans, each associated with a node in the coverage graph (BCB) and possibly /// other metadata. /// -/// The returned spans are sorted in a specific order that is expected by the -/// subsequent span-refinement step. +/// The returned spans are divided into one or more buckets, such that: +/// - The spans in each bucket are strictly after all spans in previous buckets, +/// and strictly before all spans in subsequent buckets. +/// - The contents of each bucket are also sorted, in a specific order that is +/// expected by the subsequent span-refinement step. pub(super) fn mir_to_initial_sorted_coverage_spans( mir_body: &mir::Body<'_>, hir_info: &ExtractedHirInfo, @@ -26,13 +32,21 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( let &ExtractedHirInfo { body_span, .. } = hir_info; let mut initial_spans = vec![]; + let mut holes = vec![]; for (bcb, bcb_data) in basic_coverage_blocks.iter_enumerated() { - bcb_to_initial_coverage_spans(mir_body, body_span, bcb, bcb_data, &mut initial_spans); + bcb_to_initial_coverage_spans( + mir_body, + body_span, + bcb, + bcb_data, + &mut initial_spans, + &mut holes, + ); } // Only add the signature span if we found at least one span in the body. - if !initial_spans.is_empty() { + if !initial_spans.is_empty() || !holes.is_empty() { // If there is no usable signature span, add a fake one (before refinement) // to avoid an ugly gap between the body start and the first real span. // FIXME: Find a more principled way to solve this problem. @@ -44,29 +58,82 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( remove_unwanted_macro_spans(&mut initial_spans); split_visible_macro_spans(&mut initial_spans); - initial_spans.sort_by(|a, b| { - // First sort by span start. - Ord::cmp(&a.span.lo(), &b.span.lo()) - // If span starts are the same, sort by span end in reverse order. - // This ensures that if spans A and B are adjacent in the list, - // and they overlap but are not equal, then either: - // - Span A extends further left, or - // - Both have the same start and span A extends further right - .then_with(|| Ord::cmp(&a.span.hi(), &b.span.hi()).reverse()) - // If two spans have the same lo & hi, put hole spans first, - // as they take precedence over non-hole spans. - .then_with(|| Ord::cmp(&a.is_hole, &b.is_hole).reverse()) + let compare_covspans = |a: &SpanFromMir, b: &SpanFromMir| { + compare_spans(a.span, b.span) // After deduplication, we want to keep only the most-dominated BCB. .then_with(|| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb).reverse()) - }); + }; + initial_spans.sort_by(compare_covspans); - // Among covspans with the same span, keep only one. Hole spans take - // precedence, otherwise keep the one with the most-dominated BCB. + // Among covspans with the same span, keep only one, + // preferring the one with the most-dominated BCB. // (Ideally we should try to preserve _all_ non-dominating BCBs, but that // requires a lot more complexity in the span refiner, for little benefit.) initial_spans.dedup_by(|b, a| a.span.source_equal(b.span)); - vec![initial_spans] + // Sort the holes, and merge overlapping/adjacent holes. + holes.sort_by(|a, b| compare_spans(a.span, b.span)); + holes.dedup_by(|b, a| a.merge_if_overlapping_or_adjacent(b)); + + // Now we're ready to start carving holes out of the initial coverage spans, + // and grouping them in buckets separated by the holes. + + let mut initial_spans = VecDeque::from(initial_spans); + let mut fragments: Vec = vec![]; + + // For each hole: + // - Identify the spans that are entirely or partly before the hole. + // - Put those spans in a corresponding bucket, truncated to the start of the hole. + // - If one of those spans also extends after the hole, put the rest of it + // in a "fragments" vector that is processed by the next hole. + let mut buckets = (0..holes.len()).map(|_| vec![]).collect::>(); + for (hole, bucket) in holes.iter().zip(&mut buckets) { + let fragments_from_prev = std::mem::take(&mut fragments); + + // Only inspect spans that precede or overlap this hole, + // leaving the rest to be inspected by later holes. + // (This relies on the spans and holes both being sorted.) + let relevant_initial_spans = + drain_front_while(&mut initial_spans, |c| c.span.lo() < hole.span.hi()); + + for covspan in fragments_from_prev.into_iter().chain(relevant_initial_spans) { + let (before, after) = covspan.split_around_hole_span(hole.span); + bucket.extend(before); + fragments.extend(after); + } + } + + // After finding the spans before each hole, any remaining fragments/spans + // form their own final bucket, after the final hole. + // (If there were no holes, this will just be all of the initial spans.) + fragments.extend(initial_spans); + buckets.push(fragments); + + // Make sure each individual bucket is still internally sorted. + for bucket in &mut buckets { + bucket.sort_by(compare_covspans); + } + buckets +} + +fn compare_spans(a: Span, b: Span) -> std::cmp::Ordering { + // First sort by span start. + Ord::cmp(&a.lo(), &b.lo()) + // If span starts are the same, sort by span end in reverse order. + // This ensures that if spans A and B are adjacent in the list, + // and they overlap but are not equal, then either: + // - Span A extends further left, or + // - Both have the same start and span A extends further right + .then_with(|| Ord::cmp(&a.hi(), &b.hi()).reverse()) +} + +/// Similar to `.drain(..)`, but stops just before it would remove an item not +/// satisfying the predicate. +fn drain_front_while<'a, T>( + queue: &'a mut VecDeque, + mut pred_fn: impl FnMut(&T) -> bool, +) -> impl Iterator + Captures<'a> { + std::iter::from_fn(move || if pred_fn(queue.front()?) { queue.pop_front() } else { None }) } /// Macros that expand into branches (e.g. `assert!`, `trace!`) tend to generate @@ -79,8 +146,8 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( fn remove_unwanted_macro_spans(initial_spans: &mut Vec) { let mut seen_macro_spans = FxHashSet::default(); initial_spans.retain(|covspan| { - // Ignore (retain) hole spans and non-macro-expansion spans. - if covspan.is_hole || covspan.visible_macro.is_none() { + // Ignore (retain) non-macro-expansion spans. + if covspan.visible_macro.is_none() { return true; } @@ -97,10 +164,6 @@ fn split_visible_macro_spans(initial_spans: &mut Vec) { let mut extra_spans = vec![]; initial_spans.retain(|covspan| { - if covspan.is_hole { - return true; - } - let Some(visible_macro) = covspan.visible_macro else { return true }; let split_len = visible_macro.as_str().len() as u32 + 1; @@ -113,9 +176,8 @@ fn split_visible_macro_spans(initial_spans: &mut Vec) { return true; } - assert!(!covspan.is_hole); - extra_spans.push(SpanFromMir::new(before, covspan.visible_macro, covspan.bcb, false)); - extra_spans.push(SpanFromMir::new(after, covspan.visible_macro, covspan.bcb, false)); + extra_spans.push(SpanFromMir::new(before, covspan.visible_macro, covspan.bcb)); + extra_spans.push(SpanFromMir::new(after, covspan.visible_macro, covspan.bcb)); false // Discard the original covspan that we just split. }); @@ -135,6 +197,7 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>( bcb: BasicCoverageBlock, bcb_data: &'a BasicCoverageBlockData, initial_covspans: &mut Vec, + holes: &mut Vec, ) { for &bb in &bcb_data.basic_blocks { let data = &mir_body[bb]; @@ -146,26 +209,31 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>( .filter(|(span, _)| !span.source_equal(body_span)) }; + let mut extract_statement_span = |statement| { + let expn_span = filtered_statement_span(statement)?; + let (span, visible_macro) = unexpand(expn_span)?; + + // A statement that looks like the assignment of a closure expression + // is treated as a "hole" span, to be carved out of other spans. + if is_closure_like(statement) { + holes.push(Hole { span }); + } else { + initial_covspans.push(SpanFromMir::new(span, visible_macro, bcb)); + } + Some(()) + }; for statement in data.statements.iter() { - let _: Option<()> = try { - let expn_span = filtered_statement_span(statement)?; - let (span, visible_macro) = unexpand(expn_span)?; - - // A statement that looks like the assignment of a closure expression - // is treated as a "hole" span, to be carved out of other spans. - let covspan = - SpanFromMir::new(span, visible_macro, bcb, is_closure_like(statement)); - initial_covspans.push(covspan); - }; + extract_statement_span(statement); } - let _: Option<()> = try { - let terminator = data.terminator(); + let mut extract_terminator_span = |terminator| { let expn_span = filtered_terminator_span(terminator)?; let (span, visible_macro) = unexpand(expn_span)?; - initial_covspans.push(SpanFromMir::new(span, visible_macro, bcb, false)); + initial_covspans.push(SpanFromMir::new(span, visible_macro, bcb)); + Some(()) }; + extract_terminator_span(data.terminator()); } } @@ -333,6 +401,22 @@ fn unexpand_into_body_span_with_prev( Some((curr, prev)) } +#[derive(Debug)] +struct Hole { + span: Span, +} + +impl Hole { + fn merge_if_overlapping_or_adjacent(&mut self, other: &mut Self) -> bool { + if !self.span.overlaps_or_adjacent(other.span) { + return false; + } + + self.span = self.span.to(other.span); + true + } +} + #[derive(Debug)] pub(super) struct SpanFromMir { /// A span that has been extracted from MIR and then "un-expanded" back to @@ -345,23 +429,30 @@ pub(super) struct SpanFromMir { pub(super) span: Span, visible_macro: Option, pub(super) bcb: BasicCoverageBlock, - /// If true, this covspan represents a "hole" that should be carved out - /// from other spans, e.g. because it represents a closure expression that - /// will be instrumented separately as its own function. - pub(super) is_hole: bool, } impl SpanFromMir { fn for_fn_sig(fn_sig_span: Span) -> Self { - Self::new(fn_sig_span, None, START_BCB, false) + Self::new(fn_sig_span, None, START_BCB) + } + + fn new(span: Span, visible_macro: Option, bcb: BasicCoverageBlock) -> Self { + Self { span, visible_macro, bcb } } - fn new( - span: Span, - visible_macro: Option, - bcb: BasicCoverageBlock, - is_hole: bool, - ) -> Self { - Self { span, visible_macro, bcb, is_hole } + /// Splits this span into 0-2 parts: + /// - The part that is strictly before the hole span, if any. + /// - The part that is strictly after the hole span, if any. + fn split_around_hole_span(&self, hole_span: Span) -> (Option, Option) { + let before = try { + let span = self.span.trim_end(hole_span)?; + Self { span, ..*self } + }; + let after = try { + let span = self.span.trim_start(hole_span)?; + Self { span, ..*self } + }; + + (before, after) } } diff --git a/tests/coverage/closure_macro.cov-map b/tests/coverage/closure_macro.cov-map index fd8fbd9fa7575..156947f4e21c1 100644 --- a/tests/coverage/closure_macro.cov-map +++ b/tests/coverage/closure_macro.cov-map @@ -7,16 +7,14 @@ Number of file 0 mappings: 1 - Code(Counter(0)) at (prev + 29, 1) to (start + 2, 2) Function name: closure_macro::main -Raw bytes (36): 0x[01, 01, 01, 01, 05, 06, 01, 21, 01, 01, 21, 02, 02, 09, 00, 12, 02, 00, 0f, 00, 54, 05, 00, 54, 00, 55, 02, 02, 09, 02, 0b, 01, 03, 01, 00, 02] +Raw bytes (31): 0x[01, 01, 01, 01, 05, 05, 01, 21, 01, 01, 21, 02, 02, 09, 00, 0f, 05, 00, 54, 00, 55, 02, 02, 09, 02, 0b, 01, 03, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 1 - expression 0 operands: lhs = Counter(0), rhs = Counter(1) -Number of file 0 mappings: 6 +Number of file 0 mappings: 5 - Code(Counter(0)) at (prev + 33, 1) to (start + 1, 33) -- Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 0, 18) - = (c0 - c1) -- Code(Expression(0, Sub)) at (prev + 0, 15) to (start + 0, 84) +- Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 0, 15) = (c0 - c1) - Code(Counter(1)) at (prev + 0, 84) to (start + 0, 85) - Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 2, 11) diff --git a/tests/coverage/closure_macro_async.cov-map b/tests/coverage/closure_macro_async.cov-map index 43b52008f33e3..0f2b4e0174836 100644 --- a/tests/coverage/closure_macro_async.cov-map +++ b/tests/coverage/closure_macro_async.cov-map @@ -15,16 +15,14 @@ Number of file 0 mappings: 1 - Code(Counter(0)) at (prev + 35, 1) to (start + 0, 43) Function name: closure_macro_async::test::{closure#0} -Raw bytes (36): 0x[01, 01, 01, 01, 05, 06, 01, 23, 2b, 01, 21, 02, 02, 09, 00, 12, 02, 00, 0f, 00, 54, 05, 00, 54, 00, 55, 02, 02, 09, 02, 0b, 01, 03, 01, 00, 02] +Raw bytes (31): 0x[01, 01, 01, 01, 05, 05, 01, 23, 2b, 01, 21, 02, 02, 09, 00, 0f, 05, 00, 54, 00, 55, 02, 02, 09, 02, 0b, 01, 03, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 1 - expression 0 operands: lhs = Counter(0), rhs = Counter(1) -Number of file 0 mappings: 6 +Number of file 0 mappings: 5 - Code(Counter(0)) at (prev + 35, 43) to (start + 1, 33) -- Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 0, 18) - = (c0 - c1) -- Code(Expression(0, Sub)) at (prev + 0, 15) to (start + 0, 84) +- Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 0, 15) = (c0 - c1) - Code(Counter(1)) at (prev + 0, 84) to (start + 0, 85) - Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 2, 11) From c57a1d1baa0a2fc4328f81be66dc7b518c1953bb Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 2 Jun 2024 22:35:13 +1000 Subject: [PATCH 16/25] coverage: Remove hole-carving code from the main span refiner Now that hole spans are handled by a separate earlier pass, this code never sees hole spans, and therefore doesn't need to deal with them. --- .../rustc_mir_transform/src/coverage/spans.rs | 99 ++++--------------- 1 file changed, 19 insertions(+), 80 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 9cda71cdd0770..743f1cc24beee 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -24,8 +24,7 @@ pub(super) fn extract_refined_covspans( from_mir::mir_to_initial_sorted_coverage_spans(mir_body, hir_info, basic_coverage_blocks); for bucket in sorted_span_buckets { let refined_spans = SpansRefiner::refine_sorted_spans(bucket); - code_mappings.extend(refined_spans.into_iter().map(|covspan| { - let RefinedCovspan { span, bcb, is_hole: _ } = covspan; + code_mappings.extend(refined_spans.into_iter().map(|RefinedCovspan { span, bcb }| { // Each span produced by the refiner represents an ordinary code region. mappings::CodeMapping { span, bcb } })); @@ -36,24 +35,16 @@ pub(super) fn extract_refined_covspans( struct CurrCovspan { span: Span, bcb: BasicCoverageBlock, - is_hole: bool, } impl CurrCovspan { - fn new(span: Span, bcb: BasicCoverageBlock, is_hole: bool) -> Self { - Self { span, bcb, is_hole } + fn new(span: Span, bcb: BasicCoverageBlock) -> Self { + Self { span, bcb } } fn into_prev(self) -> PrevCovspan { - let Self { span, bcb, is_hole } = self; - PrevCovspan { span, bcb, merged_spans: vec![span], is_hole } - } - - fn into_refined(self) -> RefinedCovspan { - // This is only called in cases where `curr` is a hole span that has - // been carved out of `prev`. - debug_assert!(self.is_hole); - self.into_prev().into_refined() + let Self { span, bcb } = self; + PrevCovspan { span, bcb, merged_spans: vec![span] } } } @@ -64,12 +55,11 @@ struct PrevCovspan { /// List of all the original spans from MIR that have been merged into this /// span. Mainly used to precisely skip over gaps when truncating a span. merged_spans: Vec, - is_hole: bool, } impl PrevCovspan { fn is_mergeable(&self, other: &CurrCovspan) -> bool { - self.bcb == other.bcb && !self.is_hole && !other.is_hole + self.bcb == other.bcb } fn merge_from(&mut self, other: &CurrCovspan) { @@ -87,14 +77,9 @@ impl PrevCovspan { if self.merged_spans.is_empty() { None } else { Some(self.into_refined()) } } - fn refined_copy(&self) -> RefinedCovspan { - let &Self { span, bcb, merged_spans: _, is_hole } = self; - RefinedCovspan { span, bcb, is_hole } - } - fn into_refined(self) -> RefinedCovspan { - // Even though we consume self, we can just reuse the copying impl. - self.refined_copy() + let Self { span, bcb, merged_spans: _ } = self; + RefinedCovspan { span, bcb } } } @@ -102,12 +87,11 @@ impl PrevCovspan { struct RefinedCovspan { span: Span, bcb: BasicCoverageBlock, - is_hole: bool, } impl RefinedCovspan { fn is_mergeable(&self, other: &Self) -> bool { - self.bcb == other.bcb && !self.is_hole && !other.is_hole + self.bcb == other.bcb } fn merge_from(&mut self, other: &Self) { @@ -122,8 +106,6 @@ impl RefinedCovspan { /// * Remove duplicate source code coverage regions /// * Merge spans that represent continuous (both in source code and control flow), non-branching /// execution -/// * Carve out (leave uncovered) any "hole" spans that need to be left blank -/// (e.g. closures that will be counted by their own MIR body) struct SpansRefiner { /// The initial set of coverage spans, sorted by `Span` (`lo` and `hi`) and by relative /// dominance between the `BasicCoverageBlock`s of equal `Span`s. @@ -184,13 +166,6 @@ impl SpansRefiner { ); let prev = self.take_prev().into_refined(); self.refined_spans.push(prev); - } else if prev.is_hole { - // drop any equal or overlapping span (`curr`) and keep `prev` to test again in the - // next iter - debug!(?prev, "prev (a hole) overlaps curr, so discarding curr"); - self.take_curr(); // Discards curr. - } else if curr.is_hole { - self.carve_out_span_for_hole(); } else { self.cutoff_prev_at_overlapping_curr(); } @@ -214,9 +189,6 @@ impl SpansRefiner { } }); - // Discard hole spans, since their purpose was to carve out chunks from - // other spans, but we don't want the holes themselves in the final mappings. - self.refined_spans.retain(|covspan| !covspan.is_hole); self.refined_spans } @@ -252,50 +224,17 @@ impl SpansRefiner { if let Some(curr) = self.some_curr.take() { self.some_prev = Some(curr.into_prev()); } - while let Some(curr) = self.sorted_spans_iter.next() { - debug!("FOR curr={:?}", curr); - if let Some(prev) = &self.some_prev - && prev.span.lo() > curr.span.lo() - { - // Skip curr because prev has already advanced beyond the end of curr. - // This can only happen if a prior iteration updated `prev` to skip past - // a region of code, such as skipping past a hole. - debug!(?prev, "prev.span starts after curr.span, so curr will be dropped"); - } else { - self.some_curr = Some(CurrCovspan::new(curr.span, curr.bcb, false)); - return true; + if let Some(SpanFromMir { span, bcb, .. }) = self.sorted_spans_iter.next() { + // This code only sees sorted spans after hole-carving, so there should + // be no way for `curr` to start before `prev`. + if let Some(prev) = &self.some_prev { + debug_assert!(prev.span.lo() <= span.lo()); } - } - false - } - - /// If `prev`s span extends left of the hole (`curr`), carve out the hole's span from - /// `prev`'s span. Add the portion of the span to the left of the hole; and if the span - /// extends to the right of the hole, update `prev` to that portion of the span. - fn carve_out_span_for_hole(&mut self) { - let prev = self.prev(); - let curr = self.curr(); - - let left_cutoff = curr.span.lo(); - let right_cutoff = curr.span.hi(); - let has_pre_hole_span = prev.span.lo() < right_cutoff; - let has_post_hole_span = prev.span.hi() > right_cutoff; - - if has_pre_hole_span { - let mut pre_hole = prev.refined_copy(); - pre_hole.span = pre_hole.span.with_hi(left_cutoff); - debug!(?pre_hole, "prev overlaps a hole; adding pre-hole span"); - self.refined_spans.push(pre_hole); - } - - if has_post_hole_span { - // Mutate `prev.span` to start after the hole (and discard curr). - self.prev_mut().span = self.prev().span.with_lo(right_cutoff); - debug!(prev=?self.prev(), "mutated prev to start after the hole"); - - // Prevent this curr from becoming prev. - let hole_covspan = self.take_curr().into_refined(); - self.refined_spans.push(hole_covspan); // since self.prev() was already updated + self.some_curr = Some(CurrCovspan::new(span, bcb)); + debug!(?self.some_prev, ?self.some_curr, "next_coverage_span"); + true + } else { + false } } From fd648a3c76a16a1287ed793fd28ebdc33b7124be Mon Sep 17 00:00:00 2001 From: David Carlier Date: Mon, 3 Jun 2024 19:01:35 +0000 Subject: [PATCH 17/25] std::unix::fs::get_path: using fcntl codepath for netbsd instead. on netbsd, procfs is not as central as on linux/solaris thus can be perfectly not mounted. Thus using fcntl with F_GETPATH, the kernel deals with MAXPATHLEN internally too. --- library/std/src/sys/pal/unix/fs.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/library/std/src/sys/pal/unix/fs.rs b/library/std/src/sys/pal/unix/fs.rs index fbbd40bfb796a..3d4452e73a640 100644 --- a/library/std/src/sys/pal/unix/fs.rs +++ b/library/std/src/sys/pal/unix/fs.rs @@ -1481,29 +1481,33 @@ impl FromRawFd for File { impl fmt::Debug for File { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - #[cfg(any( - target_os = "linux", - target_os = "netbsd", - target_os = "illumos", - target_os = "solaris" - ))] + #[cfg(any(target_os = "linux", target_os = "illumos", target_os = "solaris"))] fn get_path(fd: c_int) -> Option { let mut p = PathBuf::from("/proc/self/fd"); p.push(&fd.to_string()); readlink(&p).ok() } - #[cfg(target_vendor = "apple")] + #[cfg(any(target_vendor = "apple", target_os = "netbsd"))] fn get_path(fd: c_int) -> Option { // FIXME: The use of PATH_MAX is generally not encouraged, but it - // is inevitable in this case because Apple targets define `fcntl` + // is inevitable in this case because Apple targets and NetBSD define `fcntl` // with `F_GETPATH` in terms of `MAXPATHLEN`, and there are no // alternatives. If a better method is invented, it should be used // instead. let mut buf = vec![0; libc::PATH_MAX as usize]; let n = unsafe { libc::fcntl(fd, libc::F_GETPATH, buf.as_ptr()) }; if n == -1 { - return None; + cfg_if::cfg_if! { + if #[cfg(target_os = "netbsd")] { + // fallback to procfs as last resort + let mut p = PathBuf::from("/proc/self/fd"); + p.push(&fd.to_string()); + return readlink(&p).ok(); + } else { + return None; + } + } } let l = buf.iter().position(|&c| c == 0).unwrap(); buf.truncate(l as usize); From dc020ae5d8f396b35e0741aaba677b59e3e7f168 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 29 May 2024 10:13:28 +0000 Subject: [PATCH 18/25] Use a `LocalDefId` for `HirTyLowerer::item_def_id`, since we only ever (can) use it for local items --- compiler/rustc_hir_analysis/src/collect.rs | 4 ++-- .../src/collect/predicates_of.rs | 1 - .../src/hir_ty_lowering/mod.rs | 19 +++++++++---------- compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs | 4 ++-- 4 files changed, 13 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index 7e7460061484e..afdad6935fc9b 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -370,8 +370,8 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> { self.tcx } - fn item_def_id(&self) -> DefId { - self.item_def_id.to_def_id() + fn item_def_id(&self) -> LocalDefId { + self.item_def_id } fn allow_infer(&self) -> bool { diff --git a/compiler/rustc_hir_analysis/src/collect/predicates_of.rs b/compiler/rustc_hir_analysis/src/collect/predicates_of.rs index 072bb7279016a..0abe4f07e190e 100644 --- a/compiler/rustc_hir_analysis/src/collect/predicates_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/predicates_of.rs @@ -117,7 +117,6 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen let mut is_trait = None; let mut is_default_impl_trait = None; - // FIXME: Should ItemCtxt take a LocalDefId? let icx = ItemCtxt::new(tcx, def_id); const NO_GENERICS: &hir::Generics<'_> = hir::Generics::empty(); diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index 2f54349d267ac..9f136ff97b766 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -89,8 +89,8 @@ pub enum PredicateFilter { pub trait HirTyLowerer<'tcx> { fn tcx(&self) -> TyCtxt<'tcx>; - /// Returns the [`DefId`] of the overarching item whose constituents get lowered. - fn item_def_id(&self) -> DefId; + /// Returns the [`LocalDefId`] of the overarching item whose constituents get lowered. + fn item_def_id(&self) -> LocalDefId; /// Returns `true` if the current context allows the use of inference variables. fn allow_infer(&self) -> bool; @@ -1493,16 +1493,15 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { let def_id = self.item_def_id(); debug!(item_def_id = ?def_id); - let parent_def_id = def_id - .as_local() - .map(|def_id| tcx.local_def_id_to_hir_id(def_id)) - .map(|hir_id| tcx.hir().get_parent_item(hir_id).to_def_id()); + // FIXME: document why/how this is different from `tcx.local_parent(def_id)` + let parent_def_id = + tcx.hir().get_parent_item(tcx.local_def_id_to_hir_id(def_id)).to_def_id(); debug!(?parent_def_id); // If the trait in segment is the same as the trait defining the item, // use the `` syntax in the error. - let is_part_of_self_trait_constraints = def_id == trait_def_id; - let is_part_of_fn_in_self_trait = parent_def_id == Some(trait_def_id); + let is_part_of_self_trait_constraints = def_id.to_def_id() == trait_def_id; + let is_part_of_fn_in_self_trait = parent_def_id == trait_def_id; let type_names = if is_part_of_self_trait_constraints || is_part_of_fn_in_self_trait { vec!["Self".to_string()] @@ -1983,7 +1982,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } let sig_generics = self.tcx().generics_of(sig_id); - let parent = self.tcx().parent(self.item_def_id()); + let parent = self.tcx().local_parent(self.item_def_id()); let parent_generics = self.tcx().generics_of(parent); let parent_is_trait = (self.tcx().def_kind(parent) == DefKind::Trait) as usize; @@ -2022,7 +2021,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { let sig = self.tcx().fn_sig(sig_id); let sig_generics = self.tcx().generics_of(sig_id); - let parent = self.tcx().parent(self.item_def_id()); + let parent = self.tcx().local_parent(self.item_def_id()); let parent_def_kind = self.tcx().def_kind(parent); let sig = if let DefKind::Impl { .. } = parent_def_kind diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs index afba812a8e7bb..4d79b3686e2b1 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs @@ -218,8 +218,8 @@ impl<'a, 'tcx> HirTyLowerer<'tcx> for FnCtxt<'a, 'tcx> { self.tcx } - fn item_def_id(&self) -> DefId { - self.body_id.to_def_id() + fn item_def_id(&self) -> LocalDefId { + self.body_id } fn allow_infer(&self) -> bool { From 9d387d14e00f8211d1c52bc1190311de3947c030 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 31 May 2024 10:52:00 +0000 Subject: [PATCH 19/25] Simplify some code paths and remove an unused field `ct_infer` and `lower_ty` will correctly result in an error constant or type respectively, as they go through a `HirTyLowerer` method (just like `HirTyLowerer::allow_infer` is a method implemented by both implementors --- .../src/hir_ty_lowering/mod.rs | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index 9f136ff97b766..055a0f84efc1f 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -421,7 +421,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { def_id: DefId, generic_args: &'a GenericArgs<'tcx>, span: Span, - inferred_params: Vec, infer_args: bool, incorrect_args: &'a Result<(), GenericArgCountMismatch>, } @@ -450,7 +449,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } } - let mut handle_ty_args = |has_default, ty: &hir::Ty<'tcx>| { + let handle_ty_args = |has_default, ty: &hir::Ty<'tcx>| { if has_default { tcx.check_optional_stability( param.def_id, @@ -467,12 +466,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { }, ); } - if let (hir::TyKind::Infer, false) = (&ty.kind, self.lowerer.allow_infer()) { - self.inferred_params.push(ty.span); - Ty::new_misc_error(tcx).into() - } else { - self.lowerer.lower_ty(ty).into() - } + self.lowerer.lower_ty(ty).into() }; match (¶m.kind, arg) { @@ -496,12 +490,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { .type_of(param.def_id) .no_bound_vars() .expect("const parameter types cannot be generic"); - if self.lowerer.allow_infer() { - self.lowerer.ct_infer(ty, Some(param), inf.span).into() - } else { - self.inferred_params.push(inf.span); - ty::Const::new_misc_error(tcx, ty).into() - } + self.lowerer.ct_infer(ty, Some(param), inf.span).into() } (kind, arg) => span_bug!( self.span, @@ -604,7 +593,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { def_id, span, generic_args: segment.args(), - inferred_params: vec![], infer_args: segment.infer_args, incorrect_args: &arg_count.correct, }; From abd308b8863ae43ee96fc98fa4a5e90ecb31f12f Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 31 May 2024 13:09:18 +0000 Subject: [PATCH 20/25] Remove an `Option` and instead eagerly create error lifetimes --- compiler/rustc_hir_analysis/src/collect.rs | 25 +++++++++-- .../src/hir_ty_lowering/mod.rs | 43 ++++++------------- .../src/hir_ty_lowering/object_safety.rs | 19 +------- .../rustc_hir_typeck/src/fn_ctxt/_impl.rs | 2 +- compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs | 9 +++- 5 files changed, 43 insertions(+), 55 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index afdad6935fc9b..538d87a300d42 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -18,7 +18,7 @@ use rustc_ast::Recovered; use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::{FxHashSet, FxIndexMap}; use rustc_data_structures::unord::UnordMap; -use rustc_errors::{Applicability, Diag, ErrorGuaranteed, StashKey}; +use rustc_errors::{struct_span_code_err, Applicability, Diag, ErrorGuaranteed, StashKey, E0228}; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LocalDefId}; @@ -378,8 +378,27 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> { false } - fn re_infer(&self, _: Option<&ty::GenericParamDef>, _: Span) -> Option> { - None + fn re_infer( + &self, + _: Option<&ty::GenericParamDef>, + span: Span, + object_lifetime_default: bool, + ) -> ty::Region<'tcx> { + if object_lifetime_default { + let e = struct_span_code_err!( + self.tcx().dcx(), + span, + E0228, + "the lifetime bound for this object type cannot be deduced \ + from context; please supply an explicit bound" + ) + .emit(); + self.set_tainted_by_errors(e); + ty::Region::new_error(self.tcx(), e) + } else { + // This indicates an illegal lifetime in a non-assoc-trait position + ty::Region::new_error_with_message(self.tcx(), span, "unelided lifetime in signature") + } } fn ty_infer(&self, _: Option<&ty::GenericParamDef>, span: Span) -> Ty<'tcx> { diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index 055a0f84efc1f..7076983284c5a 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -96,8 +96,14 @@ pub trait HirTyLowerer<'tcx> { fn allow_infer(&self) -> bool; /// Returns the region to use when a lifetime is omitted (and not elided). - fn re_infer(&self, param: Option<&ty::GenericParamDef>, span: Span) - -> Option>; + /// + /// The `object_lifetime_default` argument states whether this lifetime is from a reference. + fn re_infer( + &self, + param: Option<&ty::GenericParamDef>, + span: Span, + object_lifetime_default: bool, + ) -> ty::Region<'tcx>; /// Returns the type to use when a type is omitted. fn ty_infer(&self, param: Option<&ty::GenericParamDef>, span: Span) -> Ty<'tcx>; @@ -292,21 +298,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { Some(rbv::ResolvedArg::Error(guar)) => ty::Region::new_error(tcx, guar), - None => { - self.re_infer(def, lifetime.ident.span).unwrap_or_else(|| { - debug!(?lifetime, "unelided lifetime in signature"); - - // This indicates an illegal lifetime - // elision. `resolve_lifetime` should have - // reported an error in this case -- but if - // not, let's error out. - ty::Region::new_error_with_message( - tcx, - lifetime.ident.span, - "unelided lifetime in signature", - ) - }) - } + None => self.re_infer(def, lifetime.ident.span, false), } } @@ -513,20 +505,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } } match param.kind { - GenericParamDefKind::Lifetime => self - .lowerer - .re_infer(Some(param), self.span) - .unwrap_or_else(|| { - debug!(?param, "unelided lifetime in signature"); - - // This indicates an illegal lifetime in a non-assoc-trait position - ty::Region::new_error_with_message( - tcx, - self.span, - "unelided lifetime in signature", - ) - }) - .into(), + GenericParamDefKind::Lifetime => { + self.lowerer.re_infer(Some(param), self.span, false).into() + } GenericParamDefKind::Type { has_default, .. } => { if !infer_args && has_default { // No type parameter provided, but a default exists. diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/object_safety.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/object_safety.rs index 4f7a39d02503b..fef80102b6234 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/object_safety.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/object_safety.rs @@ -327,24 +327,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { if tcx.named_bound_var(lifetime.hir_id).is_some() { self.lower_lifetime(lifetime, None) } else { - self.re_infer(None, span).unwrap_or_else(|| { - let err = struct_span_code_err!( - tcx.dcx(), - span, - E0228, - "the lifetime bound for this object type cannot be deduced \ - from context; please supply an explicit bound" - ); - let e = if borrowed { - // We will have already emitted an error E0106 complaining about a - // missing named lifetime in `&dyn Trait`, so we elide this one. - err.delay_as_bug() - } else { - err.emit() - }; - self.set_tainted_by_errors(e); - ty::Region::new_error(tcx, e) - }) + self.re_infer(None, span, !borrowed) } }) }; diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs index 58eb0c2817987..fb44e542f7ad5 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs @@ -1325,7 +1325,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let tcx = self.fcx.tcx(); match param.kind { GenericParamDefKind::Lifetime => { - self.fcx.re_infer(Some(param), self.span).unwrap().into() + self.fcx.re_infer(Some(param), self.span, false).into() } GenericParamDefKind::Type { has_default, .. } => { if !infer_args && has_default { diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs index 4d79b3686e2b1..d250dee5113d5 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs @@ -226,12 +226,17 @@ impl<'a, 'tcx> HirTyLowerer<'tcx> for FnCtxt<'a, 'tcx> { true } - fn re_infer(&self, def: Option<&ty::GenericParamDef>, span: Span) -> Option> { + fn re_infer( + &self, + def: Option<&ty::GenericParamDef>, + span: Span, + _object_lifetime_default: bool, + ) -> ty::Region<'tcx> { let v = match def { Some(def) => infer::RegionParameterDefinition(span, def.name), None => infer::MiscVariable(span), }; - Some(self.next_region_var(v)) + self.next_region_var(v) } fn ty_infer(&self, param: Option<&ty::GenericParamDef>, span: Span) -> Ty<'tcx> { From 6a2e15a6f08fa1869cd9aacb89c7b2d033c54e65 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 31 May 2024 13:36:27 +0000 Subject: [PATCH 21/25] Only collect infer vars to error about in case infer vars are actually forbidden --- compiler/rustc_hir_analysis/src/collect.rs | 87 ++++++++++++++++- .../src/hir_ty_lowering/mod.rs | 93 ++----------------- compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs | 16 ++++ 3 files changed, 111 insertions(+), 85 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index 538d87a300d42..f6e5aeb539626 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -19,10 +19,10 @@ use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::{FxHashSet, FxIndexMap}; use rustc_data_structures::unord::UnordMap; use rustc_errors::{struct_span_code_err, Applicability, Diag, ErrorGuaranteed, StashKey, E0228}; -use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LocalDefId}; -use rustc_hir::intravisit::{self, Visitor}; +use rustc_hir::intravisit::{self, walk_generics, Visitor}; +use rustc_hir::{self as hir}; use rustc_hir::{GenericParamKind, Node}; use rustc_infer::infer::{InferCtxt, TyCtxtInferExt}; use rustc_infer::traits::ObligationCause; @@ -529,6 +529,89 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> { fn set_tainted_by_errors(&self, err: ErrorGuaranteed) { self.tainted_by_errors.set(Some(err)); } + + fn lower_fn_sig( + &self, + decl: &hir::FnDecl<'tcx>, + generics: Option<&hir::Generics<'_>>, + hir_id: rustc_hir::HirId, + hir_ty: Option<&hir::Ty<'_>>, + ) -> (Vec>, Ty<'tcx>) { + let tcx = self.tcx(); + // We proactively collect all the inferred type params to emit a single error per fn def. + let mut visitor = HirPlaceholderCollector::default(); + let mut infer_replacements = vec![]; + + if let Some(generics) = generics { + walk_generics(&mut visitor, generics); + } + + let input_tys = decl + .inputs + .iter() + .enumerate() + .map(|(i, a)| { + if let hir::TyKind::Infer = a.kind { + if let Some(suggested_ty) = + self.lowerer().suggest_trait_fn_ty_for_impl_fn_infer(hir_id, Some(i)) + { + infer_replacements.push((a.span, suggested_ty.to_string())); + return Ty::new_error_with_message(tcx, a.span, suggested_ty.to_string()); + } + } + + // Only visit the type looking for `_` if we didn't fix the type above + visitor.visit_ty(a); + self.lowerer().lower_arg_ty(a, None) + }) + .collect(); + + let output_ty = match decl.output { + hir::FnRetTy::Return(output) => { + if let hir::TyKind::Infer = output.kind + && let Some(suggested_ty) = + self.lowerer().suggest_trait_fn_ty_for_impl_fn_infer(hir_id, None) + { + infer_replacements.push((output.span, suggested_ty.to_string())); + Ty::new_error_with_message(tcx, output.span, suggested_ty.to_string()) + } else { + visitor.visit_ty(output); + self.lower_ty(output) + } + } + hir::FnRetTy::DefaultReturn(..) => tcx.types.unit, + }; + + if !(visitor.0.is_empty() && infer_replacements.is_empty()) { + // We check for the presence of + // `ident_span` to not emit an error twice when we have `fn foo(_: fn() -> _)`. + + let mut diag = crate::collect::placeholder_type_error_diag( + tcx, + generics, + visitor.0, + infer_replacements.iter().map(|(s, _)| *s).collect(), + true, + hir_ty, + "function", + ); + + if !infer_replacements.is_empty() { + diag.multipart_suggestion( + format!( + "try replacing `_` with the type{} in the corresponding trait method signature", + rustc_errors::pluralize!(infer_replacements.len()), + ), + infer_replacements, + Applicability::MachineApplicable, + ); + } + + self.set_tainted_by_errors(diag.emit()); + } + + (input_tys, output_ty) + } } /// Synthesize a new lifetime name that doesn't clash with any of the lifetimes already present. diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index 7076983284c5a..8f498fcdf3664 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -20,7 +20,6 @@ mod lint; mod object_safety; use crate::bounds::Bounds; -use crate::collect::HirPlaceholderCollector; use crate::errors::{AmbiguousLifetimeBound, WildPatTy}; use crate::hir_ty_lowering::errors::{prohibit_assoc_item_constraint, GenericsArgsErrExtend}; use crate::hir_ty_lowering::generics::{check_generic_arg_count, lower_generic_args}; @@ -34,7 +33,6 @@ use rustc_errors::{ use rustc_hir as hir; use rustc_hir::def::{CtorOf, DefKind, Namespace, Res}; use rustc_hir::def_id::{DefId, LocalDefId}; -use rustc_hir::intravisit::{walk_generics, Visitor as _}; use rustc_hir::{GenericArg, GenericArgs, HirId}; use rustc_infer::infer::{InferCtxt, TyCtxtInferExt}; use rustc_infer::traits::ObligationCause; @@ -157,6 +155,14 @@ pub trait HirTyLowerer<'tcx> { poly_trait_ref: ty::PolyTraitRef<'tcx>, ) -> Ty<'tcx>; + fn lower_fn_sig( + &self, + decl: &hir::FnDecl<'tcx>, + generics: Option<&hir::Generics<'_>>, + hir_id: HirId, + hir_ty: Option<&hir::Ty<'_>>, + ) -> (Vec>, Ty<'tcx>); + /// Returns `AdtDef` if `ty` is an ADT. /// /// Note that `ty` might be a alias type that needs normalization. @@ -2306,92 +2312,13 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { let bound_vars = tcx.late_bound_vars(hir_id); debug!(?bound_vars); - // We proactively collect all the inferred type params to emit a single error per fn def. - let mut visitor = HirPlaceholderCollector::default(); - let mut infer_replacements = vec![]; - - if let Some(generics) = generics { - walk_generics(&mut visitor, generics); - } - - let input_tys: Vec<_> = decl - .inputs - .iter() - .enumerate() - .map(|(i, a)| { - if let hir::TyKind::Infer = a.kind - && !self.allow_infer() - { - if let Some(suggested_ty) = - self.suggest_trait_fn_ty_for_impl_fn_infer(hir_id, Some(i)) - { - infer_replacements.push((a.span, suggested_ty.to_string())); - return Ty::new_error_with_message( - self.tcx(), - a.span, - suggested_ty.to_string(), - ); - } - } - - // Only visit the type looking for `_` if we didn't fix the type above - visitor.visit_ty(a); - self.lower_arg_ty(a, None) - }) - .collect(); - - let output_ty = match decl.output { - hir::FnRetTy::Return(output) => { - if let hir::TyKind::Infer = output.kind - && !self.allow_infer() - && let Some(suggested_ty) = - self.suggest_trait_fn_ty_for_impl_fn_infer(hir_id, None) - { - infer_replacements.push((output.span, suggested_ty.to_string())); - Ty::new_error_with_message(self.tcx(), output.span, suggested_ty.to_string()) - } else { - visitor.visit_ty(output); - self.lower_ty(output) - } - } - hir::FnRetTy::DefaultReturn(..) => tcx.types.unit, - }; + let (input_tys, output_ty) = self.lower_fn_sig(decl, generics, hir_id, hir_ty); debug!(?output_ty); let fn_ty = tcx.mk_fn_sig(input_tys, output_ty, decl.c_variadic, safety, abi); let bare_fn_ty = ty::Binder::bind_with_vars(fn_ty, bound_vars); - if !self.allow_infer() && !(visitor.0.is_empty() && infer_replacements.is_empty()) { - // We always collect the spans for placeholder types when evaluating `fn`s, but we - // only want to emit an error complaining about them if infer types (`_`) are not - // allowed. `allow_infer` gates this behavior. We check for the presence of - // `ident_span` to not emit an error twice when we have `fn foo(_: fn() -> _)`. - - let mut diag = crate::collect::placeholder_type_error_diag( - tcx, - generics, - visitor.0, - infer_replacements.iter().map(|(s, _)| *s).collect(), - true, - hir_ty, - "function", - ); - - if !infer_replacements.is_empty() { - diag.multipart_suggestion( - format!( - "try replacing `_` with the type{} in the corresponding trait method signature", - rustc_errors::pluralize!(infer_replacements.len()), - ), - infer_replacements, - Applicability::MachineApplicable, - ); - } - - self.set_tainted_by_errors(diag.emit()); - } - // Find any late-bound regions declared in return type that do // not appear in the arguments. These are not well-formed. // @@ -2421,7 +2348,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { /// corresponding function in the trait that the impl implements, if it exists. /// If arg_idx is Some, then it corresponds to an input type index, otherwise it /// corresponds to the return type. - fn suggest_trait_fn_ty_for_impl_fn_infer( + pub(super) fn suggest_trait_fn_ty_for_impl_fn_infer( &self, fn_hir_id: HirId, arg_idx: Option, diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs index d250dee5113d5..e62c4639b43d5 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs @@ -355,6 +355,22 @@ impl<'a, 'tcx> HirTyLowerer<'tcx> for FnCtxt<'a, 'tcx> { fn set_tainted_by_errors(&self, e: ErrorGuaranteed) { self.infcx.set_tainted_by_errors(e) } + + fn lower_fn_sig( + &self, + decl: &rustc_hir::FnDecl<'tcx>, + _generics: Option<&rustc_hir::Generics<'_>>, + _hir_id: rustc_hir::HirId, + _hir_ty: Option<&hir::Ty<'_>>, + ) -> (Vec>, Ty<'tcx>) { + let input_tys = decl.inputs.iter().map(|a| self.lowerer().lower_arg_ty(a, None)).collect(); + + let output_ty = match decl.output { + hir::FnRetTy::Return(output) => self.lowerer().lower_ty(output), + hir::FnRetTy::DefaultReturn(..) => self.tcx().types.unit, + }; + (input_tys, output_ty) + } } /// The `ty` representation of a user-provided type. Depending on the use-site From 78706740d57d54a6da8abc8eda148c3bcdb64d40 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 31 May 2024 13:42:13 +0000 Subject: [PATCH 22/25] Remove `allows_infer` now that every use of it is delegated to `HirTyLowerer` --- compiler/rustc_hir_analysis/src/collect.rs | 4 ---- compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs | 3 --- compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs | 4 ---- 3 files changed, 11 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index f6e5aeb539626..f5bcbf852bda6 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -374,10 +374,6 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> { self.item_def_id } - fn allow_infer(&self) -> bool { - false - } - fn re_infer( &self, _: Option<&ty::GenericParamDef>, diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index 8f498fcdf3664..87c34ac535fe0 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -90,9 +90,6 @@ pub trait HirTyLowerer<'tcx> { /// Returns the [`LocalDefId`] of the overarching item whose constituents get lowered. fn item_def_id(&self) -> LocalDefId; - /// Returns `true` if the current context allows the use of inference variables. - fn allow_infer(&self) -> bool; - /// Returns the region to use when a lifetime is omitted (and not elided). /// /// The `object_lifetime_default` argument states whether this lifetime is from a reference. diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs index e62c4639b43d5..6e4a464905fa5 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs @@ -222,10 +222,6 @@ impl<'a, 'tcx> HirTyLowerer<'tcx> for FnCtxt<'a, 'tcx> { self.body_id } - fn allow_infer(&self) -> bool { - true - } - fn re_infer( &self, def: Option<&ty::GenericParamDef>, From 4146b8280a70212ac8cec8ce69e5aff2a4a74577 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 31 May 2024 13:52:37 +0000 Subject: [PATCH 23/25] Remove some unnecessary explicit lifetimes --- compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs index 6e4a464905fa5..954a0956b70de 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs @@ -213,8 +213,8 @@ impl<'a, 'tcx> Deref for FnCtxt<'a, 'tcx> { } } -impl<'a, 'tcx> HirTyLowerer<'tcx> for FnCtxt<'a, 'tcx> { - fn tcx<'b>(&'b self) -> TyCtxt<'tcx> { +impl<'tcx> HirTyLowerer<'tcx> for FnCtxt<'_, 'tcx> { + fn tcx(&self) -> TyCtxt<'tcx> { self.tcx } From c8a331ac527264ce46b28a8592c31dcef2a5759b Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 4 Jun 2024 13:36:28 +0000 Subject: [PATCH 24/25] Unify optional param info with object lifetime default boolean into an enum that exhaustively supports all call sites --- compiler/rustc_hir_analysis/src/collect.rs | 11 ++---- .../src/collect/predicates_of.rs | 13 ++++--- .../src/hir_ty_lowering/bounds.rs | 4 ++- .../src/hir_ty_lowering/mod.rs | 35 +++++++++++-------- .../src/hir_ty_lowering/object_safety.rs | 17 ++++++--- .../rustc_hir_typeck/src/fn_ctxt/_impl.rs | 20 +++++++---- compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs | 15 +++----- .../rustc_hir_typeck/src/method/confirm.rs | 13 ++++--- 8 files changed, 75 insertions(+), 53 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index f5bcbf852bda6..d84dcb78ad2e1 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -44,7 +44,7 @@ use std::ops::Bound; use crate::check::intrinsic::intrinsic_operation_unsafety; use crate::errors; -use crate::hir_ty_lowering::HirTyLowerer; +use crate::hir_ty_lowering::{HirTyLowerer, RegionInferReason}; pub use type_of::test_opaque_hidden_types; mod generics_of; @@ -374,13 +374,8 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> { self.item_def_id } - fn re_infer( - &self, - _: Option<&ty::GenericParamDef>, - span: Span, - object_lifetime_default: bool, - ) -> ty::Region<'tcx> { - if object_lifetime_default { + fn re_infer(&self, span: Span, reason: RegionInferReason<'_>) -> ty::Region<'tcx> { + if let RegionInferReason::BorrowedObjectLifetimeDefault = reason { let e = struct_span_code_err!( self.tcx().dcx(), span, diff --git a/compiler/rustc_hir_analysis/src/collect/predicates_of.rs b/compiler/rustc_hir_analysis/src/collect/predicates_of.rs index 0abe4f07e190e..913fae6b5b692 100644 --- a/compiler/rustc_hir_analysis/src/collect/predicates_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/predicates_of.rs @@ -1,7 +1,7 @@ use crate::bounds::Bounds; use crate::collect::ItemCtxt; use crate::constrained_generic_params as cgp; -use crate::hir_ty_lowering::{HirTyLowerer, OnlySelfBounds, PredicateFilter}; +use crate::hir_ty_lowering::{HirTyLowerer, OnlySelfBounds, PredicateFilter, RegionInferReason}; use hir::{HirId, Node}; use rustc_data_structures::fx::FxIndexSet; use rustc_hir as hir; @@ -243,12 +243,15 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen } hir::WherePredicate::RegionPredicate(region_pred) => { - let r1 = icx.lowerer().lower_lifetime(region_pred.lifetime, None); + let r1 = icx + .lowerer() + .lower_lifetime(region_pred.lifetime, RegionInferReason::RegionPredicate); predicates.extend(region_pred.bounds.iter().map(|bound| { let (r2, span) = match bound { - hir::GenericBound::Outlives(lt) => { - (icx.lowerer().lower_lifetime(lt, None), lt.ident.span) - } + hir::GenericBound::Outlives(lt) => ( + icx.lowerer().lower_lifetime(lt, RegionInferReason::RegionPredicate), + lt.ident.span, + ), bound => { span_bug!( bound.span(), diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs index b6a1799c03f11..73ce577907ebe 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs @@ -18,6 +18,8 @@ use crate::bounds::Bounds; use crate::errors; use crate::hir_ty_lowering::{HirTyLowerer, OnlySelfBounds, PredicateFilter}; +use super::RegionInferReason; + impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { /// Add a `Sized` bound to the `bounds` if appropriate. /// @@ -166,7 +168,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { ); } hir::GenericBound::Outlives(lifetime) => { - let region = self.lower_lifetime(lifetime, None); + let region = self.lower_lifetime(lifetime, RegionInferReason::OutlivesBound); bounds.push_region_bound( self.tcx(), ty::Binder::bind_with_vars( diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index 87c34ac535fe0..9b8edbc10ad89 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -80,6 +80,20 @@ pub enum PredicateFilter { SelfAndAssociatedTypeBounds, } +#[derive(Debug)] +pub enum RegionInferReason<'a> { + /// Lifetime on a trait object behind a reference. + /// This allows inferring information from the reference. + BorrowedObjectLifetimeDefault, + /// A trait object's lifetime. + ObjectLifetimeDefault, + /// Generic lifetime parameter + Param(&'a ty::GenericParamDef), + RegionPredicate, + Reference, + OutlivesBound, +} + /// A context which can lower type-system entities from the [HIR][hir] to /// the [`rustc_middle::ty`] representation. /// @@ -91,14 +105,7 @@ pub trait HirTyLowerer<'tcx> { fn item_def_id(&self) -> LocalDefId; /// Returns the region to use when a lifetime is omitted (and not elided). - /// - /// The `object_lifetime_default` argument states whether this lifetime is from a reference. - fn re_infer( - &self, - param: Option<&ty::GenericParamDef>, - span: Span, - object_lifetime_default: bool, - ) -> ty::Region<'tcx>; + fn re_infer(&self, span: Span, reason: RegionInferReason<'_>) -> ty::Region<'tcx>; /// Returns the type to use when a type is omitted. fn ty_infer(&self, param: Option<&ty::GenericParamDef>, span: Span) -> Ty<'tcx>; @@ -267,7 +274,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { pub fn lower_lifetime( &self, lifetime: &hir::Lifetime, - def: Option<&ty::GenericParamDef>, + reason: RegionInferReason<'_>, ) -> ty::Region<'tcx> { let tcx = self.tcx(); let lifetime_name = |def_id| tcx.hir().name(tcx.local_def_id_to_hir_id(def_id)); @@ -301,7 +308,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { Some(rbv::ResolvedArg::Error(guar)) => ty::Region::new_error(tcx, guar), - None => self.re_infer(def, lifetime.ident.span, false), + None => self.re_infer(lifetime.ident.span, reason), } } @@ -466,7 +473,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { match (¶m.kind, arg) { (GenericParamDefKind::Lifetime, GenericArg::Lifetime(lt)) => { - self.lowerer.lower_lifetime(lt, Some(param)).into() + self.lowerer.lower_lifetime(lt, RegionInferReason::Param(param)).into() } (&GenericParamDefKind::Type { has_default, .. }, GenericArg::Type(ty)) => { handle_ty_args(has_default, ty) @@ -509,7 +516,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } match param.kind { GenericParamDefKind::Lifetime => { - self.lowerer.re_infer(Some(param), self.span, false).into() + self.lowerer.re_infer(self.span, RegionInferReason::Param(param)).into() } GenericParamDefKind::Type { has_default, .. } => { if !infer_args && has_default { @@ -2041,7 +2048,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { hir::TyKind::Slice(ty) => Ty::new_slice(tcx, self.lower_ty(ty)), hir::TyKind::Ptr(mt) => Ty::new_ptr(tcx, self.lower_ty(mt.ty), mt.mutbl), hir::TyKind::Ref(region, mt) => { - let r = self.lower_lifetime(region, None); + let r = self.lower_lifetime(region, RegionInferReason::Reference); debug!(?r); let t = self.lower_ty_common(mt.ty, true, false); Ty::new_ref(tcx, r, t, mt.mutbl) @@ -2270,7 +2277,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { &lifetimes[i] ) }; - self.lower_lifetime(lifetime, None).into() + self.lower_lifetime(lifetime, RegionInferReason::Param(¶m)).into() } else { tcx.mk_param_from_def(param) } diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/object_safety.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/object_safety.rs index fef80102b6234..b9c5ae0c65b55 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/object_safety.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/object_safety.rs @@ -1,5 +1,7 @@ use crate::bounds::Bounds; -use crate::hir_ty_lowering::{GenericArgCountMismatch, GenericArgCountResult, OnlySelfBounds}; +use crate::hir_ty_lowering::{ + GenericArgCountMismatch, GenericArgCountResult, OnlySelfBounds, RegionInferReason, +}; use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet}; use rustc_errors::{codes::*, struct_span_code_err}; use rustc_hir as hir; @@ -321,13 +323,20 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { // Use explicitly-specified region bound. let region_bound = if !lifetime.is_elided() { - self.lower_lifetime(lifetime, None) + self.lower_lifetime(lifetime, RegionInferReason::ObjectLifetimeDefault) } else { self.compute_object_lifetime_bound(span, existential_predicates).unwrap_or_else(|| { if tcx.named_bound_var(lifetime.hir_id).is_some() { - self.lower_lifetime(lifetime, None) + self.lower_lifetime(lifetime, RegionInferReason::ObjectLifetimeDefault) } else { - self.re_infer(None, span, !borrowed) + self.re_infer( + span, + if borrowed { + RegionInferReason::ObjectLifetimeDefault + } else { + RegionInferReason::BorrowedObjectLifetimeDefault + }, + ) } }) }; diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs index fb44e542f7ad5..250e9c3c313e5 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs @@ -16,7 +16,7 @@ use rustc_hir_analysis::hir_ty_lowering::generics::{ }; use rustc_hir_analysis::hir_ty_lowering::{ ExplicitLateBound, GenericArgCountMismatch, GenericArgCountResult, GenericArgsLowerer, - GenericPathSegment, HirTyLowerer, IsMethodCall, + GenericPathSegment, HirTyLowerer, IsMethodCall, RegionInferReason, }; use rustc_infer::infer::canonical::{Canonical, OriginalQueryValues, QueryResponse}; use rustc_infer::infer::error_reporting::TypeAnnotationNeeded::E0282; @@ -1280,9 +1280,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { arg: &GenericArg<'tcx>, ) -> ty::GenericArg<'tcx> { match (¶m.kind, arg) { - (GenericParamDefKind::Lifetime, GenericArg::Lifetime(lt)) => { - self.fcx.lowerer().lower_lifetime(lt, Some(param)).into() - } + (GenericParamDefKind::Lifetime, GenericArg::Lifetime(lt)) => self + .fcx + .lowerer() + .lower_lifetime(lt, RegionInferReason::Param(param)) + .into(), (GenericParamDefKind::Type { .. }, GenericArg::Type(ty)) => { self.fcx.lower_ty(ty).raw.into() } @@ -1324,9 +1326,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) -> ty::GenericArg<'tcx> { let tcx = self.fcx.tcx(); match param.kind { - GenericParamDefKind::Lifetime => { - self.fcx.re_infer(Some(param), self.span, false).into() - } + GenericParamDefKind::Lifetime => self + .fcx + .re_infer( + self.span, + rustc_hir_analysis::hir_ty_lowering::RegionInferReason::Param(param), + ) + .into(), GenericParamDefKind::Type { has_default, .. } => { if !infer_args && has_default { // If we have a default, then it doesn't matter that we're not diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs index 954a0956b70de..b9ff348c42ff5 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs @@ -15,7 +15,7 @@ use hir::def_id::CRATE_DEF_ID; use rustc_errors::DiagCtxt; use rustc_hir as hir; use rustc_hir::def_id::{DefId, LocalDefId}; -use rustc_hir_analysis::hir_ty_lowering::HirTyLowerer; +use rustc_hir_analysis::hir_ty_lowering::{HirTyLowerer, RegionInferReason}; use rustc_infer::infer; use rustc_infer::infer::error_reporting::sub_relations::SubRelations; use rustc_infer::infer::error_reporting::TypeErrCtxt; @@ -222,15 +222,10 @@ impl<'tcx> HirTyLowerer<'tcx> for FnCtxt<'_, 'tcx> { self.body_id } - fn re_infer( - &self, - def: Option<&ty::GenericParamDef>, - span: Span, - _object_lifetime_default: bool, - ) -> ty::Region<'tcx> { - let v = match def { - Some(def) => infer::RegionParameterDefinition(span, def.name), - None => infer::MiscVariable(span), + fn re_infer(&self, span: Span, reason: RegionInferReason<'_>) -> ty::Region<'tcx> { + let v = match reason { + RegionInferReason::Param(def) => infer::RegionParameterDefinition(span, def.name), + _ => infer::MiscVariable(span), }; self.next_region_var(v) } diff --git a/compiler/rustc_hir_typeck/src/method/confirm.rs b/compiler/rustc_hir_typeck/src/method/confirm.rs index 0825e66137303..949b924100537 100644 --- a/compiler/rustc_hir_typeck/src/method/confirm.rs +++ b/compiler/rustc_hir_typeck/src/method/confirm.rs @@ -7,7 +7,9 @@ use rustc_hir::GenericArg; use rustc_hir_analysis::hir_ty_lowering::generics::{ check_generic_arg_count_for_call, lower_generic_args, }; -use rustc_hir_analysis::hir_ty_lowering::{GenericArgsLowerer, HirTyLowerer, IsMethodCall}; +use rustc_hir_analysis::hir_ty_lowering::{ + GenericArgsLowerer, HirTyLowerer, IsMethodCall, RegionInferReason, +}; use rustc_infer::infer::{self, DefineOpaqueTypes, InferOk}; use rustc_middle::traits::{ObligationCauseCode, UnifyReceiverContext}; use rustc_middle::ty::adjustment::{Adjust, Adjustment, PointerCoercion}; @@ -388,9 +390,12 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> { arg: &GenericArg<'tcx>, ) -> ty::GenericArg<'tcx> { match (¶m.kind, arg) { - (GenericParamDefKind::Lifetime, GenericArg::Lifetime(lt)) => { - self.cfcx.fcx.lowerer().lower_lifetime(lt, Some(param)).into() - } + (GenericParamDefKind::Lifetime, GenericArg::Lifetime(lt)) => self + .cfcx + .fcx + .lowerer() + .lower_lifetime(lt, RegionInferReason::Param(param)) + .into(), (GenericParamDefKind::Type { .. }, GenericArg::Type(ty)) => { self.cfcx.lower_ty(ty).raw.into() } From a8e091de4afb964480aa52642838ee5049c89cad Mon Sep 17 00:00:00 2001 From: lcnr Date: Wed, 5 Jun 2024 11:46:52 +0200 Subject: [PATCH 25/25] bivariant alias: set `has_unconstrained_ty_var` --- .../src/infer/relate/generalize.rs | 12 +++++++++-- .../next-solver/generalize/bivariant-alias.rs | 20 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 tests/ui/traits/next-solver/generalize/bivariant-alias.rs diff --git a/compiler/rustc_infer/src/infer/relate/generalize.rs b/compiler/rustc_infer/src/infer/relate/generalize.rs index d4c7d752c953f..aaea3b4820b35 100644 --- a/compiler/rustc_infer/src/infer/relate/generalize.rs +++ b/compiler/rustc_infer/src/infer/relate/generalize.rs @@ -329,6 +329,14 @@ impl<'tcx> Generalizer<'_, 'tcx> { } } + /// Create a new type variable in the universe of the target when + /// generalizing an alias. This has to set `has_unconstrained_ty_var` + /// if we're currently in a bivariant context. + fn next_ty_var_for_alias(&mut self) -> Ty<'tcx> { + self.has_unconstrained_ty_var |= self.ambient_variance == ty::Bivariant; + self.infcx.next_ty_var_in_universe(self.span, self.for_universe) + } + /// An occurs check failure inside of an alias does not mean /// that the types definitely don't unify. We may be able /// to normalize the alias after all. @@ -358,7 +366,7 @@ impl<'tcx> Generalizer<'_, 'tcx> { // // cc trait-system-refactor-initiative#110 if self.infcx.next_trait_solver() && !alias.has_escaping_bound_vars() && !self.in_alias { - return Ok(self.infcx.next_ty_var_in_universe(self.span, self.for_universe)); + return Ok(self.next_ty_var_for_alias()); } let is_nested_alias = mem::replace(&mut self.in_alias, true); @@ -378,7 +386,7 @@ impl<'tcx> Generalizer<'_, 'tcx> { } debug!("generalization failure in alias"); - Ok(self.infcx.next_ty_var_in_universe(self.span, self.for_universe)) + Ok(self.next_ty_var_for_alias()) } } }; diff --git a/tests/ui/traits/next-solver/generalize/bivariant-alias.rs b/tests/ui/traits/next-solver/generalize/bivariant-alias.rs new file mode 100644 index 0000000000000..b03d547838a01 --- /dev/null +++ b/tests/ui/traits/next-solver/generalize/bivariant-alias.rs @@ -0,0 +1,20 @@ +//@ revisions: old next +//@[next] compile-flags: -Znext-solver +//@ ignore-compare-mode-next-solver (explicit revisions) +//@ check-pass + +// When generalizing an alias in a bivariant context, we have to set +// `has_unconstrained_ty_var` as we may otherwise never check for +// well-formedness of the generalized type, causing us to error due +// to ambiguity. +trait Trait { + type Assoc; +} + +struct BivariantArg>(T); + +fn generalize(input: BivariantArg) { + let _generalized = input; +} + +pub fn main() {}