From 81c068a7a6cd6c7efb85e372769ca6deb4a892ea Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 14 Feb 2024 13:31:50 +0300 Subject: [PATCH 01/14] install tools documentations Previously, we were trying to install all doc files under "share/doc/rust" which caused `rust-installer` tool to create backup files (*.old) due to filename conflicts. With this change, doc files is now installed under "share/doc/{package}", where {package} could be rustc, cargo, clippy, etc. Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/install.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/build_steps/install.rs b/src/bootstrap/src/core/build_steps/install.rs index 1c565e7f7cc06..0225f8f24a5ef 100644 --- a/src/bootstrap/src/core/build_steps/install.rs +++ b/src/bootstrap/src/core/build_steps/install.rs @@ -95,7 +95,7 @@ fn install_sh( } let datadir = prefix.join(default_path(&builder.config.datadir, "share")); - let docdir = prefix.join(default_path(&builder.config.docdir, "share/doc/rust")); + let docdir = prefix.join(default_path(&builder.config.docdir, &format!("share/doc/{package}"))); let mandir = prefix.join(default_path(&builder.config.mandir, "share/man")); let libdir = prefix.join(default_path(&builder.config.libdir, "lib")); let bindir = prefix.join(&builder.config.bindir); // Default in config.rs 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 02/14] 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 ec95d259e41a9da382cf0b35769f71fe23b49986 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Thu, 23 Nov 2023 10:25:25 +0000 Subject: [PATCH 03/14] simplify codegen for trivially droppable types --- library/alloc/src/collections/vec_deque/drain.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/collections/vec_deque/drain.rs b/library/alloc/src/collections/vec_deque/drain.rs index 0be274a3822d3..7d40ce65ad75b 100644 --- a/library/alloc/src/collections/vec_deque/drain.rs +++ b/library/alloc/src/collections/vec_deque/drain.rs @@ -96,8 +96,9 @@ impl Drop for Drain<'_, T, A> { struct DropGuard<'r, 'a, T, A: Allocator>(&'r mut Drain<'a, T, A>); impl<'r, 'a, T, A: Allocator> Drop for DropGuard<'r, 'a, T, A> { + #[inline] fn drop(&mut self) { - if self.0.remaining != 0 { + if mem::needs_drop::() && self.0.remaining != 0 { unsafe { // SAFETY: We just checked that `self.remaining != 0`. let (front, back) = self.0.as_slices(); @@ -158,7 +159,7 @@ impl Drop for Drain<'_, T, A> { } let guard = DropGuard(self); - if guard.0.remaining != 0 { + if mem::needs_drop::() && guard.0.remaining != 0 { unsafe { // SAFETY: We just checked that `self.remaining != 0`. let (front, back) = guard.0.as_slices(); From 500ef678770d9a0840e913ec02fab801ea455187 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Thu, 23 Nov 2023 10:41:35 +0000 Subject: [PATCH 04/14] reduce amount of math --- .../alloc/src/collections/vec_deque/drain.rs | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/library/alloc/src/collections/vec_deque/drain.rs b/library/alloc/src/collections/vec_deque/drain.rs index 7d40ce65ad75b..aa5cc0fb1ce2b 100644 --- a/library/alloc/src/collections/vec_deque/drain.rs +++ b/library/alloc/src/collections/vec_deque/drain.rs @@ -27,8 +27,8 @@ pub struct Drain< drain_len: usize, // index into the logical array, not the physical one (always lies in [0..deque.len)) idx: usize, - // number of elements after the drain range - tail_len: usize, + // number of elements remaining after dropping the drain + new_len: usize, remaining: usize, // Needed to make Drain covariant over T _marker: PhantomData<&'a T>, @@ -41,12 +41,12 @@ impl<'a, T, A: Allocator> Drain<'a, T, A> { drain_len: usize, ) -> Self { let orig_len = mem::replace(&mut deque.len, drain_start); - let tail_len = orig_len - drain_start - drain_len; + let new_len = orig_len - drain_len; Drain { deque: NonNull::from(deque), drain_len, idx: drain_start, - tail_len, + new_len, remaining: drain_len, _marker: PhantomData, } @@ -79,7 +79,7 @@ impl fmt::Debug for Drain<'_, T, A> { f.debug_tuple("Drain") .field(&self.drain_len) .field(&self.idx) - .field(&self.tail_len) + .field(&self.new_len) .field(&self.remaining) .finish() } @@ -111,18 +111,16 @@ impl Drop for Drain<'_, T, A> { let drain_start = source_deque.len(); let drain_len = self.0.drain_len; - let drain_end = drain_start + drain_len; - - let orig_len = self.0.tail_len + drain_end; + let new_len = self.0.new_len; if T::IS_ZST { // no need to copy around any memory if T is a ZST - source_deque.len = orig_len - drain_len; + source_deque.len = new_len; return; } let head_len = drain_start; - let tail_len = self.0.tail_len; + let tail_len = new_len - head_len; match (head_len, tail_len) { (0, 0) => { @@ -131,10 +129,10 @@ impl Drop for Drain<'_, T, A> { } (0, _) => { source_deque.head = source_deque.to_physical_idx(drain_len); - source_deque.len = orig_len - drain_len; + source_deque.len = new_len; } (_, 0) => { - source_deque.len = orig_len - drain_len; + source_deque.len = new_len; } _ => unsafe { if head_len <= tail_len { @@ -144,14 +142,14 @@ impl Drop for Drain<'_, T, A> { head_len, ); source_deque.head = source_deque.to_physical_idx(drain_len); - source_deque.len = orig_len - drain_len; + source_deque.len = new_len; } else { source_deque.wrap_copy( source_deque.to_physical_idx(head_len + drain_len), source_deque.to_physical_idx(head_len), tail_len, ); - source_deque.len = orig_len - drain_len; + source_deque.len = new_len; } }, } From 1e3849aed010c3ff2a4c7b7b644ca67d087ab4c1 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Thu, 23 Nov 2023 16:03:21 +0000 Subject: [PATCH 05/14] reduce branchiness --- .../alloc/src/collections/vec_deque/drain.rs | 53 ++++++++----------- 1 file changed, 22 insertions(+), 31 deletions(-) diff --git a/library/alloc/src/collections/vec_deque/drain.rs b/library/alloc/src/collections/vec_deque/drain.rs index aa5cc0fb1ce2b..7a413ddf971d7 100644 --- a/library/alloc/src/collections/vec_deque/drain.rs +++ b/library/alloc/src/collections/vec_deque/drain.rs @@ -109,7 +109,6 @@ impl Drop for Drain<'_, T, A> { let source_deque = unsafe { self.0.deque.as_mut() }; - let drain_start = source_deque.len(); let drain_len = self.0.drain_len; let new_len = self.0.new_len; @@ -119,40 +118,32 @@ impl Drop for Drain<'_, T, A> { return; } - let head_len = drain_start; + let head_len = source_deque.len(); let tail_len = new_len - head_len; - match (head_len, tail_len) { - (0, 0) => { - source_deque.head = 0; - source_deque.len = 0; - } - (0, _) => { - source_deque.head = source_deque.to_physical_idx(drain_len); - source_deque.len = new_len; - } - (_, 0) => { - source_deque.len = new_len; + if head_len != 0 && tail_len != 0 { + let (src, dst, len); + if head_len < tail_len { + src = source_deque.head; + dst = source_deque.to_physical_idx(drain_len); + len = head_len; + } else { + src = source_deque.to_physical_idx(head_len + drain_len); + dst = source_deque.to_physical_idx(head_len); + len = tail_len; + }; + + unsafe { + source_deque.wrap_copy(src, dst, len); } - _ => unsafe { - if head_len <= tail_len { - source_deque.wrap_copy( - source_deque.head, - source_deque.to_physical_idx(drain_len), - head_len, - ); - source_deque.head = source_deque.to_physical_idx(drain_len); - source_deque.len = new_len; - } else { - source_deque.wrap_copy( - source_deque.to_physical_idx(head_len + drain_len), - source_deque.to_physical_idx(head_len), - tail_len, - ); - source_deque.len = new_len; - } - }, } + + if new_len == 0 { + source_deque.head = 0; + } else if head_len < tail_len { + source_deque.head = source_deque.to_physical_idx(drain_len); + } + source_deque.len = new_len; } } From b6702939f72c6c55a74fc5f343d34c01e3d8521b Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Thu, 23 Nov 2023 23:31:19 +0100 Subject: [PATCH 06/14] outline large copies --- .../alloc/src/collections/vec_deque/drain.rs | 27 ++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/library/alloc/src/collections/vec_deque/drain.rs b/library/alloc/src/collections/vec_deque/drain.rs index 7a413ddf971d7..bf8c9b2d47aed 100644 --- a/library/alloc/src/collections/vec_deque/drain.rs +++ b/library/alloc/src/collections/vec_deque/drain.rs @@ -121,7 +121,27 @@ impl Drop for Drain<'_, T, A> { let head_len = source_deque.len(); let tail_len = new_len - head_len; + // When draining at the front (`.drain(..n)`) or at the back (`.drain(n..)`), + // we don't need to copy any data. + // Outlining this function helps LLVM to eliminate the copies in these cases. if head_len != 0 && tail_len != 0 { + copy_data(source_deque, drain_len, head_len, tail_len); + } + + if new_len == 0 { + source_deque.head = 0; + } else if head_len < tail_len { + source_deque.head = source_deque.to_physical_idx(drain_len); + } + source_deque.len = new_len; + + #[cold] + fn copy_data( + source_deque: &mut VecDeque, + drain_len: usize, + head_len: usize, + tail_len: usize, + ) { let (src, dst, len); if head_len < tail_len { src = source_deque.head; @@ -137,13 +157,6 @@ impl Drop for Drain<'_, T, A> { source_deque.wrap_copy(src, dst, len); } } - - if new_len == 0 { - source_deque.head = 0; - } else if head_len < tail_len { - source_deque.head = source_deque.to_physical_idx(drain_len); - } - source_deque.len = new_len; } } From 8f259ade66fa051b7918412a83f2fb8ef3ded92f Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Thu, 23 Nov 2023 23:11:22 +0100 Subject: [PATCH 07/14] add codegen test --- tests/codegen/vecdeque-drain.rs | 69 +++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 tests/codegen/vecdeque-drain.rs diff --git a/tests/codegen/vecdeque-drain.rs b/tests/codegen/vecdeque-drain.rs new file mode 100644 index 0000000000000..f8263c6957244 --- /dev/null +++ b/tests/codegen/vecdeque-drain.rs @@ -0,0 +1,69 @@ +// Check that draining at the front or back doesn't copy memory. + +// compile-flags: -O +// ignore-debug: the debug assertions get in the way + +#![crate_type = "lib"] + +use std::collections::VecDeque; + +// CHECK-LABEL: @clear +// CHECK-NOT: call +// CHECK-NOT: br +// CHECK: getelementptr inbounds +// CHECK-NEXT: {{call void @llvm.memset|store}} +// CHECK-NEXT: ret void +#[no_mangle] +pub fn clear(v: &mut VecDeque) { + v.drain(..); +} + +// CHECK-LABEL: @truncate +// CHECK-NOT: call +// CHECK: br +// CHECK-NOT: call +// CHECK: br +// CHECK-NOT: call +// CHECK: br +// CHECK-NOT: call +// CHECK: br +// CHECK-NOT: call +// CHECK-NOT: br +// CHECK: ret void +#[no_mangle] +pub fn truncate(v: &mut VecDeque, n: usize) { + if n < v.len() { + v.drain(n..); + } +} + +// CHECK-LABEL: @advance +// CHECK-NOT: call +// CHECK: br +// CHECK-NOT: call +// CHECK: br +// CHECK-NOT: call +// CHECK: br +// CHECK-NOT: call +// CHECK: br +// CHECK-NOT: call +// CHECK: br +// CHECK-NOT: call +// CHECK-NOT: br +// CHECK: ret void +#[no_mangle] +pub fn advance(v: &mut VecDeque, n: usize) { + if n < v.len() { + v.drain(..n); + } else { + v.clear(); + } +} + +// CHECK-LABEL: @remove +// CHECK: call +// CHECK: ret void +#[no_mangle] +pub fn remove(v: &mut VecDeque, a: usize, b: usize) { + v.drain(a..b); +} From 5d989770f285b6680a2540502fbfd0b4b21b16b3 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Sun, 11 Feb 2024 17:13:23 +0100 Subject: [PATCH 08/14] address review comments --- .../alloc/src/collections/vec_deque/drain.rs | 143 ++++++++++++------ 1 file changed, 99 insertions(+), 44 deletions(-) diff --git a/library/alloc/src/collections/vec_deque/drain.rs b/library/alloc/src/collections/vec_deque/drain.rs index bf8c9b2d47aed..1373e60149274 100644 --- a/library/alloc/src/collections/vec_deque/drain.rs +++ b/library/alloc/src/collections/vec_deque/drain.rs @@ -95,6 +95,22 @@ impl Drop for Drain<'_, T, A> { fn drop(&mut self) { struct DropGuard<'r, 'a, T, A: Allocator>(&'r mut Drain<'a, T, A>); + let guard = DropGuard(self); + + if mem::needs_drop::() && guard.0.remaining != 0 { + unsafe { + // SAFETY: We just checked that `self.remaining != 0`. + let (front, back) = guard.0.as_slices(); + // since idx is a logical index, we don't need to worry about wrapping. + guard.0.idx += front.len(); + guard.0.remaining -= front.len(); + ptr::drop_in_place(front); + guard.0.remaining = 0; + ptr::drop_in_place(back); + } + } + + // Dropping `guard` handles moving the remaining elements into place. impl<'r, 'a, T, A: Allocator> Drop for DropGuard<'r, 'a, T, A> { #[inline] fn drop(&mut self) { @@ -118,63 +134,102 @@ impl Drop for Drain<'_, T, A> { return; } - let head_len = source_deque.len(); - let tail_len = new_len - head_len; + let head_len = source_deque.len; // #elements in front of the drain + let tail_len = new_len - head_len; // #elements behind the drain + + // Next, we will fill the hole left by the drain with as few writes as possible. + // The code below handles the following control flow and reduces the amount of + // branches under the assumption that `head_len == 0 || tail_len == 0`, i.e. + // draining at the front or at the back of the dequeue is especially common. + // + // H = "head index" = `deque.head` + // h = elements in front of the drain + // d = elements in the drain + // t = elements behind the drain + // + // Note that the buffer may wrap at any point and the wrapping is handled by + // `wrap_copy` and `to_physical_idx`. + // + // Case 1: if `head_len == 0 && tail_len == 0` + // Everything was drained, reset the head index back to 0. + // H + // [ . . . . . d d d d . . . . . ] + // H + // [ . . . . . . . . . . . . . . ] + // + // Case 2: else if `tail_len == 0` + // Don't move data or the head index. + // H + // [ . . . h h h h d d d d . . . ] + // H + // [ . . . h h h h . . . . . . . ] + // + // Case 3: else if `head_len == 0` + // Don't move data, but move the head index. + // H + // [ . . . d d d d t t t t . . . ] + // H + // [ . . . . . . . t t t t . . . ] + // + // Case 4: else if `tail_len <= head_len` + // Move data, but not the head index. + // H + // [ . . h h h h d d d d t t . . ] + // H + // [ . . h h h h t t . . . . . . ] + // + // Case 5: else + // Move data and the head index. + // H + // [ . . h h d d d d t t t t . . ] + // H + // [ . . . . . . h h t t t t . . ] // When draining at the front (`.drain(..n)`) or at the back (`.drain(n..)`), - // we don't need to copy any data. - // Outlining this function helps LLVM to eliminate the copies in these cases. + // we don't need to copy any data. The number of elements copied would be 0. if head_len != 0 && tail_len != 0 { - copy_data(source_deque, drain_len, head_len, tail_len); + join_head_and_tail_wrapping(source_deque, drain_len, head_len, tail_len); + // Marking this function as cold helps LLVM to eliminate it entirely if + // this branch is never taken. + // We use `#[cold]` instead of `#[inline(never)]`, because inlining this + // function into the general case (`.drain(n..m)`) is fine. + // See `tests/codegen/vecdeque-drain.rs` for a test. + #[cold] + fn join_head_and_tail_wrapping( + source_deque: &mut VecDeque, + drain_len: usize, + head_len: usize, + tail_len: usize, + ) { + // Pick whether to move the head or the tail here. + let (src, dst, len); + if head_len < tail_len { + src = source_deque.head; + dst = source_deque.to_physical_idx(drain_len); + len = head_len; + } else { + src = source_deque.to_physical_idx(head_len + drain_len); + dst = source_deque.to_physical_idx(head_len); + len = tail_len; + }; + + unsafe { + source_deque.wrap_copy(src, dst, len); + } + } } if new_len == 0 { + // Special case: If the entire dequeue was drained, reset the head back to 0, + // like `.clear()` does. source_deque.head = 0; } else if head_len < tail_len { + // If we moved the head above, then we need to adjust the head index here. source_deque.head = source_deque.to_physical_idx(drain_len); } source_deque.len = new_len; - - #[cold] - fn copy_data( - source_deque: &mut VecDeque, - drain_len: usize, - head_len: usize, - tail_len: usize, - ) { - let (src, dst, len); - if head_len < tail_len { - src = source_deque.head; - dst = source_deque.to_physical_idx(drain_len); - len = head_len; - } else { - src = source_deque.to_physical_idx(head_len + drain_len); - dst = source_deque.to_physical_idx(head_len); - len = tail_len; - }; - - unsafe { - source_deque.wrap_copy(src, dst, len); - } - } - } - } - - let guard = DropGuard(self); - if mem::needs_drop::() && guard.0.remaining != 0 { - unsafe { - // SAFETY: We just checked that `self.remaining != 0`. - let (front, back) = guard.0.as_slices(); - // since idx is a logical index, we don't need to worry about wrapping. - guard.0.idx += front.len(); - guard.0.remaining -= front.len(); - ptr::drop_in_place(front); - guard.0.remaining = 0; - ptr::drop_in_place(back); } } - - // Dropping `guard` handles moving the remaining elements into place. } } 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 09/14] 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 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 10/14] 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 11/14] 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 12/14] 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 13/14] 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; From 6ff9c6f96b2ddd9fd83fe32b7089d77d61f40508 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 14 Feb 2024 13:52:45 +0300 Subject: [PATCH 14/14] don't use entire sysroot binary path for rustc tarballs Previously, we used the entire sysroot binary path to prepare rustc tarballs. Since we also copy tool binaries to the sysroot binary path, installing rustc and tools with `x install` results in attempting to install the same binaries more than once. This causes rust-installer to create backup files (*.old) due to file conflicts. This change fixes that. Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/dist.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/dist.rs b/src/bootstrap/src/core/build_steps/dist.rs index f50026368dabd..1339b2a926613 100644 --- a/src/bootstrap/src/core/build_steps/dist.rs +++ b/src/bootstrap/src/core/build_steps/dist.rs @@ -398,10 +398,11 @@ impl Step for Rustc { let host = compiler.host; let src = builder.sysroot(compiler); - // Copy rustc/rustdoc binaries + // Copy rustc binary + builder.install(&src.join("bin").join(exe("rustc", host)), &image.join("bin"), 0o755); t!(fs::create_dir_all(image.join("bin"))); - builder.cp_r(&src.join("bin"), &image.join("bin")); + // If enabled, copy rustdoc binary if builder .config .tools