From d907a6b8ed33f0cc332992ef330e099c9474faba Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 31 Jan 2024 14:34:29 +0100 Subject: [PATCH 1/9] rustdoc: Correctly handle long crate names on mobile --- src/librustdoc/html/static/css/rustdoc.css | 7 +------ src/librustdoc/html/static/js/main.js | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 9c593aa85d987..899c4383986d4 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -1943,13 +1943,8 @@ in src-script.js and main.js pixels to avoid overflowing the topbar when the user sets a bigger font size. */ font-size: 24px; - } - - .mobile-topbar h2 a { - display: block; - text-overflow: ellipsis; - overflow: hidden; white-space: nowrap; + text-overflow: ellipsis; } .mobile-topbar .logo-container > img { diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index b46701b55ea7a..b26efb75ff657 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -45,7 +45,7 @@ function setMobileTopbar() { const mobileTitle = document.createElement("h2"); mobileTitle.className = "location"; if (hasClass(document.querySelector(".rustdoc"), "crate")) { - mobileTitle.innerText = `Crate ${window.currentCrate}`; + mobileTitle.innerHTML = `Crate ${window.currentCrate}`; } else if (locationTitle) { mobileTitle.innerHTML = locationTitle.innerHTML; } From 5d29f5a6de0cac97c69fd33d81e9063187102741 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 31 Jan 2024 14:35:03 +0100 Subject: [PATCH 2/9] Add regression test for #120471 to ensure that long crate name are handled as expected on mobile --- tests/rustdoc-gui/mobile-crate-name.goml | 22 +++++++++++++++++++ .../rustdoc-gui/type-declation-overflow.goml | 4 ++-- 2 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 tests/rustdoc-gui/mobile-crate-name.goml diff --git a/tests/rustdoc-gui/mobile-crate-name.goml b/tests/rustdoc-gui/mobile-crate-name.goml new file mode 100644 index 0000000000000..a0c96eec8a5ad --- /dev/null +++ b/tests/rustdoc-gui/mobile-crate-name.goml @@ -0,0 +1,22 @@ +// Checks that if the crate name is too long on mobile, it will not grow and overflow its parent +// (thanks to text overflow ellipsis). + +go-to: "file://" + |DOC_PATH| + "/test_docs/index.html" +// First we change the title to make it big. +set-window-size: (350, 800) +// We ensure that the "format" of the title is the same as the one we'll use. +assert-text: (".mobile-topbar .location a", "test_docs") +// We store the height we know is correct. +store-property: (".mobile-topbar .location", {"offsetHeight": height}) +// We change the crate name to something longer. +set-text: (".mobile-topbar .location a", "cargo_packager_resource_resolver") +// And we check that the size remained the same. +assert-property: (".mobile-topbar .location", {"offsetHeight": |height|}) + +// Now we check if it works for the non-crate pages as well. +go-to: "file://" + |DOC_PATH| + "/test_docs/struct.Foo.html" +// We store the height we know is correct. +store-property: (".mobile-topbar .location", {"offsetHeight": height}) +set-text: (".mobile-topbar .location a", "Something_incredibly_long_because") +// And we check that the size remained the same. +assert-property: (".mobile-topbar .location", {"offsetHeight": |height|}) diff --git a/tests/rustdoc-gui/type-declation-overflow.goml b/tests/rustdoc-gui/type-declation-overflow.goml index 5780f5c88f82c..a97cc98897ae2 100644 --- a/tests/rustdoc-gui/type-declation-overflow.goml +++ b/tests/rustdoc-gui/type-declation-overflow.goml @@ -39,8 +39,8 @@ assert-property: ("pre.item-decl", {"scrollWidth": "950"}) set-window-size: (600, 600) go-to: "file://" + |DOC_PATH| + "/lib2/too_long/struct.SuperIncrediblyLongLongLongLongLongLongLongGigaGigaGigaMegaLongLongLongStructName.html" // It shouldn't have an overflow in the topbar either. -store-property: (".mobile-topbar h2", {"scrollWidth": scrollWidth}) -assert-property: (".mobile-topbar h2", {"clientWidth": |scrollWidth|}) +store-property: (".mobile-topbar", {"scrollWidth": scrollWidth}) +assert-property: (".mobile-topbar", {"clientWidth": |scrollWidth|}) assert-css: (".mobile-topbar h2", {"overflow-x": "hidden"}) // Check wrapping for top main-heading h1 and out-of-band. From 9b3fcf9ad44faa6362be348ef233bbc81d128308 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 14 Feb 2024 17:48:40 +0000 Subject: [PATCH 3/9] Detect when method call on argument could be removed to fulfill failed trait bound When encountering ```rust struct Foo; struct Bar; impl From for Foo { fn from(_: Bar) -> Self { Foo } } fn qux(_: impl From) {} fn main() { qux(Bar.into()); } ``` Suggest removing `.into()`: ``` error[E0283]: type annotations needed --> f100.rs:8:13 | 8 | qux(Bar.into()); | --- ^^^^ | | | required by a bound introduced by this call | = note: cannot satisfy `_: From` note: required by a bound in `qux` --> f100.rs:6:16 | 6 | fn qux(_: impl From) {} | ^^^^^^^^^ required by this bound in `qux` help: try using a fully qualified path to specify the expected types | 8 | qux(>::into(Bar)); | +++++++++++++++++++++++ ~ help: consider removing this method call, as the receiver has type `Bar` and `Bar: From` can be fulfilled | 8 - qux(Bar.into()); 8 + qux(Bar); | ``` Fix #71252 --- .../src/traits/error_reporting/suggestions.rs | 20 +++++++++++++- tests/ui/async-await/issue-72442.stderr | 5 ++++ tests/ui/error-should-say-copy-not-pod.stderr | 5 ++++ .../suggestions/issue-84973-blacklist.stderr | 5 ++++ .../argument-with-unnecessary-method-call.rs | 11 ++++++++ ...gument-with-unnecessary-method-call.stderr | 27 +++++++++++++++++++ 6 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 tests/ui/trait-bounds/argument-with-unnecessary-method-call.rs create mode 100644 tests/ui/trait-bounds/argument-with-unnecessary-method-call.stderr diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 335e6ff28226e..00672af52eada 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -3961,6 +3961,24 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { if let Node::Expr(expr) = tcx.hir_node(arg_hir_id) && let Some(typeck_results) = &self.typeck_results { + if let hir::Expr { kind: hir::ExprKind::MethodCall(_, rcvr, _, _), .. } = expr + && let Some(ty) = typeck_results.node_type_opt(rcvr.hir_id) + && let Some(failed_pred) = failed_pred.to_opt_poly_trait_pred() + && let pred = failed_pred.map_bound(|pred| pred.with_self_ty(tcx, ty)) + && self.predicate_must_hold_modulo_regions(&Obligation::misc( + tcx, expr.span, body_id, param_env, pred, + )) + { + err.span_suggestion_verbose( + expr.span.with_lo(rcvr.span.hi()), + format!( + "consider removing this method call, as the receiver has type `{ty}` and \ + `{pred}` trivially holds", + ), + "", + Applicability::MaybeIncorrect, + ); + } if let hir::Expr { kind: hir::ExprKind::Block(block, _), .. } = expr { let inner_expr = expr.peel_blocks(); let ty = typeck_results @@ -4096,7 +4114,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } } - if let Node::Expr(expr) = tcx.hir_node(call_hir_id) { + if let Node::Expr(expr) = call_node { if let hir::ExprKind::Call(hir::Expr { span, .. }, _) | hir::ExprKind::MethodCall( hir::PathSegment { ident: Ident { span, .. }, .. }, diff --git a/tests/ui/async-await/issue-72442.stderr b/tests/ui/async-await/issue-72442.stderr index 313f6079c7c16..e68f7a299908f 100644 --- a/tests/ui/async-await/issue-72442.stderr +++ b/tests/ui/async-await/issue-72442.stderr @@ -8,6 +8,11 @@ LL | let mut f = File::open(path.to_str())?; | note: required by a bound in `File::open` --> $SRC_DIR/std/src/fs.rs:LL:COL +help: consider removing this method call, as the receiver has type `&Path` and `&Path: AsRef` trivially holds + | +LL - let mut f = File::open(path.to_str())?; +LL + let mut f = File::open(path)?; + | error: aborting due to 1 previous error diff --git a/tests/ui/error-should-say-copy-not-pod.stderr b/tests/ui/error-should-say-copy-not-pod.stderr index 658584e2ff4bb..6aa129fa29b53 100644 --- a/tests/ui/error-should-say-copy-not-pod.stderr +++ b/tests/ui/error-should-say-copy-not-pod.stderr @@ -11,6 +11,11 @@ note: required by a bound in `check_bound` | LL | fn check_bound(_: T) {} | ^^^^ required by this bound in `check_bound` +help: consider removing this method call, as the receiver has type `&'static str` and `&'static str: Copy` trivially holds + | +LL - check_bound("nocopy".to_string()); +LL + check_bound("nocopy"); + | error: aborting due to 1 previous error diff --git a/tests/ui/suggestions/issue-84973-blacklist.stderr b/tests/ui/suggestions/issue-84973-blacklist.stderr index 8e980997089e6..3333822832856 100644 --- a/tests/ui/suggestions/issue-84973-blacklist.stderr +++ b/tests/ui/suggestions/issue-84973-blacklist.stderr @@ -11,6 +11,11 @@ note: required by a bound in `f_copy` | LL | fn f_copy(t: T) {} | ^^^^ required by this bound in `f_copy` +help: consider removing this method call, as the receiver has type `&'static str` and `&'static str: Copy` trivially holds + | +LL - f_copy("".to_string()); +LL + f_copy(""); + | error[E0277]: the trait bound `S: Clone` is not satisfied --> $DIR/issue-84973-blacklist.rs:16:13 diff --git a/tests/ui/trait-bounds/argument-with-unnecessary-method-call.rs b/tests/ui/trait-bounds/argument-with-unnecessary-method-call.rs new file mode 100644 index 0000000000000..d8fd1d44a9858 --- /dev/null +++ b/tests/ui/trait-bounds/argument-with-unnecessary-method-call.rs @@ -0,0 +1,11 @@ +struct Foo; +struct Bar; +impl From for Foo { + fn from(_: Bar) -> Self { Foo } +} +fn qux(_: impl From) {} +fn main() { + qux(Bar.into()); //~ ERROR type annotations needed + //~| HELP try using a fully qualified path to specify the expected types + //~| HELP consider removing this method call, as the receiver has type `Bar` and `Bar: From` trivially holds +} diff --git a/tests/ui/trait-bounds/argument-with-unnecessary-method-call.stderr b/tests/ui/trait-bounds/argument-with-unnecessary-method-call.stderr new file mode 100644 index 0000000000000..49230c98a12c1 --- /dev/null +++ b/tests/ui/trait-bounds/argument-with-unnecessary-method-call.stderr @@ -0,0 +1,27 @@ +error[E0283]: type annotations needed + --> $DIR/argument-with-unnecessary-method-call.rs:8:13 + | +LL | qux(Bar.into()); + | --- ^^^^ + | | + | required by a bound introduced by this call + | + = note: cannot satisfy `_: From` +note: required by a bound in `qux` + --> $DIR/argument-with-unnecessary-method-call.rs:6:16 + | +LL | fn qux(_: impl From) {} + | ^^^^^^^^^ required by this bound in `qux` +help: try using a fully qualified path to specify the expected types + | +LL | qux(>::into(Bar)); + | +++++++++++++++++++++++ ~ +help: consider removing this method call, as the receiver has type `Bar` and `Bar: From` trivially holds + | +LL - qux(Bar.into()); +LL + qux(Bar); + | + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0283`. From 5fbb1b2f4dd858f21da7ba56f9e5011b43a0af4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Thu, 15 Feb 2024 20:41:20 +0100 Subject: [PATCH 4/9] rustdoc: fix and refactor HTML rendering a bit --- src/librustdoc/html/format.rs | 454 +++++++----------- ...{bounds-in-multiple-parts.rs => bounds.rs} | 12 + .../const-generics/generic_const_exprs.rs | 2 +- 3 files changed, 186 insertions(+), 282 deletions(-) rename tests/rustdoc/{bounds-in-multiple-parts.rs => bounds.rs} (54%) diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 4ba1665bdc9f1..bb68c84f529a7 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -1,6 +1,6 @@ //! HTML formatting module //! -//! This module contains a large number of `fmt::Display` implementations for +//! This module contains a large number of `Display` implementations for //! various types in `rustdoc::clean`. //! //! These implementations all emit HTML. As an internal implementation detail, @@ -9,7 +9,7 @@ use std::borrow::Cow; use std::cell::Cell; -use std::fmt::{self, Write}; +use std::fmt::{self, Display, Write}; use std::iter::{self, once}; use rustc_ast as ast; @@ -150,16 +150,16 @@ impl Buffer { } } -pub(crate) fn comma_sep( +pub(crate) fn comma_sep( items: impl Iterator, space_after_comma: bool, -) -> impl fmt::Display { +) -> impl Display { display_fn(move |f| { for (i, item) in items.enumerate() { if i != 0 { write!(f, ",{}", if space_after_comma { " " } else { "" })?; } - fmt::Display::fmt(&item, f)?; + item.fmt(f)?; } Ok(()) }) @@ -168,7 +168,7 @@ pub(crate) fn comma_sep( pub(crate) fn print_generic_bounds<'a, 'tcx: 'a>( bounds: &'a [clean::GenericBound], cx: &'a Context<'tcx>, -) -> impl fmt::Display + 'a + Captures<'tcx> { +) -> impl Display + 'a + Captures<'tcx> { display_fn(move |f| { let mut bounds_dup = FxHashSet::default(); @@ -176,7 +176,7 @@ pub(crate) fn print_generic_bounds<'a, 'tcx: 'a>( if i > 0 { f.write_str(" + ")?; } - fmt::Display::fmt(&bound.print(cx), f)?; + bound.print(cx).fmt(f)?; } Ok(()) }) @@ -186,7 +186,7 @@ impl clean::GenericParamDef { pub(crate) fn print<'a, 'tcx: 'a>( &'a self, cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'a + Captures<'tcx> { + ) -> impl Display + 'a + Captures<'tcx> { display_fn(move |f| match &self.kind { clean::GenericParamDefKind::Lifetime { outlives } => { write!(f, "{}", self.name)?; @@ -207,35 +207,27 @@ impl clean::GenericParamDef { f.write_str(self.name.as_str())?; if !bounds.is_empty() { - if f.alternate() { - write!(f, ": {:#}", print_generic_bounds(bounds, cx))?; - } else { - write!(f, ": {}", print_generic_bounds(bounds, cx))?; - } + f.write_str(": ")?; + print_generic_bounds(bounds, cx).fmt(f)?; } if let Some(ref ty) = default { - if f.alternate() { - write!(f, " = {:#}", ty.print(cx))?; - } else { - write!(f, " = {}", ty.print(cx))?; - } + f.write_str(" = ")?; + ty.print(cx).fmt(f)?; } Ok(()) } clean::GenericParamDefKind::Const { ty, default, .. } => { - if f.alternate() { - write!(f, "const {}: {:#}", self.name, ty.print(cx))?; - } else { - write!(f, "const {}: {}", self.name, ty.print(cx))?; - } + write!(f, "const {}: ", self.name)?; + ty.print(cx).fmt(f)?; if let Some(default) = default { + f.write_str(" = ")?; if f.alternate() { - write!(f, " = {default:#}")?; + write!(f, "{default}")?; } else { - write!(f, " = {default}")?; + write!(f, "{}", Escape(default))?; } } @@ -249,7 +241,7 @@ impl clean::Generics { pub(crate) fn print<'a, 'tcx: 'a>( &'a self, cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'a + Captures<'tcx> { + ) -> impl Display + 'a + Captures<'tcx> { display_fn(move |f| { let mut real_params = self.params.iter().filter(|p| !p.is_synthetic_param()).peekable(); if real_params.peek().is_none() { @@ -279,63 +271,50 @@ pub(crate) fn print_where_clause<'a, 'tcx: 'a>( cx: &'a Context<'tcx>, indent: usize, ending: Ending, -) -> impl fmt::Display + 'a + Captures<'tcx> { +) -> impl Display + 'a + Captures<'tcx> { display_fn(move |f| { - let mut where_predicates = gens.where_predicates.iter().filter(|pred| { - !matches!(pred, clean::WherePredicate::BoundPredicate { bounds, .. } if bounds.is_empty()) - }).map(|pred| { - display_fn(move |f| { - if f.alternate() { - f.write_str(" ")?; - } else { - f.write_str("\n")?; - } - - match pred { - clean::WherePredicate::BoundPredicate { ty, bounds, bound_params } => { - let ty_cx = ty.print(cx); - let generic_bounds = print_generic_bounds(bounds, cx); + let mut where_predicates = gens + .where_predicates + .iter() + .map(|pred| { + display_fn(move |f| { + if f.alternate() { + f.write_str(" ")?; + } else { + f.write_str("\n")?; + } - if bound_params.is_empty() { - if f.alternate() { - write!(f, "{ty_cx:#}: {generic_bounds:#}") - } else { - write!(f, "{ty_cx}: {generic_bounds}") + match pred { + clean::WherePredicate::BoundPredicate { ty, bounds, bound_params } => { + print_higher_ranked_params_with_space(bound_params, cx).fmt(f)?; + ty.print(cx).fmt(f)?; + f.write_str(":")?; + if !bounds.is_empty() { + f.write_str(" ")?; + print_generic_bounds(bounds, cx).fmt(f)?; } - } else { + Ok(()) + } + clean::WherePredicate::RegionPredicate { lifetime, bounds } => { + // We don't need to check `alternate` since we can be certain that neither + // the lifetime nor the bounds contain any characters which need escaping. + write!(f, "{}:", lifetime.print())?; + if !bounds.is_empty() { + write!(f, " {}", print_generic_bounds(bounds, cx))?; + } + Ok(()) + } + clean::WherePredicate::EqPredicate { lhs, rhs } => { if f.alternate() { - write!( - f, - "for<{:#}> {ty_cx:#}: {generic_bounds:#}", - comma_sep(bound_params.iter().map(|lt| lt.print(cx)), true) - ) + write!(f, "{:#} == {:#}", lhs.print(cx), rhs.print(cx)) } else { - write!( - f, - "for<{}> {ty_cx}: {generic_bounds}", - comma_sep(bound_params.iter().map(|lt| lt.print(cx)), true) - ) + write!(f, "{} == {}", lhs.print(cx), rhs.print(cx)) } } } - clean::WherePredicate::RegionPredicate { lifetime, bounds } => { - let mut bounds_display = String::new(); - for bound in bounds.iter().map(|b| b.print(cx)) { - write!(bounds_display, "{bound} + ")?; - } - bounds_display.truncate(bounds_display.len() - " + ".len()); - write!(f, "{}: {bounds_display}", lifetime.print()) - } - clean::WherePredicate::EqPredicate { lhs, rhs } => { - if f.alternate() { - write!(f, "{:#} == {:#}", lhs.print(cx), rhs.print(cx)) - } else { - write!(f, "{} == {}", lhs.print(cx), rhs.print(cx)) - } - } - } + }) }) - }).peekable(); + .peekable(); if where_predicates.peek().is_none() { return Ok(()); @@ -392,13 +371,13 @@ pub(crate) fn print_where_clause<'a, 'tcx: 'a>( } impl clean::Lifetime { - pub(crate) fn print(&self) -> impl fmt::Display + '_ { + pub(crate) fn print(&self) -> impl Display + '_ { self.0.as_str() } } impl clean::Constant { - pub(crate) fn print(&self, tcx: TyCtxt<'_>) -> impl fmt::Display + '_ { + pub(crate) fn print(&self, tcx: TyCtxt<'_>) -> impl Display + '_ { let expr = self.expr(tcx); display_fn( move |f| { @@ -409,31 +388,10 @@ impl clean::Constant { } impl clean::PolyTrait { - fn print<'a, 'tcx: 'a>( - &'a self, - cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'a + Captures<'tcx> { + fn print<'a, 'tcx: 'a>(&'a self, cx: &'a Context<'tcx>) -> impl Display + 'a + Captures<'tcx> { display_fn(move |f| { - if !self.generic_params.is_empty() { - if f.alternate() { - write!( - f, - "for<{:#}> ", - comma_sep(self.generic_params.iter().map(|g| g.print(cx)), true) - )?; - } else { - write!( - f, - "for<{}> ", - comma_sep(self.generic_params.iter().map(|g| g.print(cx)), true) - )?; - } - } - if f.alternate() { - write!(f, "{:#}", self.trait_.print(cx)) - } else { - write!(f, "{}", self.trait_.print(cx)) - } + print_higher_ranked_params_with_space(&self.generic_params, cx).fmt(f)?; + self.trait_.print(cx).fmt(f) }) } } @@ -442,32 +400,25 @@ impl clean::GenericBound { pub(crate) fn print<'a, 'tcx: 'a>( &'a self, cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'a + Captures<'tcx> { + ) -> impl Display + 'a + Captures<'tcx> { display_fn(move |f| match self { clean::GenericBound::Outlives(lt) => write!(f, "{}", lt.print()), clean::GenericBound::TraitBound(ty, modifier) => { - let modifier_str = match modifier { + f.write_str(match modifier { hir::TraitBoundModifier::None => "", hir::TraitBoundModifier::Maybe => "?", hir::TraitBoundModifier::Negative => "!", // `const` and `~const` trait bounds are experimental; don't render them. hir::TraitBoundModifier::Const | hir::TraitBoundModifier::MaybeConst => "", - }; - if f.alternate() { - write!(f, "{modifier_str}{ty:#}", ty = ty.print(cx)) - } else { - write!(f, "{modifier_str}{ty}", ty = ty.print(cx)) - } + })?; + ty.print(cx).fmt(f) } }) } } impl clean::GenericArgs { - fn print<'a, 'tcx: 'a>( - &'a self, - cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'a + Captures<'tcx> { + fn print<'a, 'tcx: 'a>(&'a self, cx: &'a Context<'tcx>) -> impl Display + 'a + Captures<'tcx> { display_fn(move |f| { match self { clean::GenericArgs::AngleBracketed { args, bindings } => { @@ -515,11 +466,7 @@ impl clean::GenericArgs { f.write_str(", ")?; } comma = true; - if f.alternate() { - write!(f, "{:#}", ty.print(cx))?; - } else { - write!(f, "{}", ty.print(cx))?; - } + ty.print(cx).fmt(f)?; } f.write_str(")")?; if let Some(ref ty) = *output { @@ -973,31 +920,43 @@ fn primitive_link_fragment( None => {} } } - std::fmt::Display::fmt(&name, f)?; + Display::fmt(&name, f)?; if needs_termination { write!(f, "")?; } Ok(()) } -/// Helper to render type parameters fn tybounds<'a, 'tcx: 'a>( bounds: &'a [clean::PolyTrait], lt: &'a Option, cx: &'a Context<'tcx>, -) -> impl fmt::Display + 'a + Captures<'tcx> { +) -> impl Display + 'a + Captures<'tcx> { display_fn(move |f| { for (i, bound) in bounds.iter().enumerate() { if i > 0 { write!(f, " + ")?; } - - fmt::Display::fmt(&bound.print(cx), f)?; + bound.print(cx).fmt(f)?; } - if let Some(lt) = lt { - write!(f, " + ")?; - fmt::Display::fmt(<.print(), f)?; + // We don't need to check `alternate` since we can be certain that + // the lifetime doesn't contain any characters which need escaping. + write!(f, " + {}", lt.print())?; + } + Ok(()) + }) +} + +fn print_higher_ranked_params_with_space<'a, 'tcx: 'a>( + params: &'a [clean::GenericParamDef], + cx: &'a Context<'tcx>, +) -> impl Display + 'a + Captures<'tcx> { + display_fn(move |f| { + if !params.is_empty() { + f.write_str(if f.alternate() { "for<" } else { "for<" })?; + comma_sep(params.iter().map(|lt| lt.print(cx)), true).fmt(f)?; + f.write_str(if f.alternate() { "> " } else { "> " })?; } Ok(()) }) @@ -1007,7 +966,7 @@ pub(crate) fn anchor<'a, 'cx: 'a>( did: DefId, text: Symbol, cx: &'cx Context<'_>, -) -> impl fmt::Display + 'a { +) -> impl Display + 'a { let parts = href(did, cx); display_fn(move |f| { if let Ok((url, short_ty, fqp)) = parts { @@ -1039,7 +998,7 @@ fn fmt_type<'cx>( } clean::DynTrait(ref bounds, ref lt) => { f.write_str("dyn ")?; - fmt::Display::fmt(&tybounds(bounds, lt, cx), f) + tybounds(bounds, lt, cx).fmt(f) } clean::Infer => write!(f, "_"), clean::Primitive(clean::PrimitiveType::Never) => { @@ -1049,80 +1008,62 @@ fn fmt_type<'cx>( primitive_link(f, prim, format_args!("{}", prim.as_sym().as_str()), cx) } clean::BareFunction(ref decl) => { + print_higher_ranked_params_with_space(&decl.generic_params, cx).fmt(f)?; + decl.unsafety.print_with_space().fmt(f)?; + print_abi_with_space(decl.abi).fmt(f)?; if f.alternate() { - write!( - f, - "{:#}{}{:#}fn{:#}", - decl.print_hrtb_with_space(cx), - decl.unsafety.print_with_space(), - print_abi_with_space(decl.abi), - decl.decl.print(cx), - ) + f.write_str("fn")?; } else { - write!( - f, - "{}{}{}", - decl.print_hrtb_with_space(cx), - decl.unsafety.print_with_space(), - print_abi_with_space(decl.abi) - )?; primitive_link(f, PrimitiveType::Fn, format_args!("fn"), cx)?; - write!(f, "{}", decl.decl.print(cx)) } + decl.decl.print(cx).fmt(f) } - clean::Tuple(ref typs) => { - match &typs[..] { - &[] => primitive_link(f, PrimitiveType::Unit, format_args!("()"), cx), - [one] => { - if let clean::Generic(name) = one { - primitive_link(f, PrimitiveType::Tuple, format_args!("({name},)"), cx) - } else { - write!(f, "(")?; - // Carry `f.alternate()` into this display w/o branching manually. - fmt::Display::fmt(&one.print(cx), f)?; - write!(f, ",)") - } + clean::Tuple(ref typs) => match &typs[..] { + &[] => primitive_link(f, PrimitiveType::Unit, format_args!("()"), cx), + [one] => { + if let clean::Generic(name) = one { + primitive_link(f, PrimitiveType::Tuple, format_args!("({name},)"), cx) + } else { + write!(f, "(")?; + one.print(cx).fmt(f)?; + write!(f, ",)") } - many => { - let generic_names: Vec = many - .iter() - .filter_map(|t| match t { - clean::Generic(name) => Some(*name), - _ => None, - }) - .collect(); - let is_generic = generic_names.len() == many.len(); - if is_generic { - primitive_link( - f, - PrimitiveType::Tuple, - format_args!( - "({})", - generic_names.iter().map(|s| s.as_str()).join(", ") - ), - cx, - ) - } else { - write!(f, "(")?; - for (i, item) in many.iter().enumerate() { - if i != 0 { - write!(f, ", ")?; - } - // Carry `f.alternate()` into this display w/o branching manually. - fmt::Display::fmt(&item.print(cx), f)?; + } + many => { + let generic_names: Vec = many + .iter() + .filter_map(|t| match t { + clean::Generic(name) => Some(*name), + _ => None, + }) + .collect(); + let is_generic = generic_names.len() == many.len(); + if is_generic { + primitive_link( + f, + PrimitiveType::Tuple, + format_args!("({})", generic_names.iter().map(|s| s.as_str()).join(", ")), + cx, + ) + } else { + write!(f, "(")?; + for (i, item) in many.iter().enumerate() { + if i != 0 { + write!(f, ", ")?; } - write!(f, ")") + item.print(cx).fmt(f)?; } + write!(f, ")") } } - } + }, clean::Slice(ref t) => match **t { clean::Generic(name) => { primitive_link(f, PrimitiveType::Slice, format_args!("[{name}]"), cx) } _ => { write!(f, "[")?; - fmt::Display::fmt(&t.print(cx), f)?; + t.print(cx).fmt(f)?; write!(f, "]") } }, @@ -1135,7 +1076,7 @@ fn fmt_type<'cx>( ), _ => { write!(f, "[")?; - fmt::Display::fmt(&t.print(cx), f)?; + t.print(cx).fmt(f)?; if f.alternate() { write!(f, "; {n}")?; } else { @@ -1175,7 +1116,7 @@ fn fmt_type<'cx>( } } else { primitive_link(f, clean::PrimitiveType::RawPointer, format_args!("*{m} "), cx)?; - fmt::Display::fmt(&t.print(cx), f) + t.print(cx).fmt(f) } } clean::BorrowedRef { lifetime: ref l, mutability, type_: ref ty } => { @@ -1216,11 +1157,8 @@ fn fmt_type<'cx>( Ok(()) } clean::ImplTrait(ref bounds) => { - if f.alternate() { - write!(f, "impl {:#}", print_generic_bounds(bounds, cx)) - } else { - write!(f, "impl {}", print_generic_bounds(bounds, cx)) - } + f.write_str("impl ")?; + print_generic_bounds(bounds, cx).fmt(f) } clean::QPath(box clean::QPathData { ref assoc, @@ -1292,8 +1230,7 @@ fn fmt_type<'cx>( write!(f, "{}", assoc.name) }?; - // Carry `f.alternate()` into this display w/o branching manually. - fmt::Display::fmt(&assoc.args.print(cx), f) + assoc.args.print(cx).fmt(f) } } } @@ -1302,7 +1239,7 @@ impl clean::Type { pub(crate) fn print<'b, 'a: 'b, 'tcx: 'a>( &'a self, cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'b + Captures<'tcx> { + ) -> impl Display + 'b + Captures<'tcx> { display_fn(move |f| fmt_type(self, f, false, cx)) } } @@ -1311,7 +1248,7 @@ impl clean::Path { pub(crate) fn print<'b, 'a: 'b, 'tcx: 'a>( &'a self, cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'b + Captures<'tcx> { + ) -> impl Display + 'b + Captures<'tcx> { display_fn(move |f| resolved_path(f, self.def_id(), self, false, false, cx)) } } @@ -1321,20 +1258,18 @@ impl clean::Impl { &'a self, use_absolute: bool, cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'a + Captures<'tcx> { + ) -> impl Display + 'a + Captures<'tcx> { display_fn(move |f| { - if f.alternate() { - write!(f, "impl{:#} ", self.generics.print(cx))?; - } else { - write!(f, "impl{} ", self.generics.print(cx))?; - } + f.write_str("impl")?; + self.generics.print(cx).fmt(f)?; + f.write_str(" ")?; if let Some(ref ty) = self.trait_ { match self.polarity { ty::ImplPolarity::Positive | ty::ImplPolarity::Reservation => {} ty::ImplPolarity::Negative => write!(f, "!")?, } - fmt::Display::fmt(&ty.print(cx), f)?; + ty.print(cx).fmt(f)?; write!(f, " for ")?; } @@ -1359,14 +1294,9 @@ impl clean::Impl { // Hardcoded anchor library/core/src/primitive_docs.rs // Link should match `# Trait implementations` - let hrtb = bare_fn.print_hrtb_with_space(cx); - let unsafety = bare_fn.unsafety.print_with_space(); - let abi = print_abi_with_space(bare_fn.abi); - if f.alternate() { - write!(f, "{hrtb:#}{unsafety}{abi:#}",)?; - } else { - write!(f, "{hrtb}{unsafety}{abi}",)?; - } + print_higher_ranked_params_with_space(&bare_fn.generic_params, cx).fmt(f)?; + bare_fn.unsafety.print_with_space().fmt(f)?; + print_abi_with_space(bare_fn.abi).fmt(f)?; let ellipsis = if bare_fn.decl.c_variadic { ", ..." } else { "" }; primitive_link_fragment( f, @@ -1386,8 +1316,7 @@ impl clean::Impl { fmt_type(&self.for_, f, use_absolute, cx)?; } - fmt::Display::fmt(&print_where_clause(&self.generics, cx, 0, Ending::Newline), f)?; - Ok(()) + print_where_clause(&self.generics, cx, 0, Ending::Newline).fmt(f) }) } } @@ -1396,16 +1325,11 @@ impl clean::Arguments { pub(crate) fn print<'a, 'tcx: 'a>( &'a self, cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'a + Captures<'tcx> { + ) -> impl Display + 'a + Captures<'tcx> { display_fn(move |f| { for (i, input) in self.values.iter().enumerate() { write!(f, "{}: ", input.name)?; - - if f.alternate() { - write!(f, "{:#}", input.type_.print(cx))?; - } else { - write!(f, "{}", input.type_.print(cx))?; - } + input.type_.print(cx).fmt(f)?; if i + 1 < self.values.len() { write!(f, ", ")?; } @@ -1415,25 +1339,6 @@ impl clean::Arguments { } } -impl clean::BareFunctionDecl { - fn print_hrtb_with_space<'a, 'tcx: 'a>( - &'a self, - cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'a + Captures<'tcx> { - display_fn(move |f| { - if !self.generic_params.is_empty() { - write!( - f, - "for<{}> ", - comma_sep(self.generic_params.iter().map(|g| g.print(cx)), true) - ) - } else { - Ok(()) - } - }) - } -} - // Implements Write but only counts the bytes "written". struct WriteCounter(usize); @@ -1447,7 +1352,7 @@ impl std::fmt::Write for WriteCounter { // Implements Display by emitting the given number of spaces. struct Indent(usize); -impl fmt::Display for Indent { +impl Display for Indent { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { (0..self.0).for_each(|_| { f.write_char(' ').unwrap(); @@ -1460,7 +1365,7 @@ impl clean::FnDecl { pub(crate) fn print<'b, 'a: 'b, 'tcx: 'a>( &'a self, cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'b + Captures<'tcx> { + ) -> impl Display + 'b + Captures<'tcx> { display_fn(move |f| { let ellipsis = if self.c_variadic { ", ..." } else { "" }; if f.alternate() { @@ -1494,7 +1399,7 @@ impl clean::FnDecl { header_len: usize, indent: usize, cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'a + Captures<'tcx> { + ) -> impl Display + 'a + Captures<'tcx> { display_fn(move |f| { // First, generate the text form of the declaration, with no line wrapping, and count the bytes. let mut counter = WriteCounter(0); @@ -1554,7 +1459,7 @@ impl clean::FnDecl { } clean::SelfExplicit(ref typ) => { write!(f, "self: ")?; - fmt::Display::fmt(&typ.print(cx), f)?; + typ.print(cx).fmt(f)?; } } } else { @@ -1562,7 +1467,7 @@ impl clean::FnDecl { write!(f, "const ")?; } write!(f, "{}: ", input.name)?; - fmt::Display::fmt(&input.type_.print(cx), f)?; + input.type_.print(cx).fmt(f)?; } } @@ -1578,14 +1483,13 @@ impl clean::FnDecl { Some(n) => write!(f, "\n{})", Indent(n))?, }; - fmt::Display::fmt(&self.print_output(cx), f)?; - Ok(()) + self.print_output(cx).fmt(f) } fn print_output<'a, 'tcx: 'a>( &'a self, cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'a + Captures<'tcx> { + ) -> impl Display + 'a + Captures<'tcx> { display_fn(move |f| match &self.output { clean::Tuple(tys) if tys.is_empty() => Ok(()), ty if f.alternate() => { @@ -1600,7 +1504,7 @@ pub(crate) fn visibility_print_with_space<'a, 'tcx: 'a>( visibility: Option>, item_did: ItemId, cx: &'a Context<'tcx>, -) -> impl fmt::Display + 'a + Captures<'tcx> { +) -> impl Display + 'a + Captures<'tcx> { use std::fmt::Write as _; let to_print: Cow<'static, str> = match visibility { @@ -1648,7 +1552,7 @@ pub(crate) fn visibility_to_src_with_space<'a, 'tcx: 'a>( visibility: Option>, tcx: TyCtxt<'tcx>, item_did: DefId, -) -> impl fmt::Display + 'a + Captures<'tcx> { +) -> impl Display + 'a + Captures<'tcx> { let to_print: Cow<'static, str> = match visibility { None => "".into(), Some(ty::Visibility::Public) => "pub ".into(), @@ -1727,7 +1631,7 @@ impl clean::Import { pub(crate) fn print<'a, 'tcx: 'a>( &'a self, cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'a + Captures<'tcx> { + ) -> impl Display + 'a + Captures<'tcx> { display_fn(move |f| match self.kind { clean::ImportKind::Simple(name) => { if name == self.source.path.last() { @@ -1751,7 +1655,7 @@ impl clean::ImportSource { pub(crate) fn print<'a, 'tcx: 'a>( &'a self, cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'a + Captures<'tcx> { + ) -> impl Display + 'a + Captures<'tcx> { display_fn(move |f| match self.did { Some(did) => resolved_path(f, did, &self.path, true, false, cx), _ => { @@ -1779,29 +1683,19 @@ impl clean::TypeBinding { pub(crate) fn print<'a, 'tcx: 'a>( &'a self, cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'a + Captures<'tcx> { + ) -> impl Display + 'a + Captures<'tcx> { display_fn(move |f| { f.write_str(self.assoc.name.as_str())?; - if f.alternate() { - write!(f, "{:#}", self.assoc.args.print(cx))?; - } else { - write!(f, "{}", self.assoc.args.print(cx))?; - } + self.assoc.args.print(cx).fmt(f)?; match self.kind { clean::TypeBindingKind::Equality { ref term } => { - if f.alternate() { - write!(f, " = {:#}", term.print(cx))?; - } else { - write!(f, " = {}", term.print(cx))?; - } + f.write_str(" = ")?; + term.print(cx).fmt(f)?; } clean::TypeBindingKind::Constraint { ref bounds } => { if !bounds.is_empty() { - if f.alternate() { - write!(f, ": {:#}", print_generic_bounds(bounds, cx))?; - } else { - write!(f, ": {}", print_generic_bounds(bounds, cx))?; - } + f.write_str(": ")?; + print_generic_bounds(bounds, cx).fmt(f)?; } } } @@ -1810,7 +1704,7 @@ impl clean::TypeBinding { } } -pub(crate) fn print_abi_with_space(abi: Abi) -> impl fmt::Display { +pub(crate) fn print_abi_with_space(abi: Abi) -> impl Display { display_fn(move |f| { let quot = if f.alternate() { "\"" } else { """ }; match abi { @@ -1828,34 +1722,32 @@ impl clean::GenericArg { pub(crate) fn print<'a, 'tcx: 'a>( &'a self, cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'a + Captures<'tcx> { + ) -> impl Display + 'a + Captures<'tcx> { display_fn(move |f| match self { - clean::GenericArg::Lifetime(lt) => fmt::Display::fmt(<.print(), f), - clean::GenericArg::Type(ty) => fmt::Display::fmt(&ty.print(cx), f), - clean::GenericArg::Const(ct) => fmt::Display::fmt(&ct.print(cx.tcx()), f), - clean::GenericArg::Infer => fmt::Display::fmt("_", f), + clean::GenericArg::Lifetime(lt) => lt.print().fmt(f), + clean::GenericArg::Type(ty) => ty.print(cx).fmt(f), + clean::GenericArg::Const(ct) => ct.print(cx.tcx()).fmt(f), + clean::GenericArg::Infer => Display::fmt("_", f), }) } } -impl clean::types::Term { +impl clean::Term { pub(crate) fn print<'a, 'tcx: 'a>( &'a self, cx: &'a Context<'tcx>, - ) -> impl fmt::Display + 'a + Captures<'tcx> { + ) -> impl Display + 'a + Captures<'tcx> { display_fn(move |f| match self { - clean::types::Term::Type(ty) => fmt::Display::fmt(&ty.print(cx), f), - clean::types::Term::Constant(ct) => fmt::Display::fmt(&ct.print(cx.tcx()), f), + clean::Term::Type(ty) => ty.print(cx).fmt(f), + clean::Term::Constant(ct) => ct.print(cx.tcx()).fmt(f), }) } } -pub(crate) fn display_fn( - f: impl FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result, -) -> impl fmt::Display { +pub(crate) fn display_fn(f: impl FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result) -> impl Display { struct WithFormatter(Cell>); - impl fmt::Display for WithFormatter + impl Display for WithFormatter where F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result, { diff --git a/tests/rustdoc/bounds-in-multiple-parts.rs b/tests/rustdoc/bounds.rs similarity index 54% rename from tests/rustdoc/bounds-in-multiple-parts.rs rename to tests/rustdoc/bounds.rs index 279e3c148887e..da09e3f2a528f 100644 --- a/tests/rustdoc/bounds-in-multiple-parts.rs +++ b/tests/rustdoc/bounds.rs @@ -18,3 +18,15 @@ pub trait T2 { fn f() where Self: Eq, Self: Eq2, T: Eq2; } + +// Checking that we support empty bounds (we used to crash on empty outlives-bounds). +// Note that we don't want to hide them since they have a semantic effect. +// For outlives-bounds, they force the lifetime param to be early-bound instead of late-bound. +// For trait bounds, it can affect well-formedness (see `ClauseKind::WellFormed`). +// @has 'foo/fn.empty.html' +// @has - '//pre[@class="rust item-decl"]' "empty<'a, T>()where T:, 'a:," +pub fn empty<'a, T>() + where + T:, + 'a:, +{} diff --git a/tests/rustdoc/const-generics/generic_const_exprs.rs b/tests/rustdoc/const-generics/generic_const_exprs.rs index e23b3006da6cd..2d2d31d7231a7 100644 --- a/tests/rustdoc/const-generics/generic_const_exprs.rs +++ b/tests/rustdoc/const-generics/generic_const_exprs.rs @@ -3,5 +3,5 @@ #![allow(incomplete_features)] // make sure that `ConstEvaluatable` predicates dont cause rustdoc to ICE #77647 // @has foo/struct.Ice.html '//pre[@class="rust item-decl"]' \ -// 'pub struct Ice;' +// 'pub struct Ice where [(); { _ }]:;' pub struct Ice where [(); N + 1]:; From f5d43a052b9eb464e54af819143467954d814a24 Mon Sep 17 00:00:00 2001 From: Shoyu Vanilla Date: Sat, 17 Feb 2024 14:27:05 +0900 Subject: [PATCH 5/9] Fix missing trait impls for type in rustc docs --- src/librustdoc/clean/inline.rs | 19 ++++++++++++------- src/librustdoc/config.rs | 4 ++++ .../inline_cross/auxiliary/issue-76736-1.rs | 6 ++++++ .../inline_cross/auxiliary/issue-76736-2.rs | 5 +++++ tests/rustdoc/inline_cross/issue-76736-1.rs | 15 +++++++++++++++ tests/rustdoc/inline_cross/issue-76736-2.rs | 16 ++++++++++++++++ tests/rustdoc/inline_cross/issue-76736-3.rs | 16 ++++++++++++++++ 7 files changed, 74 insertions(+), 7 deletions(-) create mode 100644 tests/rustdoc/inline_cross/auxiliary/issue-76736-1.rs create mode 100644 tests/rustdoc/inline_cross/auxiliary/issue-76736-2.rs create mode 100644 tests/rustdoc/inline_cross/issue-76736-1.rs create mode 100644 tests/rustdoc/inline_cross/issue-76736-2.rs create mode 100644 tests/rustdoc/inline_cross/issue-76736-3.rs diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index e2f2c9a5e5663..03f62f41a26f4 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -443,11 +443,13 @@ pub(crate) fn build_impl( return; } - if let Some(stab) = tcx.lookup_stability(did) - && stab.is_unstable() - && stab.feature == sym::rustc_private - { - return; + if !tcx.features().rustc_private && !cx.render_options.force_unstable_if_unmarked { + if let Some(stab) = tcx.lookup_stability(did) + && stab.is_unstable() + && stab.feature == sym::rustc_private + { + return; + } } } @@ -477,8 +479,11 @@ pub(crate) fn build_impl( return; } - if let Some(stab) = tcx.lookup_stability(did) { - if stab.is_unstable() && stab.feature == sym::rustc_private { + if !tcx.features().rustc_private && !cx.render_options.force_unstable_if_unmarked { + if let Some(stab) = tcx.lookup_stability(did) + && stab.is_unstable() + && stab.feature == sym::rustc_private + { return; } } diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index c46047aa0dbad..4b1a417b21127 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -281,6 +281,8 @@ pub(crate) struct RenderOptions { pub(crate) no_emit_shared: bool, /// If `true`, HTML source code pages won't be generated. pub(crate) html_no_source: bool, + /// Whether `-Zforce-unstable-if-unmarked` unstable option is set + pub(crate) force_unstable_if_unmarked: bool, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -347,6 +349,7 @@ impl Options { let codegen_options = CodegenOptions::build(early_dcx, matches); let unstable_opts = UnstableOptions::build(early_dcx, matches); + let force_unstable_if_unmarked = unstable_opts.force_unstable_if_unmarked; let dcx = new_dcx(error_format, None, diagnostic_width, &unstable_opts); @@ -760,6 +763,7 @@ impl Options { call_locations, no_emit_shared: false, html_no_source, + force_unstable_if_unmarked, }; Some((options, render_options)) } diff --git a/tests/rustdoc/inline_cross/auxiliary/issue-76736-1.rs b/tests/rustdoc/inline_cross/auxiliary/issue-76736-1.rs new file mode 100644 index 0000000000000..4ae9f79a5321a --- /dev/null +++ b/tests/rustdoc/inline_cross/auxiliary/issue-76736-1.rs @@ -0,0 +1,6 @@ +#![feature(staged_api)] +#![unstable(feature = "rustc_private", issue = "none")] + +pub trait MaybeResult {} + +impl MaybeResult for T {} diff --git a/tests/rustdoc/inline_cross/auxiliary/issue-76736-2.rs b/tests/rustdoc/inline_cross/auxiliary/issue-76736-2.rs new file mode 100644 index 0000000000000..b5fbac970822d --- /dev/null +++ b/tests/rustdoc/inline_cross/auxiliary/issue-76736-2.rs @@ -0,0 +1,5 @@ +#![feature(rustc_private)] + +extern crate issue_76736_1; + +pub struct Bar; diff --git a/tests/rustdoc/inline_cross/issue-76736-1.rs b/tests/rustdoc/inline_cross/issue-76736-1.rs new file mode 100644 index 0000000000000..25feae2c8d68c --- /dev/null +++ b/tests/rustdoc/inline_cross/issue-76736-1.rs @@ -0,0 +1,15 @@ +// aux-build:issue-76736-1.rs +// aux-build:issue-76736-2.rs + +#![crate_name = "foo"] + +extern crate issue_76736_1; +extern crate issue_76736_2; + +// @has foo/struct.Foo.html +// @!has - '//*[@class="impl"]//h3[@class="code-header"]' 'MaybeResult' +pub struct Foo; + +// @has foo/struct.Bar.html +// @!has - '//*[@class="impl"]//h3[@class="code-header"]' 'MaybeResult' +pub use issue_76736_2::Bar; diff --git a/tests/rustdoc/inline_cross/issue-76736-2.rs b/tests/rustdoc/inline_cross/issue-76736-2.rs new file mode 100644 index 0000000000000..e43c825d6e142 --- /dev/null +++ b/tests/rustdoc/inline_cross/issue-76736-2.rs @@ -0,0 +1,16 @@ +// aux-build:issue-76736-1.rs +// aux-build:issue-76736-2.rs + +#![crate_name = "foo"] +#![feature(rustc_private)] + +extern crate issue_76736_1; +extern crate issue_76736_2; + +// @has foo/struct.Foo.html +// @has - '//*[@class="impl"]//h3[@class="code-header"]' 'MaybeResult' +pub struct Foo; + +// @has foo/struct.Bar.html +// @has - '//*[@class="impl"]//h3[@class="code-header"]' 'MaybeResult' +pub use issue_76736_2::Bar; diff --git a/tests/rustdoc/inline_cross/issue-76736-3.rs b/tests/rustdoc/inline_cross/issue-76736-3.rs new file mode 100644 index 0000000000000..9542f3f3557a8 --- /dev/null +++ b/tests/rustdoc/inline_cross/issue-76736-3.rs @@ -0,0 +1,16 @@ +// compile-flags: -Zforce-unstable-if-unmarked +// aux-build:issue-76736-1.rs +// aux-build:issue-76736-2.rs + +#![crate_name = "foo"] + +extern crate issue_76736_1; +extern crate issue_76736_2; + +// @has foo/struct.Foo.html +// @has - '//*[@class="impl"]//h3[@class="code-header"]' 'MaybeResult' +pub struct Foo; + +// @has foo/struct.Bar.html +// @has - '//*[@class="impl"]//h3[@class="code-header"]' 'MaybeResult' +pub use issue_76736_2::Bar; From f62a695572770330f0853cf9744beb5b3068e6c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Thu, 28 Dec 2023 02:54:22 +0100 Subject: [PATCH 6/9] Update comments and variable names --- .../rustc_hir_analysis/src/astconv/bounds.rs | 52 ++++++++++--------- .../rustc_hir_analysis/src/astconv/errors.rs | 21 ++++---- 2 files changed, 39 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/astconv/bounds.rs b/compiler/rustc_hir_analysis/src/astconv/bounds.rs index dab1e2d525345..0c35ec99dae67 100644 --- a/compiler/rustc_hir_analysis/src/astconv/bounds.rs +++ b/compiler/rustc_hir_analysis/src/astconv/bounds.rs @@ -390,14 +390,12 @@ impl<'tcx> dyn AstConv<'tcx> + '_ { { alias_ty } else { - return Err(self.tcx().dcx().emit_err( - crate::errors::ReturnTypeNotationOnNonRpitit { - span: binding.span, - ty: tcx.liberate_late_bound_regions(assoc_item.def_id, output), - fn_span: tcx.hir().span_if_local(assoc_item.def_id), - note: (), - }, - )); + return Err(tcx.dcx().emit_err(crate::errors::ReturnTypeNotationOnNonRpitit { + span: binding.span, + ty: tcx.liberate_late_bound_regions(assoc_item.def_id, output), + fn_span: tcx.hir().span_if_local(assoc_item.def_id), + note: (), + })); }; // Finally, move the fn return type's bound vars over to account for the early bound @@ -410,7 +408,9 @@ impl<'tcx> dyn AstConv<'tcx> + '_ { let bound_vars = tcx.late_bound_vars(binding.hir_id); ty::Binder::bind_with_vars(instantiation_output, bound_vars) } else { - // Append the generic arguments of the associated type to the `trait_ref`. + // Create the generic arguments for the associated type or constant by joining the + // parent arguments (the arguments of the trait) and the own arguments (the ones of + // the associated item itself) and construct an alias type using them. candidate.map_bound(|trait_ref| { let ident = Ident::new(assoc_item.name, binding.item_name.span); let item_segment = hir::PathSegment { @@ -421,16 +421,18 @@ impl<'tcx> dyn AstConv<'tcx> + '_ { infer_args: false, }; - let args_trait_ref_and_assoc_item = self.create_args_for_associated_item( + let alias_args = self.create_args_for_associated_item( path_span, assoc_item.def_id, &item_segment, trait_ref.args, ); + debug!(?alias_args); - debug!(?args_trait_ref_and_assoc_item); - - ty::AliasTy::new(tcx, assoc_item.def_id, args_trait_ref_and_assoc_item) + // Note that we're indeed also using `AliasTy` (alias *type*) for associated + // *constants* to represent *const projections*. Alias *term* would be a more + // appropriate name but alas. + ty::AliasTy::new(tcx, assoc_item.def_id, alias_args) }) }; @@ -442,20 +444,22 @@ impl<'tcx> dyn AstConv<'tcx> + '_ { // // for<'a> ::Item = &'a str // <-- 'a is bad // for<'a> >::Output = &'a str // <-- 'a is ok - if let ConvertedBindingKind::Equality(ty) = binding.kind { - let late_bound_in_trait_ref = + if let ConvertedBindingKind::Equality(term) = binding.kind { + let late_bound_in_projection_ty = tcx.collect_constrained_late_bound_regions(&projection_ty); - let late_bound_in_ty = - tcx.collect_referenced_late_bound_regions(&trait_ref.rebind(ty.node)); - debug!(?late_bound_in_trait_ref); - debug!(?late_bound_in_ty); + let late_bound_in_term = + tcx.collect_referenced_late_bound_regions(&trait_ref.rebind(term.node)); + debug!(?late_bound_in_projection_ty); + debug!(?late_bound_in_term); + // NOTE(associated_const_equality): This error should be impossible to trigger + // with associated const equality bounds. // FIXME: point at the type params that don't have appropriate lifetimes: // struct S1 Fn(&i32, &i32) -> &'a i32>(F); // ---- ---- ^^^^^^^ self.validate_late_bound_regions( - late_bound_in_trait_ref, - late_bound_in_ty, + late_bound_in_projection_ty, + late_bound_in_term, |br_name| { struct_span_code_err!( tcx.dcx(), @@ -473,9 +477,9 @@ impl<'tcx> dyn AstConv<'tcx> + '_ { match binding.kind { ConvertedBindingKind::Equality(..) if let ty::AssocKind::Fn = assoc_kind => { - return Err(self.tcx().dcx().emit_err( - crate::errors::ReturnTypeNotationEqualityBound { span: binding.span }, - )); + return Err(tcx.dcx().emit_err(crate::errors::ReturnTypeNotationEqualityBound { + span: binding.span, + })); } ConvertedBindingKind::Equality(term) => { // "Desugar" a constraint like `T: Iterator` this to diff --git a/compiler/rustc_hir_analysis/src/astconv/errors.rs b/compiler/rustc_hir_analysis/src/astconv/errors.rs index ea8d364bba6c1..f04114229bf99 100644 --- a/compiler/rustc_hir_analysis/src/astconv/errors.rs +++ b/compiler/rustc_hir_analysis/src/astconv/errors.rs @@ -243,7 +243,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { None, ) && suggested_name != assoc_name.name { - // We suggested constraining a type parameter, but the associated type on it + // We suggested constraining a type parameter, but the associated item on it // was also not an exact match, so we also suggest changing it. err.span_suggestion_verbose( assoc_name.span, @@ -258,16 +258,17 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { } } - // If we still couldn't find any associated type, and only one associated type exists, + // If we still couldn't find any associated item, and only one associated item exists, // suggests using it. if let [candidate_name] = all_candidate_names.as_slice() { - // this should still compile, except on `#![feature(associated_type_defaults)]` - // where it could suggests `type A = Self::A`, thus recursing infinitely - let applicability = if tcx.features().associated_type_defaults { - Applicability::Unspecified - } else { - Applicability::MaybeIncorrect - }; + // This should still compile, except on `#![feature(associated_type_defaults)]` + // where it could suggests `type A = Self::A`, thus recursing infinitely. + let applicability = + if assoc_kind == ty::AssocKind::Type && tcx.features().associated_type_defaults { + Applicability::Unspecified + } else { + Applicability::MaybeIncorrect + }; err.sugg = Some(errors::AssocItemNotFoundSugg::Other { span: assoc_name.span, @@ -323,7 +324,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { }; // For equality bounds, we want to blame the term (RHS) instead of the item (LHS) since - // one can argue that that's more “untuitive” to the user. + // one can argue that that's more “intuitive” to the user. let (span, expected_because_label, expected, got) = if let Some(binding) = binding && let ConvertedBindingKind::Equality(term) = binding.kind { From 38a2a65f0b7bcf137bc4756c4fa1db1c3ba3b0c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Thu, 28 Dec 2023 16:38:24 +0100 Subject: [PATCH 7/9] Remove astconv::ConvertedBinding --- .../rustc_hir_analysis/src/astconv/bounds.rs | 132 +++++++++--------- .../rustc_hir_analysis/src/astconv/errors.rs | 24 ++-- .../rustc_hir_analysis/src/astconv/mod.rs | 91 +++--------- 3 files changed, 96 insertions(+), 151 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/astconv/bounds.rs b/compiler/rustc_hir_analysis/src/astconv/bounds.rs index 0c35ec99dae67..6940b4a504584 100644 --- a/compiler/rustc_hir_analysis/src/astconv/bounds.rs +++ b/compiler/rustc_hir_analysis/src/astconv/bounds.rs @@ -9,9 +9,7 @@ use rustc_span::{ErrorGuaranteed, Span}; use rustc_trait_selection::traits; use smallvec::SmallVec; -use crate::astconv::{ - AstConv, ConvertedBinding, ConvertedBindingKind, OnlySelfBounds, PredicateFilter, -}; +use crate::astconv::{AstConv, OnlySelfBounds, PredicateFilter}; use crate::bounds::Bounds; use crate::errors; @@ -238,7 +236,7 @@ impl<'tcx> dyn AstConv<'tcx> + '_ { &self, hir_ref_id: hir::HirId, trait_ref: ty::PolyTraitRef<'tcx>, - binding: &ConvertedBinding<'_, 'tcx>, + binding: &hir::TypeBinding<'tcx>, bounds: &mut Bounds<'tcx>, speculative: bool, dup_bindings: &mut FxIndexMap, @@ -263,21 +261,20 @@ impl<'tcx> dyn AstConv<'tcx> + '_ { let tcx = self.tcx(); - let assoc_kind = - if binding.gen_args.parenthesized == hir::GenericArgsParentheses::ReturnTypeNotation { - ty::AssocKind::Fn - } else if let ConvertedBindingKind::Equality(term) = binding.kind - && let ty::TermKind::Const(_) = term.node.unpack() - { - ty::AssocKind::Const - } else { - ty::AssocKind::Type - }; + let assoc_kind = if binding.gen_args.parenthesized + == hir::GenericArgsParentheses::ReturnTypeNotation + { + ty::AssocKind::Fn + } else if let hir::TypeBindingKind::Equality { term: hir::Term::Const(_) } = binding.kind { + ty::AssocKind::Const + } else { + ty::AssocKind::Type + }; let candidate = if self.trait_defines_associated_item_named( trait_ref.def_id(), assoc_kind, - binding.item_name, + binding.ident, ) { // Simple case: The assoc item is defined in the current trait. trait_ref @@ -289,14 +286,14 @@ impl<'tcx> dyn AstConv<'tcx> + '_ { trait_ref.skip_binder().print_only_trait_name(), None, assoc_kind, - binding.item_name, + binding.ident, path_span, - Some(&binding), + Some(binding), )? }; let (assoc_ident, def_scope) = - tcx.adjust_ident_and_get_scope(binding.item_name, candidate.def_id(), hir_ref_id); + tcx.adjust_ident_and_get_scope(binding.ident, candidate.def_id(), hir_ref_id); // We have already adjusted the item name above, so compare with `.normalize_to_macros_2_0()` // instead of calling `filter_by_name_and_kind` which would needlessly normalize the @@ -312,7 +309,7 @@ impl<'tcx> dyn AstConv<'tcx> + '_ { .dcx() .struct_span_err( binding.span, - format!("{} `{}` is private", assoc_item.kind, binding.item_name), + format!("{} `{}` is private", assoc_item.kind, binding.ident), ) .with_span_label(binding.span, format!("private {}", assoc_item.kind)) .emit(); @@ -327,7 +324,7 @@ impl<'tcx> dyn AstConv<'tcx> + '_ { tcx.dcx().emit_err(errors::ValueOfAssociatedStructAlreadySpecified { span: binding.span, prev_span: *prev_span, - item_name: binding.item_name, + item_name: binding.ident, def_path: tcx.def_path_str(assoc_item.container_id(tcx)), }); }) @@ -412,7 +409,7 @@ impl<'tcx> dyn AstConv<'tcx> + '_ { // parent arguments (the arguments of the trait) and the own arguments (the ones of // the associated item itself) and construct an alias type using them. candidate.map_bound(|trait_ref| { - let ident = Ident::new(assoc_item.name, binding.item_name.span); + let ident = Ident::new(assoc_item.name, binding.ident.span); let item_segment = hir::PathSegment { ident, hir_id: binding.hir_id, @@ -436,66 +433,67 @@ impl<'tcx> dyn AstConv<'tcx> + '_ { }) }; - if !speculative { - // Find any late-bound regions declared in `ty` that are not - // declared in the trait-ref or assoc_item. These are not well-formed. - // - // Example: - // - // for<'a> ::Item = &'a str // <-- 'a is bad - // for<'a> >::Output = &'a str // <-- 'a is ok - if let ConvertedBindingKind::Equality(term) = binding.kind { - let late_bound_in_projection_ty = - tcx.collect_constrained_late_bound_regions(&projection_ty); - let late_bound_in_term = - tcx.collect_referenced_late_bound_regions(&trait_ref.rebind(term.node)); - debug!(?late_bound_in_projection_ty); - debug!(?late_bound_in_term); - - // NOTE(associated_const_equality): This error should be impossible to trigger - // with associated const equality bounds. - // FIXME: point at the type params that don't have appropriate lifetimes: - // struct S1 Fn(&i32, &i32) -> &'a i32>(F); - // ---- ---- ^^^^^^^ - self.validate_late_bound_regions( - late_bound_in_projection_ty, - late_bound_in_term, - |br_name| { - struct_span_code_err!( - tcx.dcx(), - binding.span, - E0582, - "binding for associated type `{}` references {}, \ - which does not appear in the trait input types", - binding.item_name, - br_name - ) - }, - ); - } - } - match binding.kind { - ConvertedBindingKind::Equality(..) if let ty::AssocKind::Fn = assoc_kind => { + hir::TypeBindingKind::Equality { .. } if let ty::AssocKind::Fn = assoc_kind => { return Err(tcx.dcx().emit_err(crate::errors::ReturnTypeNotationEqualityBound { span: binding.span, })); } - ConvertedBindingKind::Equality(term) => { + hir::TypeBindingKind::Equality { term } => { + let term = match term { + hir::Term::Ty(ty) => self.ast_ty_to_ty(ty).into(), + hir::Term::Const(ct) => ty::Const::from_anon_const(tcx, ct.def_id).into(), + }; + + if !speculative { + // Find any late-bound regions declared in `ty` that are not + // declared in the trait-ref or assoc_item. These are not well-formed. + // + // Example: + // + // for<'a> ::Item = &'a str // <-- 'a is bad + // for<'a> >::Output = &'a str // <-- 'a is ok + let late_bound_in_projection_ty = + tcx.collect_constrained_late_bound_regions(&projection_ty); + let late_bound_in_term = + tcx.collect_referenced_late_bound_regions(&trait_ref.rebind(term)); + debug!(?late_bound_in_projection_ty); + debug!(?late_bound_in_term); + + // FIXME: point at the type params that don't have appropriate lifetimes: + // struct S1 Fn(&i32, &i32) -> &'a i32>(F); + // ---- ---- ^^^^^^^ + // NOTE(associated_const_equality): This error should be impossible to trigger + // with associated const equality bounds. + self.validate_late_bound_regions( + late_bound_in_projection_ty, + late_bound_in_term, + |br_name| { + struct_span_code_err!( + tcx.dcx(), + binding.span, + E0582, + "binding for associated type `{}` references {}, \ + which does not appear in the trait input types", + binding.ident, + br_name + ) + }, + ); + } + // "Desugar" a constraint like `T: Iterator` this to // the "projection predicate" for: // // `::Item = u32` bounds.push_projection_bound( tcx, - projection_ty.map_bound(|projection_ty| ty::ProjectionPredicate { - projection_ty, - term: term.node, - }), + projection_ty + .map_bound(|projection_ty| ty::ProjectionPredicate { projection_ty, term }), binding.span, ); } - ConvertedBindingKind::Constraint(ast_bounds) => { + hir::TypeBindingKind::Constraint { bounds: ast_bounds } => { // "Desugar" a constraint like `T: Iterator` to // // `::Item: Debug` diff --git a/compiler/rustc_hir_analysis/src/astconv/errors.rs b/compiler/rustc_hir_analysis/src/astconv/errors.rs index f04114229bf99..ad34c31ef8fc7 100644 --- a/compiler/rustc_hir_analysis/src/astconv/errors.rs +++ b/compiler/rustc_hir_analysis/src/astconv/errors.rs @@ -1,4 +1,4 @@ -use crate::astconv::{AstConv, ConvertedBindingKind}; +use crate::astconv::AstConv; use crate::errors::{ self, AssocTypeBindingNotAllowed, ManualImplementation, MissingTypeParams, ParenthesizedFnTraitExpansion, @@ -111,7 +111,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { assoc_kind: ty::AssocKind, assoc_name: Ident, span: Span, - binding: Option<&super::ConvertedBinding<'_, 'tcx>>, + binding: Option<&hir::TypeBinding<'tcx>>, ) -> ErrorGuaranteed where I: Iterator>, @@ -290,13 +290,13 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { assoc_kind: ty::AssocKind, ident: Ident, span: Span, - binding: Option<&super::ConvertedBinding<'_, 'tcx>>, + binding: Option<&hir::TypeBinding<'tcx>>, ) -> ErrorGuaranteed { let tcx = self.tcx(); let bound_on_assoc_const_label = if let ty::AssocKind::Const = assoc_item.kind && let Some(binding) = binding - && let ConvertedBindingKind::Constraint(_) = binding.kind + && let hir::TypeBindingKind::Constraint { .. } = binding.kind { let lo = if binding.gen_args.span_ext.is_dummy() { ident.span @@ -310,14 +310,14 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { // FIXME(associated_const_equality): This has quite a few false positives and negatives. let wrap_in_braces_sugg = if let Some(binding) = binding - && let ConvertedBindingKind::Equality(term) = binding.kind - && let ty::TermKind::Ty(ty) = term.node.unpack() + && let hir::TypeBindingKind::Equality { term: hir::Term::Ty(hir_ty) } = binding.kind + && let ty = self.ast_ty_to_ty(hir_ty) && (ty.is_enum() || ty.references_error()) && tcx.features().associated_const_equality { Some(errors::AssocKindMismatchWrapInBracesSugg { - lo: term.span.shrink_to_lo(), - hi: term.span.shrink_to_hi(), + lo: hir_ty.span.shrink_to_lo(), + hi: hir_ty.span.shrink_to_hi(), }) } else { None @@ -326,9 +326,13 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { // For equality bounds, we want to blame the term (RHS) instead of the item (LHS) since // one can argue that that's more “intuitive” to the user. let (span, expected_because_label, expected, got) = if let Some(binding) = binding - && let ConvertedBindingKind::Equality(term) = binding.kind + && let hir::TypeBindingKind::Equality { term } = binding.kind { - (term.span, Some(ident.span), assoc_item.kind, assoc_kind) + let span = match term { + hir::Term::Ty(ty) => ty.span, + hir::Term::Const(ct) => tcx.def_span(ct.def_id), + }; + (span, Some(ident.span), assoc_item.kind, assoc_kind) } else { (ident.span, None, assoc_kind, assoc_item.kind) }; diff --git a/compiler/rustc_hir_analysis/src/astconv/mod.rs b/compiler/rustc_hir_analysis/src/astconv/mod.rs index 94c49453cdcf3..3ccf78567edd2 100644 --- a/compiler/rustc_hir_analysis/src/astconv/mod.rs +++ b/compiler/rustc_hir_analysis/src/astconv/mod.rs @@ -35,7 +35,6 @@ use rustc_middle::ty::{ }; use rustc_session::lint::builtin::AMBIGUOUS_ASSOCIATED_ITEMS; use rustc_span::edit_distance::find_best_match_for_name; -use rustc_span::source_map::{respan, Spanned}; use rustc_span::symbol::{kw, Ident, Symbol}; use rustc_span::{sym, BytePos, Span, DUMMY_SP}; use rustc_target::spec::abi; @@ -151,21 +150,6 @@ pub trait AstConv<'tcx> { fn infcx(&self) -> Option<&InferCtxt<'tcx>>; } -#[derive(Debug)] -struct ConvertedBinding<'a, 'tcx> { - hir_id: hir::HirId, - item_name: Ident, - kind: ConvertedBindingKind<'a, 'tcx>, - gen_args: &'tcx GenericArgs<'tcx>, - span: Span, -} - -#[derive(Debug)] -enum ConvertedBindingKind<'a, 'tcx> { - Equality(Spanned>), - Constraint(&'a [hir::GenericBound<'tcx>]), -} - /// New-typed boolean indicating whether explicit late-bound lifetimes /// are present in a set of generic arguments. /// @@ -316,7 +300,9 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { /// Given the type/lifetime/const arguments provided to some path (along with /// an implicit `Self`, if this is a trait reference), returns the complete /// set of generic arguments. This may involve applying defaulted type parameters. - /// Constraints on associated types are created from `create_assoc_bindings_for_generic_args`. + /// + /// Constraints on associated types are not converted here but + /// separately in `add_predicates_for_ast_type_binding`. /// /// Example: /// @@ -329,8 +315,8 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { /// 2. The path in question is the path to the trait `std::ops::Index`, /// which will have been resolved to a `def_id` /// 3. The `generic_args` contains info on the `<...>` contents. The `usize` type - /// parameters are returned in the `GenericArgsRef`, the associated type bindings like - /// `Output = u32` are returned from `create_assoc_bindings_for_generic_args`. + /// parameters are returned in the `GenericArgsRef` + /// 4. Associated type bindings like `Output = u32` are contained in `generic_args.bindings`. /// /// Note that the type listing given here is *exactly* what the user provided. /// @@ -591,52 +577,6 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { (args, arg_count) } - fn create_assoc_bindings_for_generic_args<'a>( - &self, - generic_args: &'a hir::GenericArgs<'tcx>, - ) -> Vec> { - // Convert associated-type bindings or constraints into a separate vector. - // Example: Given this: - // - // T: Iterator - // - // The `T` is passed in as a self-type; the `Item = u32` is - // not a "type parameter" of the `Iterator` trait, but rather - // a restriction on `::Item`, so it is passed - // back separately. - let assoc_bindings = generic_args - .bindings - .iter() - .map(|binding| { - let kind = match &binding.kind { - hir::TypeBindingKind::Equality { term } => match term { - hir::Term::Ty(ty) => ConvertedBindingKind::Equality(respan( - ty.span, - self.ast_ty_to_ty(ty).into(), - )), - hir::Term::Const(c) => { - let span = self.tcx().def_span(c.def_id); - let c = Const::from_anon_const(self.tcx(), c.def_id); - ConvertedBindingKind::Equality(respan(span, c.into())) - } - }, - hir::TypeBindingKind::Constraint { bounds } => { - ConvertedBindingKind::Constraint(bounds) - } - }; - ConvertedBinding { - hir_id: binding.hir_id, - item_name: binding.ident, - kind, - gen_args: binding.gen_args, - span: binding.span, - } - }) - .collect(); - - assoc_bindings - } - pub fn create_args_for_associated_item( &self, span: Span, @@ -742,18 +682,16 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { let bound_vars = tcx.late_bound_vars(trait_ref.hir_ref_id); debug!(?bound_vars); - let assoc_bindings = self.create_assoc_bindings_for_generic_args(args); - let poly_trait_ref = ty::Binder::bind_with_vars( ty::TraitRef::new(tcx, trait_def_id, generic_args), bound_vars, ); - debug!(?poly_trait_ref, ?assoc_bindings); + debug!(?poly_trait_ref); bounds.push_trait_bound(tcx, poly_trait_ref, span, polarity); let mut dup_bindings = FxIndexMap::default(); - for binding in &assoc_bindings { + for binding in args.bindings { // Don't register additional associated type bounds for negative bounds, // since we should have emitten an error for them earlier, and they will // not be well-formed! @@ -1029,7 +967,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { assoc_kind: ty::AssocKind, assoc_name: Ident, span: Span, - binding: Option<&ConvertedBinding<'_, 'tcx>>, + binding: Option<&hir::TypeBinding<'tcx>>, ) -> Result, ErrorGuaranteed> where I: Iterator>, @@ -1069,7 +1007,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { // Provide a more specific error code index entry for equality bindings. err.code( if let Some(binding) = binding - && let ConvertedBindingKind::Equality(_) = binding.kind + && let hir::TypeBindingKind::Equality { .. } = binding.kind { E0222 } else { @@ -1094,16 +1032,21 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { ); if let Some(binding) = binding { match binding.kind { - ConvertedBindingKind::Equality(term) => { + hir::TypeBindingKind::Equality { term } => { + let term: ty::Term<'_> = match term { + hir::Term::Ty(ty) => self.ast_ty_to_ty(ty).into(), + hir::Term::Const(ct) => { + ty::Const::from_anon_const(tcx, ct.def_id).into() + } + }; // FIXME(#97583): This isn't syntactically well-formed! where_bounds.push(format!( " T: {trait}::{assoc_name} = {term}", trait = bound.print_only_trait_path(), - term = term.node, )); } // FIXME: Provide a suggestion. - ConvertedBindingKind::Constraint(_bounds) => {} + hir::TypeBindingKind::Constraint { bounds: _ } => {} } } else { err.span_suggestion_verbose( From 62b789fba444476eeddb9ae575d69d66a53a23c6 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Fri, 16 Feb 2024 18:48:09 +0000 Subject: [PATCH 8/9] Add more checks for `unnamed_field` during HIR analysis --- compiler/rustc_hir/src/hir.rs | 6 ++ compiler/rustc_hir_analysis/messages.ftl | 2 + .../rustc_hir_analysis/src/check/check.rs | 23 ++++--- compiler/rustc_hir_analysis/src/collect.rs | 10 ++- compiler/rustc_hir_analysis/src/errors.rs | 7 +++ .../ui/union/unnamed-fields/auxiliary/dep.rs | 18 ++++++ .../union/unnamed-fields/restrict_type_hir.rs | 44 +++++++++++++ .../unnamed-fields/restrict_type_hir.stderr | 62 +++++++++++++++++++ 8 files changed, 161 insertions(+), 11 deletions(-) create mode 100644 tests/ui/union/unnamed-fields/auxiliary/dep.rs create mode 100644 tests/ui/union/unnamed-fields/restrict_type_hir.rs create mode 100644 tests/ui/union/unnamed-fields/restrict_type_hir.stderr diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 77044df9a4092..fcb15925f6a7b 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -2998,6 +2998,12 @@ impl<'hir> Item<'hir> { ItemId { owner_id: self.owner_id } } + /// Check if this is an [`ItemKind::Enum`], [`ItemKind::Struct`] or + /// [`ItemKind::Union`]. + pub fn is_adt(&self) -> bool { + matches!(self.kind, ItemKind::Enum(..) | ItemKind::Struct(..) | ItemKind::Union(..)) + } + expect_methods_self_kind! { expect_extern_crate, Option, ItemKind::ExternCrate(s), *s; diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index a61cfd0e4ce9a..f32b14aca23fd 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -198,6 +198,8 @@ hir_analysis_invalid_union_field = hir_analysis_invalid_union_field_sugg = wrap the field type in `ManuallyDrop<...>` +hir_analysis_invalid_unnamed_field_ty = unnamed fields can only have struct or union types + hir_analysis_late_bound_const_in_apit = `impl Trait` can only mention const parameters from an fn or impl .label = const parameter declared here diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 1410273e3bc9e..2c367b15df2ab 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -129,17 +129,20 @@ fn check_unnamed_fields(tcx: TyCtxt<'_>, def: ty::AdtDef<'_>) { for field in variant.fields.iter().filter(|f| f.is_unnamed()) { let field_ty = tcx.type_of(field.did).instantiate_identity(); if let Some(adt) = field_ty.ty_adt_def() - && !adt.is_anonymous() - && !adt.repr().c() + && !adt.is_enum() { - let field_ty_span = tcx.def_span(adt.did()); - tcx.dcx().emit_err(errors::UnnamedFieldsRepr::FieldMissingReprC { - span: tcx.def_span(field.did), - field_ty_span, - field_ty, - field_adt_kind: adt.descr(), - sugg_span: field_ty_span.shrink_to_lo(), - }); + if !adt.is_anonymous() && !adt.repr().c() { + let field_ty_span = tcx.def_span(adt.did()); + tcx.dcx().emit_err(errors::UnnamedFieldsRepr::FieldMissingReprC { + span: tcx.def_span(field.did), + field_ty_span, + field_ty, + field_adt_kind: adt.descr(), + sugg_span: field_ty_span.shrink_to_lo(), + }); + } + } else { + tcx.dcx().emit_err(errors::InvalidUnnamedFieldTy { span: tcx.def_span(field.did) }); } } } diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index 43f0af5bd1da7..a5ef1490bce97 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -943,7 +943,15 @@ impl<'tcx> FieldUniquenessCheckContext<'tcx> { } } hir::TyKind::Path(hir::QPath::Resolved(_, hir::Path { res, .. })) => { - self.check_field_in_nested_adt(self.tcx.adt_def(res.def_id()), field.span); + // If this is a direct path to an ADT, we can check it + // If this is a type alias or non-ADT, `check_unnamed_fields` should verify it + if let Some(def_id) = res.opt_def_id() + && let Some(local) = def_id.as_local() + && let Node::Item(item) = self.tcx.hir_node_by_def_id(local) + && item.is_adt() + { + self.check_field_in_nested_adt(self.tcx.adt_def(def_id), field.span); + } } // Abort due to errors (there must be an error if an unnamed field // has any type kind other than an anonymous adt or a named adt) diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index 6a505b961974f..3bd7687a54425 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -661,6 +661,13 @@ pub(crate) struct InvalidUnionField { pub note: (), } +#[derive(Diagnostic)] +#[diag(hir_analysis_invalid_unnamed_field_ty)] +pub struct InvalidUnnamedFieldTy { + #[primary_span] + pub span: Span, +} + #[derive(Diagnostic)] #[diag(hir_analysis_return_type_notation_on_non_rpitit)] pub(crate) struct ReturnTypeNotationOnNonRpitit<'tcx> { diff --git a/tests/ui/union/unnamed-fields/auxiliary/dep.rs b/tests/ui/union/unnamed-fields/auxiliary/dep.rs new file mode 100644 index 0000000000000..a11f3e18f52a9 --- /dev/null +++ b/tests/ui/union/unnamed-fields/auxiliary/dep.rs @@ -0,0 +1,18 @@ +#[repr(C)] +pub struct GoodStruct(()); + +pub struct BadStruct(()); + +pub enum BadEnum { + A, + B, +} + +#[repr(C)] +pub enum BadEnum2 { + A, + B, +} + +pub type GoodAlias = GoodStruct; +pub type BadAlias = i32; diff --git a/tests/ui/union/unnamed-fields/restrict_type_hir.rs b/tests/ui/union/unnamed-fields/restrict_type_hir.rs new file mode 100644 index 0000000000000..80e4608be82e6 --- /dev/null +++ b/tests/ui/union/unnamed-fields/restrict_type_hir.rs @@ -0,0 +1,44 @@ +//@ aux-build:dep.rs + +// test for #121151 + +#![allow(incomplete_features)] +#![feature(unnamed_fields)] + +extern crate dep; + +#[repr(C)] +struct A { + a: u8, +} + +enum BadEnum { + A, + B, +} + +#[repr(C)] +enum BadEnum2 { + A, + B, +} + +type MyStruct = A; +type MyI32 = i32; + +#[repr(C)] +struct L { + _: i32, //~ ERROR unnamed fields can only have struct or union types + _: MyI32, //~ ERROR unnamed fields can only have struct or union types + _: BadEnum, //~ ERROR unnamed fields can only have struct or union types + _: BadEnum2, //~ ERROR unnamed fields can only have struct or union types + _: MyStruct, + _: dep::BadStruct, //~ ERROR named type of unnamed field must have `#[repr(C)]` representation + _: dep::BadEnum, //~ ERROR unnamed fields can only have struct or union types + _: dep::BadEnum2, //~ ERROR unnamed fields can only have struct or union types + _: dep::BadAlias, //~ ERROR unnamed fields can only have struct or union types + _: dep::GoodAlias, + _: dep::GoodStruct, +} + +fn main() {} diff --git a/tests/ui/union/unnamed-fields/restrict_type_hir.stderr b/tests/ui/union/unnamed-fields/restrict_type_hir.stderr new file mode 100644 index 0000000000000..3e80d7fbf5c0e --- /dev/null +++ b/tests/ui/union/unnamed-fields/restrict_type_hir.stderr @@ -0,0 +1,62 @@ +error: unnamed fields can only have struct or union types + --> $DIR/restrict_type_hir.rs:31:5 + | +LL | _: i32, + | ^^^^^^ + +error: unnamed fields can only have struct or union types + --> $DIR/restrict_type_hir.rs:32:5 + | +LL | _: MyI32, + | ^^^^^^^^ + +error: unnamed fields can only have struct or union types + --> $DIR/restrict_type_hir.rs:33:5 + | +LL | _: BadEnum, + | ^^^^^^^^^^ + +error: unnamed fields can only have struct or union types + --> $DIR/restrict_type_hir.rs:34:5 + | +LL | _: BadEnum2, + | ^^^^^^^^^^^ + +error: named type of unnamed field must have `#[repr(C)]` representation + --> $DIR/restrict_type_hir.rs:36:5 + | +LL | _: dep::BadStruct, + | ^^^^^^^^^^^^^^^^^ unnamed field defined here + | + ::: $DIR/auxiliary/dep.rs:4:1 + | +LL | pub struct BadStruct(()); + | -------------------- `BadStruct` defined here + | +help: add `#[repr(C)]` to this struct + --> $DIR/auxiliary/dep.rs:4:1 + | +LL + #[repr(C)] +LL | pub struct BadStruct(()); + | + +error: unnamed fields can only have struct or union types + --> $DIR/restrict_type_hir.rs:37:5 + | +LL | _: dep::BadEnum, + | ^^^^^^^^^^^^^^^ + +error: unnamed fields can only have struct or union types + --> $DIR/restrict_type_hir.rs:38:5 + | +LL | _: dep::BadEnum2, + | ^^^^^^^^^^^^^^^^ + +error: unnamed fields can only have struct or union types + --> $DIR/restrict_type_hir.rs:39:5 + | +LL | _: dep::BadAlias, + | ^^^^^^^^^^^^^^^^ + +error: aborting due to 8 previous errors + From d988d8f4ba069e11ace537feea4e4e226e7f4afe Mon Sep 17 00:00:00 2001 From: Urgau Date: Sat, 17 Feb 2024 16:45:59 +0100 Subject: [PATCH 9/9] Use better heuristic for printing Cargo specific diagnostics --- compiler/rustc_codegen_ssa/src/errors.rs | 7 +++++-- compiler/rustc_hir_typeck/src/errors.rs | 2 +- compiler/rustc_incremental/src/persist/fs.rs | 2 +- .../rustc_lint/src/context/diagnostics.rs | 4 ++-- compiler/rustc_parse/src/errors.rs | 2 +- compiler/rustc_session/src/utils.rs | 20 ++++++++++++++++++- ...uggest-switching-edition-on-await-cargo.rs | 2 +- tests/ui/check-cfg/cargo-feature.rs | 2 +- tests/ui/check-cfg/diagnotics.rs | 4 ++-- tests/ui/crate-loading/missing-std.rs | 2 +- 10 files changed, 34 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index 3d7903b5efb09..e42a8bd9ed98d 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -362,8 +362,11 @@ impl IntoDiagnostic<'_, G> for LinkingFailed<'_> { // which by now we have no way to translate. if contains_undefined_ref { diag.note(fluent::codegen_ssa_extern_funcs_not_found) - .note(fluent::codegen_ssa_specify_libraries_to_link) - .note(fluent::codegen_ssa_use_cargo_directive); + .note(fluent::codegen_ssa_specify_libraries_to_link); + + if rustc_session::utils::was_invoked_from_cargo() { + diag.note(fluent::codegen_ssa_use_cargo_directive); + } } diag } diff --git a/compiler/rustc_hir_typeck/src/errors.rs b/compiler/rustc_hir_typeck/src/errors.rs index 10e12d01b1fe6..1af0b75bd23c0 100644 --- a/compiler/rustc_hir_typeck/src/errors.rs +++ b/compiler/rustc_hir_typeck/src/errors.rs @@ -293,7 +293,7 @@ pub enum HelpUseLatestEdition { impl HelpUseLatestEdition { pub fn new() -> Self { let edition = LATEST_STABLE_EDITION; - if std::env::var_os("CARGO").is_some() { + if rustc_session::utils::was_invoked_from_cargo() { Self::Cargo { edition } } else { Self::Standalone { edition } diff --git a/compiler/rustc_incremental/src/persist/fs.rs b/compiler/rustc_incremental/src/persist/fs.rs index 2578f284dee7a..23d29916922ff 100644 --- a/compiler/rustc_incremental/src/persist/fs.rs +++ b/compiler/rustc_incremental/src/persist/fs.rs @@ -492,7 +492,7 @@ fn lock_directory( lock_err, session_dir, is_unsupported_lock, - is_cargo: std::env::var_os("CARGO").map(|_| ()), + is_cargo: rustc_session::utils::was_invoked_from_cargo().then_some(()), })) } } diff --git a/compiler/rustc_lint/src/context/diagnostics.rs b/compiler/rustc_lint/src/context/diagnostics.rs index 0fa61c5d87e77..5af2b6daec185 100644 --- a/compiler/rustc_lint/src/context/diagnostics.rs +++ b/compiler/rustc_lint/src/context/diagnostics.rs @@ -205,7 +205,7 @@ pub(super) fn builtin( Vec::new() }; - let is_from_cargo = std::env::var_os("CARGO").is_some(); + let is_from_cargo = rustc_session::utils::was_invoked_from_cargo(); let mut is_feature_cfg = name == sym::feature; if is_feature_cfg && is_from_cargo { @@ -340,7 +340,7 @@ pub(super) fn builtin( .copied() .flatten() .collect(); - let is_from_cargo = std::env::var_os("CARGO").is_some(); + let is_from_cargo = rustc_session::utils::was_invoked_from_cargo(); // Show the full list if all possible values for a given name, but don't do it // for names as the possibilities could be very long diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs index 3c3a8d6fbb9c7..674f7218ea6d0 100644 --- a/compiler/rustc_parse/src/errors.rs +++ b/compiler/rustc_parse/src/errors.rs @@ -2545,7 +2545,7 @@ pub enum HelpUseLatestEdition { impl HelpUseLatestEdition { pub fn new() -> Self { let edition = LATEST_STABLE_EDITION; - if std::env::var_os("CARGO").is_some() { + if rustc_session::utils::was_invoked_from_cargo() { Self::Cargo { edition } } else { Self::Standalone { edition } diff --git a/compiler/rustc_session/src/utils.rs b/compiler/rustc_session/src/utils.rs index f76c69af526e9..50ebbdccf6722 100644 --- a/compiler/rustc_session/src/utils.rs +++ b/compiler/rustc_session/src/utils.rs @@ -1,7 +1,10 @@ use crate::session::Session; use rustc_data_structures::profiling::VerboseTimingGuard; use rustc_fs_util::try_canonicalize; -use std::path::{Path, PathBuf}; +use std::{ + path::{Path, PathBuf}, + sync::OnceLock, +}; impl Session { pub fn timer(&self, what: &'static str) -> VerboseTimingGuard<'_> { @@ -158,3 +161,18 @@ pub fn extra_compiler_flags() -> Option<(Vec, bool)> { if !result.is_empty() { Some((result, excluded_cargo_defaults)) } else { None } } + +/// Returns whenever rustc was launched by Cargo as opposed to another build system. +/// +/// To be used in diagnostics to avoid printing Cargo specific suggestions to other +/// build systems (like Bazel, Buck2, Makefile, ...). +pub fn was_invoked_from_cargo() -> bool { + static FROM_CARGO: OnceLock = OnceLock::new(); + + // To be able to detect Cargo, we use the simplest and least intrusive + // way: we check whenever the `CARGO_CRATE_NAME` env is set. + // + // Note that it is common in Makefiles to define the `CARGO` env even + // though we may not have been called by Cargo, so we avoid using it. + *FROM_CARGO.get_or_init(|| std::env::var_os("CARGO_CRATE_NAME").is_some()) +} diff --git a/tests/ui/async-await/suggest-switching-edition-on-await-cargo.rs b/tests/ui/async-await/suggest-switching-edition-on-await-cargo.rs index e5a3d54c5d036..e1fae0f0e9343 100644 --- a/tests/ui/async-await/suggest-switching-edition-on-await-cargo.rs +++ b/tests/ui/async-await/suggest-switching-edition-on-await-cargo.rs @@ -1,4 +1,4 @@ -//@ rustc-env:CARGO=/usr/bin/cargo +//@ rustc-env:CARGO_CRATE_NAME=foo use std::pin::Pin; use std::future::Future; diff --git a/tests/ui/check-cfg/cargo-feature.rs b/tests/ui/check-cfg/cargo-feature.rs index a91068ca05ae7..ba451921d794d 100644 --- a/tests/ui/check-cfg/cargo-feature.rs +++ b/tests/ui/check-cfg/cargo-feature.rs @@ -4,7 +4,7 @@ // //@ check-pass //@ revisions: some none -//@ rustc-env:CARGO=/usr/bin/cargo +//@ rustc-env:CARGO_CRATE_NAME=foo //@ compile-flags: -Z unstable-options //@ [none]compile-flags: --check-cfg=cfg(feature,values()) //@ [some]compile-flags: --check-cfg=cfg(feature,values("bitcode")) diff --git a/tests/ui/check-cfg/diagnotics.rs b/tests/ui/check-cfg/diagnotics.rs index 54138d158904d..134bfcf8ef495 100644 --- a/tests/ui/check-cfg/diagnotics.rs +++ b/tests/ui/check-cfg/diagnotics.rs @@ -1,7 +1,7 @@ //@ check-pass //@ revisions: cargo rustc -//@ [rustc]unset-rustc-env:CARGO -//@ [cargo]rustc-env:CARGO=/usr/bin/cargo +//@ [rustc]unset-rustc-env:CARGO_CRATE_NAME +//@ [cargo]rustc-env:CARGO_CRATE_NAME=foo //@ compile-flags: --check-cfg=cfg(feature,values("foo")) --check-cfg=cfg(no_values) -Z unstable-options #[cfg(featur)] diff --git a/tests/ui/crate-loading/missing-std.rs b/tests/ui/crate-loading/missing-std.rs index ca9501cda3a91..aed8b0c530a69 100644 --- a/tests/ui/crate-loading/missing-std.rs +++ b/tests/ui/crate-loading/missing-std.rs @@ -1,6 +1,6 @@ //@ compile-flags: --target x86_64-unknown-uefi //@ needs-llvm-components: x86 -//@ rustc-env:CARGO=/usr/bin/cargo +//@ rustc-env:CARGO_CRATE_NAME=foo #![feature(no_core)] #![no_core] extern crate core;