From 8bdf5c3de6c6e4e01f7f6241cd0f2a606c7486df Mon Sep 17 00:00:00 2001 From: Badel2 <2badel2@gmail.com> Date: Wed, 5 Jan 2022 22:42:21 +0100 Subject: [PATCH 01/24] Implement panic::update_hook --- library/alloc/tests/lib.rs | 1 + library/alloc/tests/slice.rs | 13 ++-- library/proc_macro/src/bridge/client.rs | 21 ++++--- library/proc_macro/src/lib.rs | 1 + library/std/src/panic.rs | 3 + library/std/src/panicking.rs | 63 +++++++++++++++++++ .../ui/panics/panic-while-updating-hook.rs | 16 +++++ 7 files changed, 102 insertions(+), 16 deletions(-) create mode 100644 src/test/ui/panics/panic-while-updating-hook.rs diff --git a/library/alloc/tests/lib.rs b/library/alloc/tests/lib.rs index eec24a5c3f7e6..7b8eeb90b5a80 100644 --- a/library/alloc/tests/lib.rs +++ b/library/alloc/tests/lib.rs @@ -38,6 +38,7 @@ #![feature(const_trait_impl)] #![feature(const_str_from_utf8)] #![feature(nonnull_slice_from_raw_parts)] +#![feature(panic_update_hook)] use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; diff --git a/library/alloc/tests/slice.rs b/library/alloc/tests/slice.rs index 18ea6a2141377..a02f7b1f2774b 100644 --- a/library/alloc/tests/slice.rs +++ b/library/alloc/tests/slice.rs @@ -1783,12 +1783,13 @@ thread_local!(static SILENCE_PANIC: Cell<bool> = Cell::new(false)); #[test] #[cfg_attr(target_os = "emscripten", ignore)] // no threads fn panic_safe() { - let prev = panic::take_hook(); - panic::set_hook(Box::new(move |info| { - if !SILENCE_PANIC.with(|s| s.get()) { - prev(info); - } - })); + panic::update_hook(|prev| { + Box::new(move |info| { + if !SILENCE_PANIC.with(|s| s.get()) { + prev(info); + } + }) + }); let mut rng = thread_rng(); diff --git a/library/proc_macro/src/bridge/client.rs b/library/proc_macro/src/bridge/client.rs index 83a2ac6f0d4f1..5ff7bbf6f96f1 100644 --- a/library/proc_macro/src/bridge/client.rs +++ b/library/proc_macro/src/bridge/client.rs @@ -310,16 +310,17 @@ impl Bridge<'_> { // NB. the server can't do this because it may use a different libstd. static HIDE_PANICS_DURING_EXPANSION: Once = Once::new(); HIDE_PANICS_DURING_EXPANSION.call_once(|| { - let prev = panic::take_hook(); - panic::set_hook(Box::new(move |info| { - let show = BridgeState::with(|state| match state { - BridgeState::NotConnected => true, - BridgeState::Connected(_) | BridgeState::InUse => force_show_panics, - }); - if show { - prev(info) - } - })); + panic::update_hook(|prev| { + Box::new(move |info| { + let show = BridgeState::with(|state| match state { + BridgeState::NotConnected => true, + BridgeState::Connected(_) | BridgeState::InUse => force_show_panics, + }); + if show { + prev(info) + } + }) + }); }); BRIDGE_STATE.with(|state| state.set(BridgeState::Connected(self), f)) diff --git a/library/proc_macro/src/lib.rs b/library/proc_macro/src/lib.rs index 69af598f91e1c..c5afca6d56a2d 100644 --- a/library/proc_macro/src/lib.rs +++ b/library/proc_macro/src/lib.rs @@ -30,6 +30,7 @@ #![feature(restricted_std)] #![feature(rustc_attrs)] #![feature(min_specialization)] +#![feature(panic_update_hook)] #![recursion_limit = "256"] #[unstable(feature = "proc_macro_internals", issue = "27812")] diff --git a/library/std/src/panic.rs b/library/std/src/panic.rs index c0605b2f4121c..02ecf2e3e822e 100644 --- a/library/std/src/panic.rs +++ b/library/std/src/panic.rs @@ -36,6 +36,9 @@ pub use core::panic::panic_2021; #[stable(feature = "panic_hooks", since = "1.10.0")] pub use crate::panicking::{set_hook, take_hook}; +#[unstable(feature = "panic_update_hook", issue = "92649")] +pub use crate::panicking::update_hook; + #[stable(feature = "panic_hooks", since = "1.10.0")] pub use core::panic::{Location, PanicInfo}; diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index 87854fe4f2970..cf970dccfc940 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -180,6 +180,69 @@ pub fn take_hook() -> Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send> { } } +/// Atomic combination of [`take_hook`] + [`set_hook`]. +/// +/// [`take_hook`]: ./fn.take_hook.html +/// [`set_hook`]: ./fn.set_hook.html +/// +/// # Panics +/// +/// Panics if called from a panicking thread. +/// +/// Panics if the provided closure calls any of the functions [`panic::take_hook`], +/// [`panic::set_hook`], or [`panic::update_hook`]. +/// +/// Note: if the provided closure panics, the panic will not be able to be handled, resulting in a +/// double panic that aborts the process with a generic error message. +/// +/// [`panic::take_hook`]: ./fn.take_hook.html +/// [`panic::set_hook`]: ./fn.set_hook.html +/// [`panic::update_hook`]: ./fn.update_hook.html +/// +/// # Examples +/// +/// The following will print the custom message, and then the normal output of panic. +/// +/// ```should_panic +/// #![feature(panic_update_hook)] +/// use std::panic; +/// +/// panic::update_hook(|prev| { +/// Box::new(move |panic_info| { +/// println!("Print custom message and execute panic handler as usual"); +/// prev(panic_info); +/// }) +/// }); +/// +/// panic!("Custom and then normal"); +/// ``` +#[unstable(feature = "panic_update_hook", issue = "92649")] +pub fn update_hook<F>(hook_fn: F) +where + F: FnOnce( + Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send>, + ) -> Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send>, +{ + if thread::panicking() { + panic!("cannot modify the panic hook from a panicking thread"); + } + + unsafe { + let guard = HOOK_LOCK.write(); + let old_hook = HOOK; + HOOK = Hook::Default; + + let hook_for_fn = match old_hook { + Hook::Default => Box::new(default_hook), + Hook::Custom(ptr) => Box::from_raw(ptr), + }; + + let hook = hook_fn(hook_for_fn); + HOOK = Hook::Custom(Box::into_raw(hook)); + drop(guard); + } +} + fn default_hook(info: &PanicInfo<'_>) { // If this is a double panic, make sure that we print a backtrace // for this panic. Otherwise only print it if logging is enabled. diff --git a/src/test/ui/panics/panic-while-updating-hook.rs b/src/test/ui/panics/panic-while-updating-hook.rs new file mode 100644 index 0000000000000..8c95f1b8b7840 --- /dev/null +++ b/src/test/ui/panics/panic-while-updating-hook.rs @@ -0,0 +1,16 @@ +// run-fail +// error-pattern: panicked while processing panic +#![allow(stable_features)] + +// ignore-emscripten no threads support + +#![feature(std_panic)] +#![feature(panic_update_hook)] + +use std::panic; + +fn main() { + panic::update_hook(|_prev| { + panic!("inside update_hook"); + }) +} From 8ef3ce866e2f20bdcc567e2f7012f81ce2e60298 Mon Sep 17 00:00:00 2001 From: Badel2 <2badel2@gmail.com> Date: Fri, 7 Jan 2022 17:04:33 +0100 Subject: [PATCH 02/24] Change panic::update_hook to simplify usage And to remove possibility of panics while changing the panic handler, because that resulted in a double panic. --- library/alloc/tests/slice.rs | 10 ++--- library/proc_macro/src/bridge/client.rs | 18 ++++---- library/std/src/panicking.rs | 45 ++++++++++--------- .../panics/panic-handler-chain-update-hook.rs | 36 +++++++++++++++ .../ui/panics/panic-while-updating-hook.rs | 16 ------- 5 files changed, 71 insertions(+), 54 deletions(-) create mode 100644 src/test/ui/panics/panic-handler-chain-update-hook.rs delete mode 100644 src/test/ui/panics/panic-while-updating-hook.rs diff --git a/library/alloc/tests/slice.rs b/library/alloc/tests/slice.rs index a02f7b1f2774b..b93d7938bc9a5 100644 --- a/library/alloc/tests/slice.rs +++ b/library/alloc/tests/slice.rs @@ -1783,12 +1783,10 @@ thread_local!(static SILENCE_PANIC: Cell<bool> = Cell::new(false)); #[test] #[cfg_attr(target_os = "emscripten", ignore)] // no threads fn panic_safe() { - panic::update_hook(|prev| { - Box::new(move |info| { - if !SILENCE_PANIC.with(|s| s.get()) { - prev(info); - } - }) + panic::update_hook(move |prev, info| { + if !SILENCE_PANIC.with(|s| s.get()) { + prev(info); + } }); let mut rng = thread_rng(); diff --git a/library/proc_macro/src/bridge/client.rs b/library/proc_macro/src/bridge/client.rs index 5ff7bbf6f96f1..9e9750eb8de40 100644 --- a/library/proc_macro/src/bridge/client.rs +++ b/library/proc_macro/src/bridge/client.rs @@ -310,16 +310,14 @@ impl Bridge<'_> { // NB. the server can't do this because it may use a different libstd. static HIDE_PANICS_DURING_EXPANSION: Once = Once::new(); HIDE_PANICS_DURING_EXPANSION.call_once(|| { - panic::update_hook(|prev| { - Box::new(move |info| { - let show = BridgeState::with(|state| match state { - BridgeState::NotConnected => true, - BridgeState::Connected(_) | BridgeState::InUse => force_show_panics, - }); - if show { - prev(info) - } - }) + panic::update_hook(move |prev, info| { + let show = BridgeState::with(|state| match state { + BridgeState::NotConnected => true, + BridgeState::Connected(_) | BridgeState::InUse => force_show_panics, + }); + if show { + prev(info) + } }); }); diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index cf970dccfc940..19040cb12e02a 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -76,6 +76,12 @@ enum Hook { Custom(*mut (dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send)), } +impl Hook { + fn custom(f: impl Fn(&PanicInfo<'_>) + 'static + Sync + Send) -> Self { + Self::Custom(Box::into_raw(Box::new(f))) + } +} + static HOOK_LOCK: StaticRWLock = StaticRWLock::new(); static mut HOOK: Hook = Hook::Default; @@ -180,7 +186,8 @@ pub fn take_hook() -> Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send> { } } -/// Atomic combination of [`take_hook`] + [`set_hook`]. +/// Atomic combination of [`take_hook`] and [`set_hook`]. Use this to replace the panic handler with +/// a new panic handler that does something and then executes the old handler. /// /// [`take_hook`]: ./fn.take_hook.html /// [`set_hook`]: ./fn.set_hook.html @@ -189,16 +196,6 @@ pub fn take_hook() -> Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send> { /// /// Panics if called from a panicking thread. /// -/// Panics if the provided closure calls any of the functions [`panic::take_hook`], -/// [`panic::set_hook`], or [`panic::update_hook`]. -/// -/// Note: if the provided closure panics, the panic will not be able to be handled, resulting in a -/// double panic that aborts the process with a generic error message. -/// -/// [`panic::take_hook`]: ./fn.take_hook.html -/// [`panic::set_hook`]: ./fn.set_hook.html -/// [`panic::update_hook`]: ./fn.update_hook.html -/// /// # Examples /// /// The following will print the custom message, and then the normal output of panic. @@ -207,11 +204,15 @@ pub fn take_hook() -> Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send> { /// #![feature(panic_update_hook)] /// use std::panic; /// -/// panic::update_hook(|prev| { -/// Box::new(move |panic_info| { -/// println!("Print custom message and execute panic handler as usual"); -/// prev(panic_info); -/// }) +/// // Equivalent to +/// // let prev = panic::take_hook(); +/// // panic::set_hook(move |info| { +/// // println!("..."); +/// // prev(info); +/// // ); +/// panic::update_hook(move |prev, info| { +/// println!("Print custom message and execute panic handler as usual"); +/// prev(info); /// }); /// /// panic!("Custom and then normal"); @@ -219,9 +220,10 @@ pub fn take_hook() -> Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send> { #[unstable(feature = "panic_update_hook", issue = "92649")] pub fn update_hook<F>(hook_fn: F) where - F: FnOnce( - Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send>, - ) -> Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send>, + F: Fn(&(dyn Fn(&PanicInfo<'_>) + Send + Sync + 'static), &PanicInfo<'_>) + + Sync + + Send + + 'static, { if thread::panicking() { panic!("cannot modify the panic hook from a panicking thread"); @@ -232,13 +234,12 @@ where let old_hook = HOOK; HOOK = Hook::Default; - let hook_for_fn = match old_hook { + let prev = match old_hook { Hook::Default => Box::new(default_hook), Hook::Custom(ptr) => Box::from_raw(ptr), }; - let hook = hook_fn(hook_for_fn); - HOOK = Hook::Custom(Box::into_raw(hook)); + HOOK = Hook::custom(move |info| hook_fn(&prev, info)); drop(guard); } } diff --git a/src/test/ui/panics/panic-handler-chain-update-hook.rs b/src/test/ui/panics/panic-handler-chain-update-hook.rs new file mode 100644 index 0000000000000..4dd08ba4ad4e2 --- /dev/null +++ b/src/test/ui/panics/panic-handler-chain-update-hook.rs @@ -0,0 +1,36 @@ +// run-pass +// needs-unwind +#![allow(stable_features)] + +// ignore-emscripten no threads support + +#![feature(std_panic)] +#![feature(panic_update_hook)] + +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::panic; +use std::thread; + +static A: AtomicUsize = AtomicUsize::new(0); +static B: AtomicUsize = AtomicUsize::new(0); +static C: AtomicUsize = AtomicUsize::new(0); + +fn main() { + panic::set_hook(Box::new(|_| { A.fetch_add(1, Ordering::SeqCst); })); + panic::update_hook(|prev, info| { + B.fetch_add(1, Ordering::SeqCst); + prev(info); + }); + panic::update_hook(|prev, info| { + C.fetch_add(1, Ordering::SeqCst); + prev(info); + }); + + let _ = thread::spawn(|| { + panic!(); + }).join(); + + assert_eq!(1, A.load(Ordering::SeqCst)); + assert_eq!(1, B.load(Ordering::SeqCst)); + assert_eq!(1, C.load(Ordering::SeqCst)); +} diff --git a/src/test/ui/panics/panic-while-updating-hook.rs b/src/test/ui/panics/panic-while-updating-hook.rs deleted file mode 100644 index 8c95f1b8b7840..0000000000000 --- a/src/test/ui/panics/panic-while-updating-hook.rs +++ /dev/null @@ -1,16 +0,0 @@ -// run-fail -// error-pattern: panicked while processing panic -#![allow(stable_features)] - -// ignore-emscripten no threads support - -#![feature(std_panic)] -#![feature(panic_update_hook)] - -use std::panic; - -fn main() { - panic::update_hook(|_prev| { - panic!("inside update_hook"); - }) -} From 0c58586c9cbdd4db62c9c127ba89fd4d1d53acd7 Mon Sep 17 00:00:00 2001 From: Badel2 <2badel2@gmail.com> Date: Fri, 7 Jan 2022 23:34:45 +0100 Subject: [PATCH 03/24] Add safety comments to panic::(set/take/update)_hook --- library/std/src/panicking.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index 19040cb12e02a..44f573297eed1 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -124,6 +124,11 @@ pub fn set_hook(hook: Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send>) { panic!("cannot modify the panic hook from a panicking thread"); } + // SAFETY: + // + // - `HOOK` can only be modified while holding write access to `HOOK_LOCK`. + // - The argument of `Box::from_raw` is always a valid pointer that was created using + // `Box::into_raw`. unsafe { let guard = HOOK_LOCK.write(); let old_hook = HOOK; @@ -173,6 +178,11 @@ pub fn take_hook() -> Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send> { panic!("cannot modify the panic hook from a panicking thread"); } + // SAFETY: + // + // - `HOOK` can only be modified while holding write access to `HOOK_LOCK`. + // - The argument of `Box::from_raw` is always a valid pointer that was created using + // `Box::into_raw`. unsafe { let guard = HOOK_LOCK.write(); let hook = HOOK; @@ -229,6 +239,11 @@ where panic!("cannot modify the panic hook from a panicking thread"); } + // SAFETY: + // + // - `HOOK` can only be modified while holding write access to `HOOK_LOCK`. + // - The argument of `Box::from_raw` is always a valid pointer that was created using + // `Box::into_raw`. unsafe { let guard = HOOK_LOCK.write(); let old_hook = HOOK; From 4c3e330a8c023af20beb23a3a46b5d6d77a62839 Mon Sep 17 00:00:00 2001 From: Mahdi Dibaiee <mdibaiee@pm.me> Date: Fri, 7 Jan 2022 11:38:16 +0000 Subject: [PATCH 04/24] feat: pass_by_value lint attribute Useful for thin wrapper attributes that are best passed as value instead of reference. --- compiler/rustc_feature/src/builtin_attrs.rs | 5 + compiler/rustc_lint/src/internal.rs | 33 +----- compiler/rustc_lint/src/lib.rs | 6 +- compiler/rustc_lint/src/pass_by_value.rs | 103 ++++++++++++++++++ compiler/rustc_middle/src/ty/context.rs | 1 + compiler/rustc_middle/src/ty/mod.rs | 1 + compiler/rustc_passes/src/check_attr.rs | 24 ++++ compiler/rustc_span/src/symbol.rs | 1 + ...ss_ty_by_ref.rs => rustc_pass_by_value.rs} | 2 +- ..._ref.stderr => rustc_pass_by_value.stderr} | 30 ++--- ...ef_self.rs => rustc_pass_by_value_self.rs} | 5 +- ...stderr => rustc_pass_by_value_self.stderr} | 10 +- 12 files changed, 165 insertions(+), 56 deletions(-) create mode 100644 compiler/rustc_lint/src/pass_by_value.rs rename src/test/ui-fulldeps/internal-lints/{pass_ty_by_ref.rs => rustc_pass_by_value.rs} (97%) rename src/test/ui-fulldeps/internal-lints/{pass_ty_by_ref.stderr => rustc_pass_by_value.stderr} (80%) rename src/test/ui-fulldeps/internal-lints/{pass_ty_by_ref_self.rs => rustc_pass_by_value_self.rs} (90%) rename src/test/ui-fulldeps/internal-lints/{pass_ty_by_ref_self.stderr => rustc_pass_by_value_self.stderr} (65%) diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index f25b2d8f566c0..88bf81864b23f 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -623,6 +623,11 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ lang, Normal, template!(NameValueStr: "name"), DuplicatesOk, lang_items, "language items are subject to change", ), + rustc_attr!( + rustc_pass_by_value, Normal, + template!(Word, NameValueStr: "reason"), WarnFollowing, + "#[rustc_pass_by_value] is used to mark types that must be passed by value instead of reference." + ), BuiltinAttribute { name: sym::rustc_diagnostic_item, type_: Normal, diff --git a/compiler/rustc_lint/src/internal.rs b/compiler/rustc_lint/src/internal.rs index c64a67b6b9f1b..7353cd6b876b9 100644 --- a/compiler/rustc_lint/src/internal.rs +++ b/compiler/rustc_lint/src/internal.rs @@ -5,10 +5,7 @@ use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext} use rustc_ast as ast; use rustc_errors::Applicability; use rustc_hir::def::Res; -use rustc_hir::{ - GenericArg, HirId, Item, ItemKind, MutTy, Mutability, Node, Path, PathSegment, QPath, Ty, - TyKind, -}; +use rustc_hir::{GenericArg, HirId, Item, ItemKind, Node, Path, PathSegment, QPath, Ty, TyKind}; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::hygiene::{ExpnKind, MacroKind}; @@ -58,13 +55,6 @@ declare_tool_lint! { report_in_external_macro: true } -declare_tool_lint! { - pub rustc::TY_PASS_BY_REFERENCE, - Allow, - "passing `Ty` or `TyCtxt` by reference", - report_in_external_macro: true -} - declare_tool_lint! { pub rustc::USAGE_OF_QUALIFIED_TY, Allow, @@ -74,7 +64,6 @@ declare_tool_lint! { declare_lint_pass!(TyTyKind => [ USAGE_OF_TY_TYKIND, - TY_PASS_BY_REFERENCE, USAGE_OF_QUALIFIED_TY, ]); @@ -131,26 +120,6 @@ impl<'tcx> LateLintPass<'tcx> for TyTyKind { } } } - TyKind::Rptr(_, MutTy { ty: inner_ty, mutbl: Mutability::Not }) => { - if let Some(impl_did) = cx.tcx.impl_of_method(ty.hir_id.owner.to_def_id()) { - if cx.tcx.impl_trait_ref(impl_did).is_some() { - return; - } - } - if let Some(t) = is_ty_or_ty_ctxt(cx, &inner_ty) { - cx.struct_span_lint(TY_PASS_BY_REFERENCE, ty.span, |lint| { - lint.build(&format!("passing `{}` by reference", t)) - .span_suggestion( - ty.span, - "try passing by value", - t, - // Changing type of function argument - Applicability::MaybeIncorrect, - ) - .emit(); - }) - } - } _ => {} } } diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index c7823032b0c23..3b95a2487baed 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -56,6 +56,7 @@ mod non_ascii_idents; mod non_fmt_panic; mod nonstandard_style; mod noop_method_call; +mod pass_by_value; mod passes; mod redundant_semicolon; mod traits; @@ -85,6 +86,7 @@ use non_ascii_idents::*; use non_fmt_panic::NonPanicFmt; use nonstandard_style::*; use noop_method_call::*; +use pass_by_value::*; use redundant_semicolon::*; use traits::*; use types::*; @@ -489,6 +491,8 @@ fn register_internals(store: &mut LintStore) { store.register_late_pass(|| Box::new(ExistingDocKeyword)); store.register_lints(&TyTyKind::get_lints()); store.register_late_pass(|| Box::new(TyTyKind)); + store.register_lints(&PassByValue::get_lints()); + store.register_late_pass(|| Box::new(PassByValue)); store.register_group( false, "rustc::internal", @@ -496,8 +500,8 @@ fn register_internals(store: &mut LintStore) { vec![ LintId::of(DEFAULT_HASH_TYPES), LintId::of(USAGE_OF_TY_TYKIND), + LintId::of(PASS_BY_VALUE), LintId::of(LINT_PASS_IMPL_WITHOUT_MACRO), - LintId::of(TY_PASS_BY_REFERENCE), LintId::of(USAGE_OF_QUALIFIED_TY), LintId::of(EXISTING_DOC_KEYWORD), ], diff --git a/compiler/rustc_lint/src/pass_by_value.rs b/compiler/rustc_lint/src/pass_by_value.rs new file mode 100644 index 0000000000000..0bfa2a673c2c4 --- /dev/null +++ b/compiler/rustc_lint/src/pass_by_value.rs @@ -0,0 +1,103 @@ +use crate::{LateContext, LateLintPass, LintContext}; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_hir::def::Res; +use rustc_hir::def_id::DefId; +use rustc_hir::{GenericArg, PathSegment, QPath, TyKind}; +use rustc_middle::ty; +use rustc_span::symbol::sym; + +declare_tool_lint! { + /// The `rustc_pass_by_value` lint marks a type with `#[rustc_pass_by_value]` requiring it to always be passed by value. + /// This is usually used for types that are thin wrappers around references, so there is no benefit to an extra + /// layer of indirection. (Example: `Ty` which is a reference to a `TyS`) + pub rustc::PASS_BY_VALUE, + Warn, + "pass by reference of a type flagged as `#[rustc_pass_by_value]`", + report_in_external_macro: true +} + +declare_lint_pass!(PassByValue => [PASS_BY_VALUE]); + +impl<'tcx> LateLintPass<'tcx> for PassByValue { + fn check_ty(&mut self, cx: &LateContext<'_>, ty: &'tcx hir::Ty<'tcx>) { + match &ty.kind { + TyKind::Rptr(_, hir::MutTy { ty: inner_ty, mutbl: hir::Mutability::Not }) => { + if let Some(impl_did) = cx.tcx.impl_of_method(ty.hir_id.owner.to_def_id()) { + if cx.tcx.impl_trait_ref(impl_did).is_some() { + return; + } + } + if let Some(t) = path_for_pass_by_value(cx, &inner_ty) { + cx.struct_span_lint(PASS_BY_VALUE, ty.span, |lint| { + lint.build(&format!("passing `{}` by reference", t)) + .span_suggestion( + ty.span, + "try passing by value", + t, + // Changing type of function argument + Applicability::MaybeIncorrect, + ) + .emit(); + }) + } + } + _ => {} + } + } +} + +fn path_for_pass_by_value(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> Option<String> { + if let TyKind::Path(QPath::Resolved(_, path)) = &ty.kind { + match path.res { + Res::Def(_, def_id) if has_pass_by_value_attr(cx, def_id) => { + if let Some(name) = cx.tcx.get_diagnostic_name(def_id) { + return Some(format!("{}{}", name, gen_args(path.segments.last().unwrap()))); + } + } + Res::SelfTy(None, Some((did, _))) => { + if let ty::Adt(adt, substs) = cx.tcx.type_of(did).kind() { + if has_pass_by_value_attr(cx, adt.did) { + if let Some(name) = cx.tcx.get_diagnostic_name(adt.did) { + return Some(format!("{}<{}>", name, substs[0])); + } + } + } + } + _ => (), + } + } + + None +} + +fn has_pass_by_value_attr(cx: &LateContext<'_>, def_id: DefId) -> bool { + for attr in cx.tcx.get_attrs(def_id).iter() { + if attr.has_name(sym::rustc_pass_by_value) { + return true; + } + } + false +} + +fn gen_args(segment: &PathSegment<'_>) -> String { + if let Some(args) = &segment.args { + let lifetimes = args + .args + .iter() + .filter_map(|arg| { + if let GenericArg::Lifetime(lt) = arg { + Some(lt.name.ident().to_string()) + } else { + None + } + }) + .collect::<Vec<_>>(); + + if !lifetimes.is_empty() { + return format!("<{}>", lifetimes.join(", ")); + } + } + + String::new() +} diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index dd571e29bf695..ab85f104ce398 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -961,6 +961,7 @@ pub struct FreeRegionInfo { /// [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/ty.html #[derive(Copy, Clone)] #[rustc_diagnostic_item = "TyCtxt"] +#[cfg_attr(not(bootstrap), rustc_pass_by_value)] pub struct TyCtxt<'tcx> { gcx: &'tcx GlobalCtxt<'tcx>, } diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 78ccfbd5e8cdc..365d4c4aabaad 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -462,6 +462,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TyS<'tcx> { } #[rustc_diagnostic_item = "Ty"] +#[cfg_attr(not(bootstrap), rustc_pass_by_value)] pub type Ty<'tcx> = &'tcx TyS<'tcx>; impl ty::EarlyBoundRegion { diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index d7b00699491d4..2febb2e56ecf8 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -114,6 +114,7 @@ impl CheckAttrVisitor<'_> { } sym::must_not_suspend => self.check_must_not_suspend(&attr, span, target), sym::must_use => self.check_must_use(hir_id, &attr, span, target), + sym::rustc_pass_by_value => self.check_pass_by_value(&attr, span, target), sym::rustc_const_unstable | sym::rustc_const_stable | sym::unstable @@ -1066,6 +1067,29 @@ impl CheckAttrVisitor<'_> { is_valid } + /// Warns against some misuses of `#[pass_by_value]` + fn check_pass_by_value(&self, attr: &Attribute, span: &Span, target: Target) -> bool { + match target { + Target::Struct + | Target::Enum + | Target::Union + | Target::Trait + | Target::TraitAlias + | Target::TyAlias => true, + _ => { + self.tcx + .sess + .struct_span_err( + attr.span, + "`pass_by_value` attribute should be applied to a struct, enum, trait or type alias.", + ) + .span_label(*span, "is not a struct, enum, trait or type alias") + .emit(); + false + } + } + } + /// Warns against some misuses of `#[must_use]` fn check_must_use( &self, diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 84cf8878af809..b1d868fbb88f6 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1143,6 +1143,7 @@ symbols! { rustc_paren_sugar, rustc_partition_codegened, rustc_partition_reused, + rustc_pass_by_value, rustc_peek, rustc_peek_definite_init, rustc_peek_liveness, diff --git a/src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs similarity index 97% rename from src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.rs rename to src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs index e0fdbaeac3069..783019d894513 100644 --- a/src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs @@ -1,7 +1,7 @@ // compile-flags: -Z unstable-options #![feature(rustc_private)] -#![deny(rustc::ty_pass_by_reference)] +#![deny(rustc::pass_by_value)] #![allow(unused)] extern crate rustc_middle; diff --git a/src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr similarity index 80% rename from src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.stderr rename to src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr index 2751a37f7419d..5fbde93878931 100644 --- a/src/test/ui-fulldeps/internal-lints/pass_ty_by_ref.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr @@ -1,77 +1,77 @@ error: passing `Ty<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:13:13 + --> $DIR/rustc_pass_by_value.rs:13:13 | LL | ty_ref: &Ty<'_>, | ^^^^^^^ help: try passing by value: `Ty<'_>` | note: the lint level is defined here - --> $DIR/pass_ty_by_ref.rs:4:9 + --> $DIR/rustc_pass_by_value.rs:4:9 | -LL | #![deny(rustc::ty_pass_by_reference)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #![deny(rustc::pass_by_value)] + | ^^^^^^^^^^^^^^^^^^^^ error: passing `TyCtxt<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:15:18 + --> $DIR/rustc_pass_by_value.rs:15:18 | LL | ty_ctxt_ref: &TyCtxt<'_>, | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:19:28 + --> $DIR/rustc_pass_by_value.rs:19:28 | LL | fn ty_multi_ref(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {} | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:19:55 + --> $DIR/rustc_pass_by_value.rs:19:55 | LL | fn ty_multi_ref(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {} | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:26:17 + --> $DIR/rustc_pass_by_value.rs:26:17 | LL | ty_ref: &Ty<'_>, | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:28:22 + --> $DIR/rustc_pass_by_value.rs:28:22 | LL | ty_ctxt_ref: &TyCtxt<'_>, | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:31:41 + --> $DIR/rustc_pass_by_value.rs:31:41 | LL | fn ty_multi_ref_in_trait(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>); | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:31:68 + --> $DIR/rustc_pass_by_value.rs:31:68 | LL | fn ty_multi_ref_in_trait(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>); | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:53:17 + --> $DIR/rustc_pass_by_value.rs:53:17 | LL | ty_ref: &Ty<'_>, | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:55:22 + --> $DIR/rustc_pass_by_value.rs:55:22 | LL | ty_ctxt_ref: &TyCtxt<'_>, | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:59:38 + --> $DIR/rustc_pass_by_value.rs:59:38 | LL | fn ty_multi_ref_assoc(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {} | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/pass_ty_by_ref.rs:59:65 + --> $DIR/rustc_pass_by_value.rs:59:65 | LL | fn ty_multi_ref_assoc(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {} | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` diff --git a/src/test/ui-fulldeps/internal-lints/pass_ty_by_ref_self.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs similarity index 90% rename from src/test/ui-fulldeps/internal-lints/pass_ty_by_ref_self.rs rename to src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs index 48b140d91744c..8877148bb56bd 100644 --- a/src/test/ui-fulldeps/internal-lints/pass_ty_by_ref_self.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs @@ -5,10 +5,11 @@ // Considering that all other `internal-lints` are tested here // this seems like the cleaner solution though. #![feature(rustc_attrs)] -#![deny(rustc::ty_pass_by_reference)] +#![deny(rustc::pass_by_value)] #![allow(unused)] #[rustc_diagnostic_item = "TyCtxt"] +#[rustc_pass_by_value] struct TyCtxt<'tcx> { inner: &'tcx (), } @@ -18,12 +19,12 @@ impl<'tcx> TyCtxt<'tcx> { fn by_ref(&self) {} //~ ERROR passing `TyCtxt<'tcx>` by reference } - struct TyS<'tcx> { inner: &'tcx (), } #[rustc_diagnostic_item = "Ty"] +#[rustc_pass_by_value] type Ty<'tcx> = &'tcx TyS<'tcx>; impl<'tcx> TyS<'tcx> { diff --git a/src/test/ui-fulldeps/internal-lints/pass_ty_by_ref_self.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr similarity index 65% rename from src/test/ui-fulldeps/internal-lints/pass_ty_by_ref_self.stderr rename to src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr index 15a06e721ddcb..f86aea95aa7c6 100644 --- a/src/test/ui-fulldeps/internal-lints/pass_ty_by_ref_self.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr @@ -1,17 +1,17 @@ error: passing `TyCtxt<'tcx>` by reference - --> $DIR/pass_ty_by_ref_self.rs:18:15 + --> $DIR/rustc_pass_by_value_self.rs:19:15 | LL | fn by_ref(&self) {} | ^^^^^ help: try passing by value: `TyCtxt<'tcx>` | note: the lint level is defined here - --> $DIR/pass_ty_by_ref_self.rs:8:9 + --> $DIR/rustc_pass_by_value_self.rs:8:9 | -LL | #![deny(rustc::ty_pass_by_reference)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #![deny(rustc::pass_by_value)] + | ^^^^^^^^^^^^^^^^^^^^ error: passing `Ty<'tcx>` by reference - --> $DIR/pass_ty_by_ref_self.rs:31:21 + --> $DIR/rustc_pass_by_value_self.rs:32:21 | LL | fn by_ref(self: &Ty<'tcx>) {} | ^^^^^^^^^ help: try passing by value: `Ty<'tcx>` From ad57295fc9f5b0c747ae4f72d6208ccf03b94b0b Mon Sep 17 00:00:00 2001 From: Jack Huey <31162821+jackh726@users.noreply.github.com> Date: Sun, 9 Jan 2022 23:53:57 -0500 Subject: [PATCH 05/24] Elaborate param_env predicates when checking if type outlives involving projection holds --- .../src/infer/outlives/obligations.rs | 4 ++- compiler/rustc_infer/src/traits/util.rs | 17 +++++++++--- .../src/traits/select/confirmation.rs | 4 ++- .../generic-associated-types/issue-92096.rs | 27 +++++++++++++++++++ .../issue-92096.stderr | 18 +++++++++++++ .../generic-associated-types/issue-92280.rs | 26 ++++++++++++++++++ 6 files changed, 90 insertions(+), 6 deletions(-) create mode 100644 src/test/ui/generic-associated-types/issue-92096.rs create mode 100644 src/test/ui/generic-associated-types/issue-92096.stderr create mode 100644 src/test/ui/generic-associated-types/issue-92280.rs diff --git a/compiler/rustc_infer/src/infer/outlives/obligations.rs b/compiler/rustc_infer/src/infer/outlives/obligations.rs index 74eb263a63390..a5276afc5bfa7 100644 --- a/compiler/rustc_infer/src/infer/outlives/obligations.rs +++ b/compiler/rustc_infer/src/infer/outlives/obligations.rs @@ -164,7 +164,7 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> { "cannot process registered region obligations in a snapshot" ); - debug!("process_registered_region_obligations()"); + debug!(?param_env, "process_registered_region_obligations()"); let my_region_obligations = self.take_registered_region_obligations(); @@ -356,6 +356,8 @@ where let trait_bounds: Vec<_> = self.verify_bound.projection_declared_bounds_from_trait(projection_ty).collect(); + debug!(?trait_bounds); + // Compute the bounds we can derive from the environment. This // is an "approximate" match -- in some cases, these bounds // may not apply. diff --git a/compiler/rustc_infer/src/traits/util.rs b/compiler/rustc_infer/src/traits/util.rs index 8f5d6c85097cb..0833ebfd5f333 100644 --- a/compiler/rustc_infer/src/traits/util.rs +++ b/compiler/rustc_infer/src/traits/util.rs @@ -241,10 +241,19 @@ impl<'tcx> Elaborator<'tcx> { Component::UnresolvedInferenceVariable(_) => None, - Component::Projection(_) | Component::EscapingProjection(_) => { - // We can probably do more here. This - // corresponds to a case like `<T as - // Foo<'a>>::U: 'b`. + Component::Projection(projection) => { + // We might end up here if we have `Foo<<Bar as Baz>::Assoc>: 'a`. + // With this, we can deduce that `<Bar as Baz>::Assoc: 'a`. + let ty = + tcx.mk_projection(projection.item_def_id, projection.substs); + Some(ty::PredicateKind::TypeOutlives(ty::OutlivesPredicate( + ty, r_min, + ))) + } + + Component::EscapingProjection(_) => { + // We might be able to do more here, but we don't + // want to deal with escaping vars right now. None } }) diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index b7fc578ea3bd3..d7b92c64fd7a1 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -206,7 +206,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { })?); if let ty::Projection(..) = placeholder_self_ty.kind() { - for predicate in tcx.predicates_of(def_id).instantiate_own(tcx, substs).predicates { + let predicates = tcx.predicates_of(def_id).instantiate_own(tcx, substs).predicates; + debug!(?predicates, "projection predicates"); + for predicate in predicates { let normalized = normalize_with_depth_to( self, obligation.param_env, diff --git a/src/test/ui/generic-associated-types/issue-92096.rs b/src/test/ui/generic-associated-types/issue-92096.rs new file mode 100644 index 0000000000000..6c81babc0bccb --- /dev/null +++ b/src/test/ui/generic-associated-types/issue-92096.rs @@ -0,0 +1,27 @@ +// edition:2018 +// check-fail +// FIXME(generic_associated_types): this should pass, but we end up +// essentially requiring that `for<'s> C: 's` + +#![feature(generic_associated_types)] + +use std::future::Future; + +trait Client { + type Connecting<'a>: Future + Send + where + Self: 'a; + + fn connect(&'_ self) -> Self::Connecting<'_>; +} + +fn call_connect<C>(c: &'_ C) -> impl '_ + Future + Send +//~^ ERROR the parameter +//~| ERROR the parameter +where + C: Client + Send + Sync, +{ + async move { c.connect().await } +} + +fn main() {} diff --git a/src/test/ui/generic-associated-types/issue-92096.stderr b/src/test/ui/generic-associated-types/issue-92096.stderr new file mode 100644 index 0000000000000..a897ba5b966ec --- /dev/null +++ b/src/test/ui/generic-associated-types/issue-92096.stderr @@ -0,0 +1,18 @@ +error[E0311]: the parameter type `C` may not live long enough + --> $DIR/issue-92096.rs:18:33 + | +LL | fn call_connect<C>(c: &'_ C) -> impl '_ + Future + Send + | - ^^^^^^^^^^^^^^^^^^^^^^^ ...so that the type `C` will meet its required lifetime bounds + | | + | help: consider adding an explicit lifetime bound...: `C: 'a` + +error[E0311]: the parameter type `C` may not live long enough + --> $DIR/issue-92096.rs:18:33 + | +LL | fn call_connect<C>(c: &'_ C) -> impl '_ + Future + Send + | - ^^^^^^^^^^^^^^^^^^^^^^^ ...so that the type `C` will meet its required lifetime bounds + | | + | help: consider adding an explicit lifetime bound...: `C: 'a` + +error: aborting due to 2 previous errors + diff --git a/src/test/ui/generic-associated-types/issue-92280.rs b/src/test/ui/generic-associated-types/issue-92280.rs new file mode 100644 index 0000000000000..db26493ecadfa --- /dev/null +++ b/src/test/ui/generic-associated-types/issue-92280.rs @@ -0,0 +1,26 @@ +// check-pass + +#![feature(generic_associated_types)] +#![allow(non_camel_case_types)] + +trait HasAssoc { + type Assoc; +} + +trait Iterate<S: HasAssoc> { + type Iter<'a> + where + Self: 'a; +} + +struct KeySegment_Broken<T> { + key: T, +} +impl<S: HasAssoc> Iterate<S> for KeySegment_Broken<S::Assoc> { + type Iter<'a> + where + Self: 'a, + = (); +} + +fn main() {} From 91ed6892f7ec60fb76eeaaa024919f293a58d733 Mon Sep 17 00:00:00 2001 From: Mahdi Dibaiee <mdibaiee@pm.me> Date: Mon, 10 Jan 2022 08:54:42 +0000 Subject: [PATCH 06/24] rustc_pass_by_value lint: add test on custom types --- compiler/rustc_lint/src/pass_by_value.rs | 1 + compiler/rustc_passes/src/check_attr.rs | 11 ++-- .../internal-lints/rustc_pass_by_value.rs | 40 ++++++++++++++ .../internal-lints/rustc_pass_by_value.stderr | 52 ++++++++++++++----- 4 files changed, 82 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_lint/src/pass_by_value.rs b/compiler/rustc_lint/src/pass_by_value.rs index 0bfa2a673c2c4..0847f600f9d14 100644 --- a/compiler/rustc_lint/src/pass_by_value.rs +++ b/compiler/rustc_lint/src/pass_by_value.rs @@ -11,6 +11,7 @@ declare_tool_lint! { /// The `rustc_pass_by_value` lint marks a type with `#[rustc_pass_by_value]` requiring it to always be passed by value. /// This is usually used for types that are thin wrappers around references, so there is no benefit to an extra /// layer of indirection. (Example: `Ty` which is a reference to a `TyS`) + /// This lint relies on `#[rustc_diagnostic_item]` being available for the target. pub rustc::PASS_BY_VALUE, Warn, "pass by reference of a type flagged as `#[rustc_pass_by_value]`", diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index 2febb2e56ecf8..e700a61ce48ab 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -1070,20 +1070,15 @@ impl CheckAttrVisitor<'_> { /// Warns against some misuses of `#[pass_by_value]` fn check_pass_by_value(&self, attr: &Attribute, span: &Span, target: Target) -> bool { match target { - Target::Struct - | Target::Enum - | Target::Union - | Target::Trait - | Target::TraitAlias - | Target::TyAlias => true, + Target::Struct | Target::Enum | Target::TyAlias => true, _ => { self.tcx .sess .struct_span_err( attr.span, - "`pass_by_value` attribute should be applied to a struct, enum, trait or type alias.", + "`pass_by_value` attribute should be applied to a struct, enum or type alias.", ) - .span_label(*span, "is not a struct, enum, trait or type alias") + .span_label(*span, "is not a struct, enum or type alias") .emit(); false } diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs index 783019d894513..bf2b1fbaf45fc 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs @@ -1,5 +1,6 @@ // compile-flags: -Z unstable-options +#![feature(rustc_attrs)] #![feature(rustc_private)] #![deny(rustc::pass_by_value)] #![allow(unused)] @@ -61,4 +62,43 @@ impl Foo { //~^^ ERROR passing `TyCtxt<'_>` by reference } +#[rustc_diagnostic_item = "CustomEnum"] +#[rustc_pass_by_value] +enum CustomEnum { + A, + B, +} + +impl CustomEnum { + fn test( + value: CustomEnum, + reference: &CustomEnum, //~ ERROR passing `CustomEnum` by reference + ) { + } +} + +#[rustc_diagnostic_item = "CustomStruct"] +#[rustc_pass_by_value] +struct CustomStruct { + s: u8, +} + +#[rustc_diagnostic_item = "CustomAlias"] +#[rustc_pass_by_value] +type CustomAlias<'a> = &'a CustomStruct; //~ ERROR passing `CustomStruct` by reference + +impl CustomStruct { + fn test( + value: CustomStruct, + reference: &CustomStruct, //~ ERROR passing `CustomStruct` by reference + ) { + } + + fn test_alias( + value: CustomAlias, + reference: &CustomAlias, //~ ERROR passing `CustomAlias<>` by reference + ) { + } +} + fn main() {} diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr index 5fbde93878931..c59c1adf8997d 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr @@ -1,80 +1,104 @@ error: passing `Ty<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:13:13 + --> $DIR/rustc_pass_by_value.rs:14:13 | LL | ty_ref: &Ty<'_>, | ^^^^^^^ help: try passing by value: `Ty<'_>` | note: the lint level is defined here - --> $DIR/rustc_pass_by_value.rs:4:9 + --> $DIR/rustc_pass_by_value.rs:5:9 | LL | #![deny(rustc::pass_by_value)] | ^^^^^^^^^^^^^^^^^^^^ error: passing `TyCtxt<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:15:18 + --> $DIR/rustc_pass_by_value.rs:16:18 | LL | ty_ctxt_ref: &TyCtxt<'_>, | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:19:28 + --> $DIR/rustc_pass_by_value.rs:20:28 | LL | fn ty_multi_ref(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {} | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:19:55 + --> $DIR/rustc_pass_by_value.rs:20:55 | LL | fn ty_multi_ref(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {} | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:26:17 + --> $DIR/rustc_pass_by_value.rs:27:17 | LL | ty_ref: &Ty<'_>, | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:28:22 + --> $DIR/rustc_pass_by_value.rs:29:22 | LL | ty_ctxt_ref: &TyCtxt<'_>, | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:31:41 + --> $DIR/rustc_pass_by_value.rs:32:41 | LL | fn ty_multi_ref_in_trait(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>); | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:31:68 + --> $DIR/rustc_pass_by_value.rs:32:68 | LL | fn ty_multi_ref_in_trait(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>); | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:53:17 + --> $DIR/rustc_pass_by_value.rs:54:17 | LL | ty_ref: &Ty<'_>, | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:55:22 + --> $DIR/rustc_pass_by_value.rs:56:22 | LL | ty_ctxt_ref: &TyCtxt<'_>, | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `Ty<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:59:38 + --> $DIR/rustc_pass_by_value.rs:60:38 | LL | fn ty_multi_ref_assoc(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {} | ^^^^^^^ help: try passing by value: `Ty<'_>` error: passing `TyCtxt<'_>` by reference - --> $DIR/rustc_pass_by_value.rs:59:65 + --> $DIR/rustc_pass_by_value.rs:60:65 | LL | fn ty_multi_ref_assoc(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_>) {} | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` -error: aborting due to 12 previous errors +error: passing `CustomEnum` by reference + --> $DIR/rustc_pass_by_value.rs:75:20 + | +LL | reference: &CustomEnum, + | ^^^^^^^^^^^ help: try passing by value: `CustomEnum` + +error: passing `CustomStruct` by reference + --> $DIR/rustc_pass_by_value.rs:88:24 + | +LL | type CustomAlias<'a> = &'a CustomStruct; + | ^^^^^^^^^^^^^^^^ help: try passing by value: `CustomStruct` + +error: passing `CustomStruct` by reference + --> $DIR/rustc_pass_by_value.rs:93:20 + | +LL | reference: &CustomStruct, + | ^^^^^^^^^^^^^ help: try passing by value: `CustomStruct` + +error: passing `CustomAlias<>` by reference + --> $DIR/rustc_pass_by_value.rs:99:20 + | +LL | reference: &CustomAlias, + | ^^^^^^^^^^^^ help: try passing by value: `CustomAlias<>` + +error: aborting due to 16 previous errors From 71e33146734362984258df415ac8308618968ed4 Mon Sep 17 00:00:00 2001 From: Mahdi Dibaiee <mdibaiee@pm.me> Date: Mon, 10 Jan 2022 18:12:28 +0000 Subject: [PATCH 07/24] rustc_pass_by_value remove dependency on rustc_diagnostic_item --- compiler/rustc_lint/src/pass_by_value.rs | 11 ++++------- .../ui-fulldeps/internal-lints/rustc_pass_by_value.rs | 3 --- .../internal-lints/rustc_pass_by_value.stderr | 8 ++++---- .../internal-lints/rustc_pass_by_value_self.rs | 2 -- .../internal-lints/rustc_pass_by_value_self.stderr | 4 ++-- 5 files changed, 10 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_lint/src/pass_by_value.rs b/compiler/rustc_lint/src/pass_by_value.rs index 0847f600f9d14..c689f34d9e3b4 100644 --- a/compiler/rustc_lint/src/pass_by_value.rs +++ b/compiler/rustc_lint/src/pass_by_value.rs @@ -11,7 +11,6 @@ declare_tool_lint! { /// The `rustc_pass_by_value` lint marks a type with `#[rustc_pass_by_value]` requiring it to always be passed by value. /// This is usually used for types that are thin wrappers around references, so there is no benefit to an extra /// layer of indirection. (Example: `Ty` which is a reference to a `TyS`) - /// This lint relies on `#[rustc_diagnostic_item]` being available for the target. pub rustc::PASS_BY_VALUE, Warn, "pass by reference of a type flagged as `#[rustc_pass_by_value]`", @@ -52,16 +51,14 @@ fn path_for_pass_by_value(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> Option<Stri if let TyKind::Path(QPath::Resolved(_, path)) = &ty.kind { match path.res { Res::Def(_, def_id) if has_pass_by_value_attr(cx, def_id) => { - if let Some(name) = cx.tcx.get_diagnostic_name(def_id) { - return Some(format!("{}{}", name, gen_args(path.segments.last().unwrap()))); - } + let name = cx.tcx.item_name(def_id).to_ident_string(); + return Some(format!("{}{}", name, gen_args(path.segments.last().unwrap()))); } Res::SelfTy(None, Some((did, _))) => { if let ty::Adt(adt, substs) = cx.tcx.type_of(did).kind() { if has_pass_by_value_attr(cx, adt.did) { - if let Some(name) = cx.tcx.get_diagnostic_name(adt.did) { - return Some(format!("{}<{}>", name, substs[0])); - } + let name = cx.tcx.item_name(adt.did).to_ident_string(); + return Some(format!("{}<{}>", name, substs[0])); } } } diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs index bf2b1fbaf45fc..293464c07ef78 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs @@ -62,7 +62,6 @@ impl Foo { //~^^ ERROR passing `TyCtxt<'_>` by reference } -#[rustc_diagnostic_item = "CustomEnum"] #[rustc_pass_by_value] enum CustomEnum { A, @@ -77,13 +76,11 @@ impl CustomEnum { } } -#[rustc_diagnostic_item = "CustomStruct"] #[rustc_pass_by_value] struct CustomStruct { s: u8, } -#[rustc_diagnostic_item = "CustomAlias"] #[rustc_pass_by_value] type CustomAlias<'a> = &'a CustomStruct; //~ ERROR passing `CustomStruct` by reference diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr index c59c1adf8997d..dbb9180ed7d2f 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr @@ -77,25 +77,25 @@ LL | fn ty_multi_ref_assoc(ty_multi: &&Ty<'_>, ty_ctxt_multi: &&&&TyCtxt<'_> | ^^^^^^^^^^^ help: try passing by value: `TyCtxt<'_>` error: passing `CustomEnum` by reference - --> $DIR/rustc_pass_by_value.rs:75:20 + --> $DIR/rustc_pass_by_value.rs:74:20 | LL | reference: &CustomEnum, | ^^^^^^^^^^^ help: try passing by value: `CustomEnum` error: passing `CustomStruct` by reference - --> $DIR/rustc_pass_by_value.rs:88:24 + --> $DIR/rustc_pass_by_value.rs:85:24 | LL | type CustomAlias<'a> = &'a CustomStruct; | ^^^^^^^^^^^^^^^^ help: try passing by value: `CustomStruct` error: passing `CustomStruct` by reference - --> $DIR/rustc_pass_by_value.rs:93:20 + --> $DIR/rustc_pass_by_value.rs:90:20 | LL | reference: &CustomStruct, | ^^^^^^^^^^^^^ help: try passing by value: `CustomStruct` error: passing `CustomAlias<>` by reference - --> $DIR/rustc_pass_by_value.rs:99:20 + --> $DIR/rustc_pass_by_value.rs:96:20 | LL | reference: &CustomAlias, | ^^^^^^^^^^^^ help: try passing by value: `CustomAlias<>` diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs index 8877148bb56bd..a4529f9856368 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs @@ -8,7 +8,6 @@ #![deny(rustc::pass_by_value)] #![allow(unused)] -#[rustc_diagnostic_item = "TyCtxt"] #[rustc_pass_by_value] struct TyCtxt<'tcx> { inner: &'tcx (), @@ -23,7 +22,6 @@ struct TyS<'tcx> { inner: &'tcx (), } -#[rustc_diagnostic_item = "Ty"] #[rustc_pass_by_value] type Ty<'tcx> = &'tcx TyS<'tcx>; diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr index f86aea95aa7c6..ca47babd13d8a 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr @@ -1,5 +1,5 @@ error: passing `TyCtxt<'tcx>` by reference - --> $DIR/rustc_pass_by_value_self.rs:19:15 + --> $DIR/rustc_pass_by_value_self.rs:18:15 | LL | fn by_ref(&self) {} | ^^^^^ help: try passing by value: `TyCtxt<'tcx>` @@ -11,7 +11,7 @@ LL | #![deny(rustc::pass_by_value)] | ^^^^^^^^^^^^^^^^^^^^ error: passing `Ty<'tcx>` by reference - --> $DIR/rustc_pass_by_value_self.rs:32:21 + --> $DIR/rustc_pass_by_value_self.rs:30:21 | LL | fn by_ref(self: &Ty<'tcx>) {} | ^^^^^^^^^ help: try passing by value: `Ty<'tcx>` From 49553bbc98add28ed6f126225ffe6854a7ad7f29 Mon Sep 17 00:00:00 2001 From: Noah Lev <camelidcamel@gmail.com> Date: Thu, 6 Jan 2022 13:44:46 -0800 Subject: [PATCH 08/24] Remove hack that is no longer necessary This hack was added in 6ab1f05697c3f2df4e439a05ebcee479a9a16d80. I don't know what change allowed removing the hack, but that commit added a test (which I presume covered the hack's behavior), and all tests are passing with this change. So, I think it should be good. --- src/librustdoc/passes/collect_intra_doc_links.rs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 9d1a8b3f80fec..d02ef9dafe7ff 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -958,17 +958,7 @@ impl<'a, 'tcx> DocVisitor for LinkCollector<'a, 'tcx> { { self.cx.tcx.parent(did) } - Some(did) => match self.cx.tcx.parent(did) { - // HACK(jynelson): `clean` marks associated types as `TypedefItem`, not as `AssocTypeItem`. - // Fixing this breaks `fn render_deref_methods`. - // As a workaround, see if the parent of the item is an `impl`; if so this must be an associated item, - // regardless of what rustdoc wants to call it. - Some(parent) => { - let parent_kind = self.cx.tcx.def_kind(parent); - Some(if parent_kind == DefKind::Impl { parent } else { did }) - } - None => Some(did), - }, + Some(did) => Some(did), }; // FIXME(jynelson): this shouldn't go through stringification, rustdoc should just use the DefId directly From e18b23b7f494d57090be94351e92c1d69251f1a9 Mon Sep 17 00:00:00 2001 From: Noah Lev <camelidcamel@gmail.com> Date: Thu, 6 Jan 2022 13:59:01 -0800 Subject: [PATCH 09/24] Move two intra-doc-link tests into the `intra-doc` folder --- .../rustdoc/{intra-link-prim-self.rs => intra-doc/prim-self.rs} | 2 +- .../{intra-link-self-cache.rs => intra-doc/self-cache.rs} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/test/rustdoc/{intra-link-prim-self.rs => intra-doc/prim-self.rs} (94%) rename src/test/rustdoc/{intra-link-self-cache.rs => intra-doc/self-cache.rs} (100%) diff --git a/src/test/rustdoc/intra-link-prim-self.rs b/src/test/rustdoc/intra-doc/prim-self.rs similarity index 94% rename from src/test/rustdoc/intra-link-prim-self.rs rename to src/test/rustdoc/intra-doc/prim-self.rs index 8a564acf2ca4b..dbd0a7cf0eb45 100644 --- a/src/test/rustdoc/intra-link-prim-self.rs +++ b/src/test/rustdoc/intra-doc/prim-self.rs @@ -7,7 +7,7 @@ #[lang = "usize"] /// [Self::f] /// [Self::MAX] -// @has intra_link_prim_self/primitive.usize.html +// @has prim_self/primitive.usize.html // @has - '//a[@href="primitive.usize.html#method.f"]' 'Self::f' // @has - '//a[@href="primitive.usize.html#associatedconstant.MAX"]' 'Self::MAX' impl usize { diff --git a/src/test/rustdoc/intra-link-self-cache.rs b/src/test/rustdoc/intra-doc/self-cache.rs similarity index 100% rename from src/test/rustdoc/intra-link-self-cache.rs rename to src/test/rustdoc/intra-doc/self-cache.rs From ca20d64fb77dc0aa5448e7bf9bcaa19164f1f521 Mon Sep 17 00:00:00 2001 From: Noah Lev <camelidcamel@gmail.com> Date: Thu, 6 Jan 2022 14:12:48 -0800 Subject: [PATCH 10/24] Enable ignored part of test Inherent associated types *are* supported, just unstable. --- src/test/rustdoc/intra-doc/prim-self.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/test/rustdoc/intra-doc/prim-self.rs b/src/test/rustdoc/intra-doc/prim-self.rs index dbd0a7cf0eb45..7a65723d77b21 100644 --- a/src/test/rustdoc/intra-doc/prim-self.rs +++ b/src/test/rustdoc/intra-doc/prim-self.rs @@ -1,7 +1,9 @@ #![deny(rustdoc::broken_intra_doc_links)] +#![allow(incomplete_features)] // inherent_associated_types #![feature(lang_items)] #![feature(no_core)] #![feature(rustdoc_internals)] +#![feature(inherent_associated_types)] #![no_core] #[lang = "usize"] @@ -17,10 +19,9 @@ impl usize { /// 10 and 2^32 are basically the same. pub const MAX: usize = 10; - // FIXME(#8995) uncomment this when associated types in inherent impls are supported - // @ has - '//a[@href="{{channel}}/std/primitive.usize.html#associatedtype.ME"]' 'Self::ME' - // / [Self::ME] - //pub type ME = usize; + // @has - '//a[@href="primitive.usize.html#associatedtype.ME"]' 'Self::ME' + /// [Self::ME] + pub type ME = usize; } #[doc(primitive = "usize")] From 977a7ca2e4b9c02d7f43a999bbeee651a31241d8 Mon Sep 17 00:00:00 2001 From: Noah Lev <camelidcamel@gmail.com> Date: Thu, 6 Jan 2022 14:49:28 -0800 Subject: [PATCH 11/24] Add test for disambiguator mismatch with crate This currently calls `std` a "crate" in one part of the message and a "module" in another part. The next commits fix this so it says "crate" in both places. --- .../rustdoc-ui/intra-doc/disambiguator-mismatch.rs | 5 +++++ .../intra-doc/disambiguator-mismatch.stderr | 13 ++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/test/rustdoc-ui/intra-doc/disambiguator-mismatch.rs b/src/test/rustdoc-ui/intra-doc/disambiguator-mismatch.rs index 142008cf76508..ae48db48c1833 100644 --- a/src/test/rustdoc-ui/intra-doc/disambiguator-mismatch.rs +++ b/src/test/rustdoc-ui/intra-doc/disambiguator-mismatch.rs @@ -73,4 +73,9 @@ trait T {} //~^ ERROR incompatible link kind for `f` //~| NOTE this link resolved //~| HELP add parentheses + +/// Link to [fn@std] +//~^ ERROR unresolved link to `std` +//~| NOTE this link resolves to the crate `std` +//~| HELP to link to the module, prefix with `mod@` pub fn f() {} diff --git a/src/test/rustdoc-ui/intra-doc/disambiguator-mismatch.stderr b/src/test/rustdoc-ui/intra-doc/disambiguator-mismatch.stderr index 12122f5fa8674..1d48b5f7471b6 100644 --- a/src/test/rustdoc-ui/intra-doc/disambiguator-mismatch.stderr +++ b/src/test/rustdoc-ui/intra-doc/disambiguator-mismatch.stderr @@ -138,5 +138,16 @@ LL - /// Link to [const@f] LL + /// Link to [f()] | -error: aborting due to 12 previous errors +error: unresolved link to `std` + --> $DIR/disambiguator-mismatch.rs:77:14 + | +LL | /// Link to [fn@std] + | ^^^^^^ this link resolves to the crate `std`, which is not in the value namespace + | +help: to link to the module, prefix with `mod@` + | +LL | /// Link to [mod@std] + | ~~~~ + +error: aborting due to 13 previous errors From 9acd8133806504f54e023151ec789c134656a1cc Mon Sep 17 00:00:00 2001 From: Noah Lev <camelidcamel@gmail.com> Date: Thu, 6 Jan 2022 14:43:27 -0800 Subject: [PATCH 12/24] Use Res instead of Disambiguator for `resolved` in `report_mismatch` This allows simplifying a lot of code. It also fixes a subtle bug, exemplified by the test output changes. --- .../passes/collect_intra_doc_links.rs | 115 ++++++++---------- .../intra-doc/disambiguator-mismatch.rs | 2 +- .../intra-doc/disambiguator-mismatch.stderr | 2 +- 3 files changed, 52 insertions(+), 67 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index d02ef9dafe7ff..20e248a445cb7 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -109,6 +109,45 @@ impl Res { Res::Primitive(_) => None, } } + + /// Used for error reporting. + fn disambiguator_suggestion(self) -> Suggestion { + let kind = match self { + Res::Primitive(_) => return Suggestion::Prefix("prim"), + Res::Def(kind, _) => kind, + }; + if kind == DefKind::Macro(MacroKind::Bang) { + return Suggestion::Macro; + } else if kind == DefKind::Fn || kind == DefKind::AssocFn { + return Suggestion::Function; + } else if kind == DefKind::Field { + return Suggestion::RemoveDisambiguator; + } + + let prefix = match kind { + DefKind::Struct => "struct", + DefKind::Enum => "enum", + DefKind::Trait => "trait", + DefKind::Union => "union", + DefKind::Mod => "mod", + DefKind::Const | DefKind::ConstParam | DefKind::AssocConst | DefKind::AnonConst => { + "const" + } + DefKind::Static => "static", + DefKind::Macro(MacroKind::Derive) => "derive", + // Now handle things that don't have a specific disambiguator + _ => match kind + .ns() + .expect("tried to calculate a disambiguator for a def without a namespace?") + { + Namespace::TypeNS => "type", + Namespace::ValueNS => "value", + Namespace::MacroNS => "macro", + }, + }; + + Suggestion::Prefix(prefix) + } } impl TryFrom<ResolveRes> for Res { @@ -1267,7 +1306,7 @@ impl LinkCollector<'_, '_> { } } - let report_mismatch = |specified: Disambiguator, resolved: Disambiguator| { + let report_mismatch = |specified: Disambiguator, resolved: Res| { // The resolved item did not match the disambiguator; give a better error than 'not found' let msg = format!("incompatible link kind for `{}`", path_str); let callback = |diag: &mut DiagnosticBuilder<'_>, sp: Option<rustc_span::Span>| { @@ -1276,7 +1315,7 @@ impl LinkCollector<'_, '_> { resolved.article(), resolved.descr(), specified.article(), - specified.descr() + specified.descr(), ); if let Some(sp) = sp { diag.span_label(sp, ¬e); @@ -1311,7 +1350,7 @@ impl LinkCollector<'_, '_> { => {} (actual, Some(Disambiguator::Kind(expected))) if actual == expected => {} (_, Some(specified @ Disambiguator::Kind(_) | specified @ Disambiguator::Primitive)) => { - report_mismatch(specified, Disambiguator::Kind(kind)); + report_mismatch(specified, Res::Def(kind, id)); return None; } } @@ -1362,7 +1401,7 @@ impl LinkCollector<'_, '_> { match disambiguator { Some(Disambiguator::Primitive | Disambiguator::Namespace(_)) | None => {} Some(other) => { - report_mismatch(other, Disambiguator::Primitive); + report_mismatch(other, res); return None; } } @@ -1676,53 +1715,6 @@ impl Disambiguator { } } - fn from_res(res: Res) -> Self { - match res { - Res::Def(kind, _) => Disambiguator::Kind(kind), - Res::Primitive(_) => Disambiguator::Primitive, - } - } - - /// Used for error reporting. - fn suggestion(self) -> Suggestion { - let kind = match self { - Disambiguator::Primitive => return Suggestion::Prefix("prim"), - Disambiguator::Kind(kind) => kind, - Disambiguator::Namespace(_) => panic!("display_for cannot be used on namespaces"), - }; - if kind == DefKind::Macro(MacroKind::Bang) { - return Suggestion::Macro; - } else if kind == DefKind::Fn || kind == DefKind::AssocFn { - return Suggestion::Function; - } else if kind == DefKind::Field { - return Suggestion::RemoveDisambiguator; - } - - let prefix = match kind { - DefKind::Struct => "struct", - DefKind::Enum => "enum", - DefKind::Trait => "trait", - DefKind::Union => "union", - DefKind::Mod => "mod", - DefKind::Const | DefKind::ConstParam | DefKind::AssocConst | DefKind::AnonConst => { - "const" - } - DefKind::Static => "static", - DefKind::Macro(MacroKind::Derive) => "derive", - // Now handle things that don't have a specific disambiguator - _ => match kind - .ns() - .expect("tried to calculate a disambiguator for a def without a namespace?") - { - Namespace::TypeNS => "type", - Namespace::ValueNS => "value", - Namespace::MacroNS => "macro", - }, - }; - - Suggestion::Prefix(prefix) - } - fn ns(self) -> Namespace { match self { Self::Namespace(n) => n, @@ -2070,15 +2062,9 @@ fn resolution_failure( ResolutionFailure::NotResolved { .. } => unreachable!("handled above"), ResolutionFailure::Dummy => continue, ResolutionFailure::WrongNamespace { res, expected_ns } => { - if let Res::Def(kind, _) = res { - let disambiguator = Disambiguator::Kind(kind); - suggest_disambiguator( - disambiguator, - diag, - path_str, - diag_info.ori_link, - sp, - ) + // FIXME: does this need to be behind an `if`? + if matches!(res, Res::Def(..)) { + suggest_disambiguator(res, diag, path_str, diag_info.ori_link, sp); } format!( @@ -2214,8 +2200,7 @@ fn ambiguity_error( } for res in candidates { - let disambiguator = Disambiguator::from_res(res); - suggest_disambiguator(disambiguator, diag, path_str, diag_info.ori_link, sp); + suggest_disambiguator(res, diag, path_str, diag_info.ori_link, sp); } }); } @@ -2223,14 +2208,14 @@ fn ambiguity_error( /// In case of an ambiguity or mismatched disambiguator, suggest the correct /// disambiguator. fn suggest_disambiguator( - disambiguator: Disambiguator, + res: Res, diag: &mut DiagnosticBuilder<'_>, path_str: &str, ori_link: &str, sp: Option<rustc_span::Span>, ) { - let suggestion = disambiguator.suggestion(); - let help = format!("to link to the {}, {}", disambiguator.descr(), suggestion.descr()); + let suggestion = res.disambiguator_suggestion(); + let help = format!("to link to the {}, {}", res.descr(), suggestion.descr()); if let Some(sp) = sp { let mut spans = suggestion.as_help_span(path_str, ori_link, sp); diff --git a/src/test/rustdoc-ui/intra-doc/disambiguator-mismatch.rs b/src/test/rustdoc-ui/intra-doc/disambiguator-mismatch.rs index ae48db48c1833..2d66566119bc3 100644 --- a/src/test/rustdoc-ui/intra-doc/disambiguator-mismatch.rs +++ b/src/test/rustdoc-ui/intra-doc/disambiguator-mismatch.rs @@ -77,5 +77,5 @@ trait T {} /// Link to [fn@std] //~^ ERROR unresolved link to `std` //~| NOTE this link resolves to the crate `std` -//~| HELP to link to the module, prefix with `mod@` +//~| HELP to link to the crate, prefix with `mod@` pub fn f() {} diff --git a/src/test/rustdoc-ui/intra-doc/disambiguator-mismatch.stderr b/src/test/rustdoc-ui/intra-doc/disambiguator-mismatch.stderr index 1d48b5f7471b6..ad9102c506f7f 100644 --- a/src/test/rustdoc-ui/intra-doc/disambiguator-mismatch.stderr +++ b/src/test/rustdoc-ui/intra-doc/disambiguator-mismatch.stderr @@ -144,7 +144,7 @@ error: unresolved link to `std` LL | /// Link to [fn@std] | ^^^^^^ this link resolves to the crate `std`, which is not in the value namespace | -help: to link to the module, prefix with `mod@` +help: to link to the crate, prefix with `mod@` | LL | /// Link to [mod@std] | ~~~~ From 591ec49df312ec4cbcdec0f082f123f473c182a9 Mon Sep 17 00:00:00 2001 From: Noah Lev <camelidcamel@gmail.com> Date: Thu, 6 Jan 2022 14:45:23 -0800 Subject: [PATCH 13/24] Remove unnecessary conditional for suggesting disambiguator Now that `res` is used directly, it seems the conditional is unnecessary. --- src/librustdoc/passes/collect_intra_doc_links.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 20e248a445cb7..dd2fa9d627360 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -2062,10 +2062,7 @@ fn resolution_failure( ResolutionFailure::NotResolved { .. } => unreachable!("handled above"), ResolutionFailure::Dummy => continue, ResolutionFailure::WrongNamespace { res, expected_ns } => { - // FIXME: does this need to be behind an `if`? - if matches!(res, Res::Def(..)) { - suggest_disambiguator(res, diag, path_str, diag_info.ori_link, sp); - } + suggest_disambiguator(res, diag, path_str, diag_info.ori_link, sp); format!( "this link resolves to {}, which is not in the {} namespace", From a5f09f74d6dc0f52fd2c73fca0a9e5bb99eb756c Mon Sep 17 00:00:00 2001 From: Noah Lev <camelidcamel@gmail.com> Date: Thu, 6 Jan 2022 15:06:35 -0800 Subject: [PATCH 14/24] Update comment and make code clearer I'm still not sure why this hack works so seemingly well. --- src/librustdoc/passes/collect_intra_doc_links.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index dd2fa9d627360..13e1992a31caa 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -6,13 +6,12 @@ use rustc_ast as ast; use rustc_data_structures::{fx::FxHashMap, stable_set::FxHashSet}; use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_expand::base::SyntaxExtensionKind; -use rustc_hir as hir; use rustc_hir::def::{ DefKind, Namespace::{self, *}, PerNS, }; -use rustc_hir::def_id::{CrateNum, DefId}; +use rustc_hir::def_id::{CrateNum, DefId, CRATE_DEF_ID}; use rustc_middle::ty::{DefIdTree, Ty, TyCtxt}; use rustc_middle::{bug, span_bug, ty}; use rustc_resolve::ParentScope; @@ -1736,9 +1735,9 @@ impl Disambiguator { fn descr(self) -> &'static str { match self { Self::Namespace(n) => n.descr(), - // HACK(jynelson): by looking at the source I saw the DefId we pass - // for `expected.descr()` doesn't matter, since it's not a crate - Self::Kind(k) => k.descr(DefId::local(hir::def_id::DefIndex::from_usize(0))), + // HACK(jynelson): the source of `DefKind::descr` only uses the DefId for + // printing "module" vs "crate" so using the wrong ID is not a huge problem + Self::Kind(k) => k.descr(CRATE_DEF_ID.to_def_id()), Self::Primitive => "builtin type", } } From 895fa9cd5c79b5c30614852c4c74a963b3ec458a Mon Sep 17 00:00:00 2001 From: Noah Lev <camelidcamel@gmail.com> Date: Thu, 6 Jan 2022 15:22:22 -0800 Subject: [PATCH 15/24] Extract functions for two closures These closures were quite complex and part of a quite complex function. The fact that they are closures makes mistakes likely when refactoring. For example, earlier, I meant to use `resolved`, an argument of the closure, but I instead typed `res`, which captured a local variable and caused a subtle bug that led to a confusing test failure. Extracting them as functions makes the code easier to understand and refactor. --- .../passes/collect_intra_doc_links.rs | 180 +++++++++++------- 1 file changed, 107 insertions(+), 73 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 13e1992a31caa..13b1d5b65a58a 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1305,79 +1305,9 @@ impl LinkCollector<'_, '_> { } } - let report_mismatch = |specified: Disambiguator, resolved: Res| { - // The resolved item did not match the disambiguator; give a better error than 'not found' - let msg = format!("incompatible link kind for `{}`", path_str); - let callback = |diag: &mut DiagnosticBuilder<'_>, sp: Option<rustc_span::Span>| { - let note = format!( - "this link resolved to {} {}, which is not {} {}", - resolved.article(), - resolved.descr(), - specified.article(), - specified.descr(), - ); - if let Some(sp) = sp { - diag.span_label(sp, ¬e); - } else { - diag.note(¬e); - } - suggest_disambiguator(resolved, diag, path_str, &ori_link.link, sp); - }; - report_diagnostic(self.cx.tcx, BROKEN_INTRA_DOC_LINKS, &msg, &diag_info, callback); - }; - - let verify = |kind: DefKind, id: DefId| { - let (kind, id) = if let Some(UrlFragment::Item(ItemFragment(_, id))) = fragment { - (self.cx.tcx.def_kind(id), id) - } else { - (kind, id) - }; - debug!("intra-doc link to {} resolved to {:?} (id: {:?})", path_str, res, id); - - // Disallow e.g. linking to enums with `struct@` - debug!("saw kind {:?} with disambiguator {:?}", kind, disambiguator); - match (kind, disambiguator) { - | (DefKind::Const | DefKind::ConstParam | DefKind::AssocConst | DefKind::AnonConst, Some(Disambiguator::Kind(DefKind::Const))) - // NOTE: this allows 'method' to mean both normal functions and associated functions - // This can't cause ambiguity because both are in the same namespace. - | (DefKind::Fn | DefKind::AssocFn, Some(Disambiguator::Kind(DefKind::Fn))) - // These are namespaces; allow anything in the namespace to match - | (_, Some(Disambiguator::Namespace(_))) - // If no disambiguator given, allow anything - | (_, None) - // All of these are valid, so do nothing - => {} - (actual, Some(Disambiguator::Kind(expected))) if actual == expected => {} - (_, Some(specified @ Disambiguator::Kind(_) | specified @ Disambiguator::Primitive)) => { - report_mismatch(specified, Res::Def(kind, id)); - return None; - } - } - - // item can be non-local e.g. when using #[doc(primitive = "pointer")] - if let Some((src_id, dst_id)) = id - .as_local() - // The `expect_def_id()` should be okay because `local_def_id_to_hir_id` - // would presumably panic if a fake `DefIndex` were passed. - .and_then(|dst_id| { - item.def_id.expect_def_id().as_local().map(|src_id| (src_id, dst_id)) - }) - { - if self.cx.tcx.privacy_access_levels(()).is_exported(src_id) - && !self.cx.tcx.privacy_access_levels(()).is_exported(dst_id) - { - privacy_error(self.cx, &diag_info, path_str); - } - } - - Some(()) - }; - match res { Res::Primitive(prim) => { if let Some(UrlFragment::Item(ItemFragment(_, id))) = fragment { - let kind = self.cx.tcx.def_kind(id); - // We're actually resolving an associated item of a primitive, so we need to // verify the disambiguator (if any) matches the type of the associated item. // This case should really follow the same flow as the `Res::Def` branch below, @@ -1386,7 +1316,16 @@ impl LinkCollector<'_, '_> { // doesn't allow statements like `use str::trim;`, making this a (hopefully) // valid omission. See https://github.com/rust-lang/rust/pull/80660#discussion_r551585677 // for discussion on the matter. - verify(kind, id)?; + let kind = self.cx.tcx.def_kind(id); + self.verify_disambiguator( + path_str, + &ori_link, + kind, + id, + disambiguator, + item, + &diag_info, + )?; // FIXME: it would be nice to check that the feature gate was enabled in the original crate, not just ignore it altogether. // However I'm not sure how to check that across crates. @@ -1400,7 +1339,9 @@ impl LinkCollector<'_, '_> { match disambiguator { Some(Disambiguator::Primitive | Disambiguator::Namespace(_)) | None => {} Some(other) => { - report_mismatch(other, res); + self.report_disambiguator_mismatch( + path_str, &ori_link, other, res, &diag_info, + ); return None; } } @@ -1414,13 +1355,106 @@ impl LinkCollector<'_, '_> { }) } Res::Def(kind, id) => { - verify(kind, id)?; + let (kind_for_dis, id_for_dis) = + if let Some(UrlFragment::Item(ItemFragment(_, id))) = fragment { + (self.cx.tcx.def_kind(id), id) + } else { + (kind, id) + }; + self.verify_disambiguator( + path_str, + &ori_link, + kind_for_dis, + id_for_dis, + disambiguator, + item, + &diag_info, + )?; let id = clean::register_res(self.cx, rustc_hir::def::Res::Def(kind, id)); Some(ItemLink { link: ori_link.link, link_text, did: id, fragment }) } } } + fn verify_disambiguator( + &self, + path_str: &str, + ori_link: &MarkdownLink, + kind: DefKind, + id: DefId, + disambiguator: Option<Disambiguator>, + item: &Item, + diag_info: &DiagnosticInfo<'_>, + ) -> Option<()> { + debug!("intra-doc link to {} resolved to {:?}", path_str, (kind, id)); + + // Disallow e.g. linking to enums with `struct@` + debug!("saw kind {:?} with disambiguator {:?}", kind, disambiguator); + match (kind, disambiguator) { + | (DefKind::Const | DefKind::ConstParam | DefKind::AssocConst | DefKind::AnonConst, Some(Disambiguator::Kind(DefKind::Const))) + // NOTE: this allows 'method' to mean both normal functions and associated functions + // This can't cause ambiguity because both are in the same namespace. + | (DefKind::Fn | DefKind::AssocFn, Some(Disambiguator::Kind(DefKind::Fn))) + // These are namespaces; allow anything in the namespace to match + | (_, Some(Disambiguator::Namespace(_))) + // If no disambiguator given, allow anything + | (_, None) + // All of these are valid, so do nothing + => {} + (actual, Some(Disambiguator::Kind(expected))) if actual == expected => {} + (_, Some(specified @ Disambiguator::Kind(_) | specified @ Disambiguator::Primitive)) => { + self.report_disambiguator_mismatch(path_str,ori_link,specified, Res::Def(kind, id),diag_info); + return None; + } + } + + // item can be non-local e.g. when using #[doc(primitive = "pointer")] + if let Some((src_id, dst_id)) = id + .as_local() + // The `expect_def_id()` should be okay because `local_def_id_to_hir_id` + // would presumably panic if a fake `DefIndex` were passed. + .and_then(|dst_id| { + item.def_id.expect_def_id().as_local().map(|src_id| (src_id, dst_id)) + }) + { + if self.cx.tcx.privacy_access_levels(()).is_exported(src_id) + && !self.cx.tcx.privacy_access_levels(()).is_exported(dst_id) + { + privacy_error(self.cx, diag_info, path_str); + } + } + + Some(()) + } + + fn report_disambiguator_mismatch( + &self, + path_str: &str, + ori_link: &MarkdownLink, + specified: Disambiguator, + resolved: Res, + diag_info: &DiagnosticInfo<'_>, + ) { + // The resolved item did not match the disambiguator; give a better error than 'not found' + let msg = format!("incompatible link kind for `{}`", path_str); + let callback = |diag: &mut DiagnosticBuilder<'_>, sp: Option<rustc_span::Span>| { + let note = format!( + "this link resolved to {} {}, which is not {} {}", + resolved.article(), + resolved.descr(), + specified.article(), + specified.descr(), + ); + if let Some(sp) = sp { + diag.span_label(sp, ¬e); + } else { + diag.note(¬e); + } + suggest_disambiguator(resolved, diag, path_str, &ori_link.link, sp); + }; + report_diagnostic(self.cx.tcx, BROKEN_INTRA_DOC_LINKS, &msg, &diag_info, callback); + } + fn report_rawptr_assoc_feature_gate(&self, dox: &str, ori_link: &MarkdownLink, item: &Item) { let span = super::source_span_for_markdown_range(self.cx.tcx, dox, &ori_link.range, &item.attrs) From 28d2353f3b1150313921916ae37a8525e9c2838d Mon Sep 17 00:00:00 2001 From: Noah Lev <camelidcamel@gmail.com> Date: Thu, 6 Jan 2022 15:57:51 -0800 Subject: [PATCH 16/24] Update some comments post the side channel removal --- src/librustdoc/passes/collect_intra_doc_links.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 13b1d5b65a58a..adeafc0d7589c 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -705,10 +705,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { })) } - /// Returns: - /// - None if no associated item was found - /// - Some((_, _, Some(_))) if an item was found and should go through a side channel - /// - Some((_, _, None)) otherwise + /// Resolve an associated item, returning its containing page's `Res` + /// and the fragment targeting the associated item on its page. fn resolve_associated_item( &mut self, root_res: Res, @@ -1475,7 +1473,6 @@ impl LinkCollector<'_, '_> { diag: DiagnosticInfo<'_>, cache_resolution_failure: bool, ) -> Option<(Res, Option<UrlFragment>)> { - // Try to look up both the result and the corresponding side channel value if let Some(ref cached) = self.visited_links.get(&key) { match cached { Some(cached) => { From a6762e962e50d5b6f864e33ebb27878e90651f22 Mon Sep 17 00:00:00 2001 From: Mahdi Dibaiee <mdibaiee@pm.me> Date: Tue, 11 Jan 2022 09:28:13 +0000 Subject: [PATCH 17/24] rustc_pass_by_value: allow types with no parameters on self includes minor refactorings --- compiler/rustc_feature/src/builtin_attrs.rs | 2 +- compiler/rustc_lint/src/pass_by_value.rs | 18 +++++------------- .../internal-lints/rustc_pass_by_value_self.rs | 7 +++++++ .../rustc_pass_by_value_self.stderr | 8 +++++++- 4 files changed, 20 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 88bf81864b23f..46817bc9c3f08 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -625,7 +625,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ ), rustc_attr!( rustc_pass_by_value, Normal, - template!(Word, NameValueStr: "reason"), WarnFollowing, + template!(Word), WarnFollowing, "#[rustc_pass_by_value] is used to mark types that must be passed by value instead of reference." ), BuiltinAttribute { diff --git a/compiler/rustc_lint/src/pass_by_value.rs b/compiler/rustc_lint/src/pass_by_value.rs index c689f34d9e3b4..00b023c26f3f3 100644 --- a/compiler/rustc_lint/src/pass_by_value.rs +++ b/compiler/rustc_lint/src/pass_by_value.rs @@ -2,7 +2,6 @@ use crate::{LateContext, LateLintPass, LintContext}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::def::Res; -use rustc_hir::def_id::DefId; use rustc_hir::{GenericArg, PathSegment, QPath, TyKind}; use rustc_middle::ty; use rustc_span::symbol::sym; @@ -50,15 +49,17 @@ impl<'tcx> LateLintPass<'tcx> for PassByValue { fn path_for_pass_by_value(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> Option<String> { if let TyKind::Path(QPath::Resolved(_, path)) = &ty.kind { match path.res { - Res::Def(_, def_id) if has_pass_by_value_attr(cx, def_id) => { + Res::Def(_, def_id) if cx.tcx.has_attr(def_id, sym::rustc_pass_by_value) => { let name = cx.tcx.item_name(def_id).to_ident_string(); return Some(format!("{}{}", name, gen_args(path.segments.last().unwrap()))); } Res::SelfTy(None, Some((did, _))) => { if let ty::Adt(adt, substs) = cx.tcx.type_of(did).kind() { - if has_pass_by_value_attr(cx, adt.did) { + if cx.tcx.has_attr(adt.did, sym::rustc_pass_by_value) { let name = cx.tcx.item_name(adt.did).to_ident_string(); - return Some(format!("{}<{}>", name, substs[0])); + let param = + substs.first().map(|s| format!("<{}>", s)).unwrap_or("".to_string()); + return Some(format!("{}{}", name, param)); } } } @@ -69,15 +70,6 @@ fn path_for_pass_by_value(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> Option<Stri None } -fn has_pass_by_value_attr(cx: &LateContext<'_>, def_id: DefId) -> bool { - for attr in cx.tcx.get_attrs(def_id).iter() { - if attr.has_name(sym::rustc_pass_by_value) { - return true; - } - } - false -} - fn gen_args(segment: &PathSegment<'_>) -> String { if let Some(args) = &segment.args { let lifetimes = args diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs index a4529f9856368..1be01e21bd5f1 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs @@ -30,4 +30,11 @@ impl<'tcx> TyS<'tcx> { fn by_ref(self: &Ty<'tcx>) {} //~ ERROR passing `Ty<'tcx>` by reference } +#[rustc_pass_by_value] +struct Foo; + +impl Foo { + fn with_ref(&self) {} //~ ERROR passing `Foo` by reference +} + fn main() {} diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr index ca47babd13d8a..965e79d962cdf 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr @@ -16,5 +16,11 @@ error: passing `Ty<'tcx>` by reference LL | fn by_ref(self: &Ty<'tcx>) {} | ^^^^^^^^^ help: try passing by value: `Ty<'tcx>` -error: aborting due to 2 previous errors +error: passing `Foo` by reference + --> $DIR/rustc_pass_by_value_self.rs:37:17 + | +LL | fn with_ref(&self) {} + | ^^^^^ help: try passing by value: `Foo` + +error: aborting due to 3 previous errors From 959bf2bc2e79defd0fe7d3c9987a6023eb8503cd Mon Sep 17 00:00:00 2001 From: Mahdi Dibaiee <mdibaiee@pm.me> Date: Tue, 11 Jan 2022 19:59:06 +0000 Subject: [PATCH 18/24] rustc_pass_by_value: handle generic and const type parameters --- compiler/rustc_lint/src/pass_by_value.rs | 33 +++++++++++-------- .../internal-lints/rustc_pass_by_value.rs | 15 +++++++++ .../internal-lints/rustc_pass_by_value.stderr | 14 +++++++- .../rustc_pass_by_value_self.rs | 14 ++++++++ .../rustc_pass_by_value_self.stderr | 14 +++++++- 5 files changed, 74 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_lint/src/pass_by_value.rs b/compiler/rustc_lint/src/pass_by_value.rs index 00b023c26f3f3..3435a5a6c82db 100644 --- a/compiler/rustc_lint/src/pass_by_value.rs +++ b/compiler/rustc_lint/src/pass_by_value.rs @@ -51,15 +51,13 @@ fn path_for_pass_by_value(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> Option<Stri match path.res { Res::Def(_, def_id) if cx.tcx.has_attr(def_id, sym::rustc_pass_by_value) => { let name = cx.tcx.item_name(def_id).to_ident_string(); - return Some(format!("{}{}", name, gen_args(path.segments.last().unwrap()))); + let path_segment = path.segments.last().unwrap(); + return Some(format!("{}{}", name, gen_args(cx, path_segment))); } Res::SelfTy(None, Some((did, _))) => { if let ty::Adt(adt, substs) = cx.tcx.type_of(did).kind() { if cx.tcx.has_attr(adt.did, sym::rustc_pass_by_value) { - let name = cx.tcx.item_name(adt.did).to_ident_string(); - let param = - substs.first().map(|s| format!("<{}>", s)).unwrap_or("".to_string()); - return Some(format!("{}{}", name, param)); + return Some(cx.tcx.def_path_str_with_substs(adt.did, substs)); } } } @@ -70,22 +68,29 @@ fn path_for_pass_by_value(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> Option<Stri None } -fn gen_args(segment: &PathSegment<'_>) -> String { +fn gen_args(cx: &LateContext<'_>, segment: &PathSegment<'_>) -> String { if let Some(args) = &segment.args { - let lifetimes = args + let params = args .args .iter() - .filter_map(|arg| { - if let GenericArg::Lifetime(lt) = arg { - Some(lt.name.ident().to_string()) - } else { - None + .filter_map(|arg| match arg { + GenericArg::Lifetime(lt) => Some(lt.name.ident().to_string()), + GenericArg::Type(ty) => { + let snippet = + cx.tcx.sess.source_map().span_to_snippet(ty.span).unwrap_or_default(); + Some(snippet) } + GenericArg::Const(c) => { + let snippet = + cx.tcx.sess.source_map().span_to_snippet(c.span).unwrap_or_default(); + Some(snippet) + } + _ => None, }) .collect::<Vec<_>>(); - if !lifetimes.is_empty() { - return format!("<{}>", lifetimes.join(", ")); + if !params.is_empty() { + return format!("<{}>", params.join(", ")); } } diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs index 293464c07ef78..f8ab0f056d79f 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs @@ -98,4 +98,19 @@ impl CustomStruct { } } +#[rustc_pass_by_value] +struct WithParameters<T, const N: usize, M = u32> { + slice: [T; N], + m: M, +} + +impl<T> WithParameters<T, 1> { + fn test( + value: WithParameters<T, 1>, + reference: &WithParameters<T, 1>, //~ ERROR passing `WithParameters<T, 1>` by reference + reference_with_m: &WithParameters<T, 1, u32>, //~ ERROR passing `WithParameters<T, 1, u32>` by reference + ) { + } +} + fn main() {} diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr index dbb9180ed7d2f..c5307f0f67d9b 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr @@ -100,5 +100,17 @@ error: passing `CustomAlias<>` by reference LL | reference: &CustomAlias, | ^^^^^^^^^^^^ help: try passing by value: `CustomAlias<>` -error: aborting due to 16 previous errors +error: passing `WithParameters<T, 1>` by reference + --> $DIR/rustc_pass_by_value.rs:110:20 + | +LL | reference: &WithParameters<T, 1>, + | ^^^^^^^^^^^^^^^^^^^^^ help: try passing by value: `WithParameters<T, 1>` + +error: passing `WithParameters<T, 1, u32>` by reference + --> $DIR/rustc_pass_by_value.rs:111:27 + | +LL | reference_with_m: &WithParameters<T, 1, u32>, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try passing by value: `WithParameters<T, 1, u32>` + +error: aborting due to 18 previous errors diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs index 1be01e21bd5f1..2868517774d46 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.rs @@ -37,4 +37,18 @@ impl Foo { fn with_ref(&self) {} //~ ERROR passing `Foo` by reference } +#[rustc_pass_by_value] +struct WithParameters<T, const N: usize, M = u32> { + slice: [T; N], + m: M, +} + +impl<T> WithParameters<T, 1> { + fn with_ref(&self) {} //~ ERROR passing `WithParameters<T, 1_usize>` by reference +} + +impl<T> WithParameters<T, 1, u8> { + fn with_ref(&self) {} //~ ERROR passing `WithParameters<T, 1_usize, u8>` by reference +} + fn main() {} diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr index 965e79d962cdf..54a7cf7cab757 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value_self.stderr @@ -22,5 +22,17 @@ error: passing `Foo` by reference LL | fn with_ref(&self) {} | ^^^^^ help: try passing by value: `Foo` -error: aborting due to 3 previous errors +error: passing `WithParameters<T, 1_usize>` by reference + --> $DIR/rustc_pass_by_value_self.rs:47:17 + | +LL | fn with_ref(&self) {} + | ^^^^^ help: try passing by value: `WithParameters<T, 1_usize>` + +error: passing `WithParameters<T, 1_usize, u8>` by reference + --> $DIR/rustc_pass_by_value_self.rs:51:17 + | +LL | fn with_ref(&self) {} + | ^^^^^ help: try passing by value: `WithParameters<T, 1_usize, u8>` + +error: aborting due to 5 previous errors From 2728af7bc02ab48bf4dd861cb69b5b786ecb261d Mon Sep 17 00:00:00 2001 From: Mahdi Dibaiee <mdibaiee@pm.me> Date: Tue, 11 Jan 2022 21:28:04 +0000 Subject: [PATCH 19/24] rustc_pass_by_value: handle inferred generic types (with _) --- compiler/rustc_lint/src/pass_by_value.rs | 14 +++++--------- .../internal-lints/rustc_pass_by_value.rs | 8 +++++--- .../internal-lints/rustc_pass_by_value.stderr | 18 +++++++++++++++--- 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_lint/src/pass_by_value.rs b/compiler/rustc_lint/src/pass_by_value.rs index 3435a5a6c82db..26d0560bf89bb 100644 --- a/compiler/rustc_lint/src/pass_by_value.rs +++ b/compiler/rustc_lint/src/pass_by_value.rs @@ -73,19 +73,15 @@ fn gen_args(cx: &LateContext<'_>, segment: &PathSegment<'_>) -> String { let params = args .args .iter() - .filter_map(|arg| match arg { - GenericArg::Lifetime(lt) => Some(lt.name.ident().to_string()), + .map(|arg| match arg { + GenericArg::Lifetime(lt) => lt.name.ident().to_string(), GenericArg::Type(ty) => { - let snippet = - cx.tcx.sess.source_map().span_to_snippet(ty.span).unwrap_or_default(); - Some(snippet) + cx.tcx.sess.source_map().span_to_snippet(ty.span).unwrap_or_default() } GenericArg::Const(c) => { - let snippet = - cx.tcx.sess.source_map().span_to_snippet(c.span).unwrap_or_default(); - Some(snippet) + cx.tcx.sess.source_map().span_to_snippet(c.span).unwrap_or_default() } - _ => None, + GenericArg::Infer(_) => String::from("_"), }) .collect::<Vec<_>>(); diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs index f8ab0f056d79f..402c41f376602 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.rs @@ -105,11 +105,13 @@ struct WithParameters<T, const N: usize, M = u32> { } impl<T> WithParameters<T, 1> { - fn test( + fn test<'a>( value: WithParameters<T, 1>, - reference: &WithParameters<T, 1>, //~ ERROR passing `WithParameters<T, 1>` by reference + reference: &'a WithParameters<T, 1>, //~ ERROR passing `WithParameters<T, 1>` by reference reference_with_m: &WithParameters<T, 1, u32>, //~ ERROR passing `WithParameters<T, 1, u32>` by reference - ) { + ) -> &'a WithParameters<T, 1> { + //~^ ERROR passing `WithParameters<T, 1>` by reference + reference as &WithParameters<_, 1> //~ ERROR passing `WithParameters<_, 1>` by reference } } diff --git a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr index c5307f0f67d9b..7f6e57b38f38d 100644 --- a/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr +++ b/src/test/ui-fulldeps/internal-lints/rustc_pass_by_value.stderr @@ -103,8 +103,8 @@ LL | reference: &CustomAlias, error: passing `WithParameters<T, 1>` by reference --> $DIR/rustc_pass_by_value.rs:110:20 | -LL | reference: &WithParameters<T, 1>, - | ^^^^^^^^^^^^^^^^^^^^^ help: try passing by value: `WithParameters<T, 1>` +LL | reference: &'a WithParameters<T, 1>, + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try passing by value: `WithParameters<T, 1>` error: passing `WithParameters<T, 1, u32>` by reference --> $DIR/rustc_pass_by_value.rs:111:27 @@ -112,5 +112,17 @@ error: passing `WithParameters<T, 1, u32>` by reference LL | reference_with_m: &WithParameters<T, 1, u32>, | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try passing by value: `WithParameters<T, 1, u32>` -error: aborting due to 18 previous errors +error: passing `WithParameters<T, 1>` by reference + --> $DIR/rustc_pass_by_value.rs:112:10 + | +LL | ) -> &'a WithParameters<T, 1> { + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try passing by value: `WithParameters<T, 1>` + +error: passing `WithParameters<_, 1>` by reference + --> $DIR/rustc_pass_by_value.rs:114:22 + | +LL | reference as &WithParameters<_, 1> + | ^^^^^^^^^^^^^^^^^^^^^ help: try passing by value: `WithParameters<_, 1>` + +error: aborting due to 20 previous errors From c84f2b27d3a72526bce7ff89c7be741d2daef123 Mon Sep 17 00:00:00 2001 From: Noah Lev <camelidcamel@gmail.com> Date: Tue, 11 Jan 2022 16:23:19 -0800 Subject: [PATCH 20/24] Remove some unnecessary uses of `FieldDef::ident` --- src/librustdoc/passes/collect_intra_doc_links.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index af62232e792ac..fb055bb224408 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -398,8 +398,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } match tcx.type_of(did).kind() { ty::Adt(def, _) if def.is_enum() => { - if let Some(field) = - def.all_fields().find(|f| f.ident(tcx).name == variant_field_name) + if let Some(field) = def.all_fields().find(|f| f.name == variant_field_name) { Ok((ty_res, Some(ItemFragment(FragmentKind::VariantField, field.did)))) } else { @@ -770,11 +769,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ty::Adt(def, _) if !def.is_enum() => def, _ => return None, }; - let field = def - .non_enum_variant() - .fields - .iter() - .find(|item| item.ident(tcx).name == item_name)?; + let field = + def.non_enum_variant().fields.iter().find(|item| item.name == item_name)?; Some((root_res, ItemFragment(FragmentKind::StructField, field.did))) } Res::Def(DefKind::Trait, did) => tcx From 962582981feca4a17dbe1fe560c924d9d4c664a0 Mon Sep 17 00:00:00 2001 From: lcnr <rust@lcnr.de> Date: Wed, 12 Jan 2022 16:09:01 +0100 Subject: [PATCH 21/24] remove unused FIXME --- compiler/rustc_index/src/lib.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/compiler/rustc_index/src/lib.rs b/compiler/rustc_index/src/lib.rs index 359b1859c6889..7919e40925392 100644 --- a/compiler/rustc_index/src/lib.rs +++ b/compiler/rustc_index/src/lib.rs @@ -9,7 +9,3 @@ pub mod bit_set; pub mod interval; pub mod vec; - -// FIXME(#56935): Work around ICEs during cross-compilation. -#[allow(unused)] -extern crate rustc_macros; From 51d7665be170df3abbaf4bc0fcf435820011d214 Mon Sep 17 00:00:00 2001 From: Andy Russell <arussell123@gmail.com> Date: Wed, 12 Jan 2022 12:46:18 -0500 Subject: [PATCH 22/24] rustdoc: remove hand-rolled isatty --- Cargo.lock | 1 + src/librustdoc/Cargo.toml | 1 + src/librustdoc/lib.rs | 56 +++++++++++---------------------------- 3 files changed, 17 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ef9f91fdb434b..86f37c3087a09 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4679,6 +4679,7 @@ version = "0.0.0" dependencies = [ "arrayvec", "askama", + "atty", "expect-test", "itertools 0.9.0", "minifier", diff --git a/src/librustdoc/Cargo.toml b/src/librustdoc/Cargo.toml index 5025342c1d68c..b14ecaf0197b5 100644 --- a/src/librustdoc/Cargo.toml +++ b/src/librustdoc/Cargo.toml @@ -9,6 +9,7 @@ path = "lib.rs" [dependencies] arrayvec = { version = "0.7", default-features = false } askama = { version = "0.11", default-features = false } +atty = "0.2" pulldown-cmark = { version = "0.9", default-features = false } minifier = "0.0.41" rayon = "1.3.1" diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index d7741c4fde239..572474f9307bf 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -70,7 +70,8 @@ extern crate tikv_jemalloc_sys; use tikv_jemalloc_sys as jemalloc_sys; use std::default::Default; -use std::env; +use std::env::{self, VarError}; +use std::io; use std::process; use rustc_driver::{abort_on_err, describe_lints}; @@ -178,47 +179,20 @@ pub fn main() { } fn init_logging() { - use std::io; - - // FIXME remove these and use winapi 0.3 instead - // Duplicates: bootstrap/compile.rs, librustc_errors/emitter.rs, rustc_driver/lib.rs - #[cfg(unix)] - fn stdout_isatty() -> bool { - extern crate libc; - unsafe { libc::isatty(libc::STDOUT_FILENO) != 0 } - } - - #[cfg(windows)] - fn stdout_isatty() -> bool { - extern crate winapi; - use winapi::um::consoleapi::GetConsoleMode; - use winapi::um::processenv::GetStdHandle; - use winapi::um::winbase::STD_OUTPUT_HANDLE; - - unsafe { - let handle = GetStdHandle(STD_OUTPUT_HANDLE); - let mut out = 0; - GetConsoleMode(handle, &mut out) != 0 - } - } - - let color_logs = match std::env::var("RUSTDOC_LOG_COLOR") { - Ok(value) => match value.as_ref() { - "always" => true, - "never" => false, - "auto" => stdout_isatty(), - _ => early_error( - ErrorOutputType::default(), - &format!( - "invalid log color value '{}': expected one of always, never, or auto", - value - ), - ), - }, - Err(std::env::VarError::NotPresent) => stdout_isatty(), - Err(std::env::VarError::NotUnicode(_value)) => early_error( + let color_logs = match std::env::var("RUSTDOC_LOG_COLOR").as_deref() { + Ok("always") => true, + Ok("never") => false, + Ok("auto") | Err(VarError::NotPresent) => atty::is(atty::Stream::Stdout), + Ok(value) => early_error( + ErrorOutputType::default(), + &format!("invalid log color value '{}': expected one of always, never, or auto", value), + ), + Err(VarError::NotUnicode(value)) => early_error( ErrorOutputType::default(), - "non-Unicode log color value: expected one of always, never, or auto", + &format!( + "invalid log color value '{}': expected one of always, never, or auto", + value.to_string_lossy() + ), ), }; let filter = tracing_subscriber::EnvFilter::from_env("RUSTDOC_LOG"); From 9ff8ae097e88937a4aa6cf62d73471e479bac1df Mon Sep 17 00:00:00 2001 From: Mahdi Dibaiee <mdibaiee@pm.me> Date: Tue, 11 Jan 2022 22:20:01 +0000 Subject: [PATCH 23/24] rustdoc: fix intra-link for generic trait impls --- .../passes/collect_intra_doc_links.rs | 13 ++++++++++++- .../rustdoc/intra-doc/generic-trait-impl.rs | 19 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 src/test/rustdoc/intra-doc/generic-trait-impl.rs diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 9d1a8b3f80fec..0bc10404dc0f0 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -903,7 +903,18 @@ fn traits_implemented_by<'a>( ty ); // Fast path: if this is a primitive simple `==` will work - let saw_impl = impl_type == ty; + // NOTE: the `match` is necessary; see #92662. + // this allows us to ignore generics because the user input + // may not include the generic placeholders + // e.g. this allows us to match Foo (user comment) with Foo<T> (actual type) + let saw_impl = impl_type == ty + || match (impl_type.kind(), ty.kind()) { + (ty::Adt(impl_def, _), ty::Adt(ty_def, _)) => { + debug!("impl def_id: {:?}, ty def_id: {:?}", impl_def.did, ty_def.did); + impl_def.did == ty_def.did + } + _ => false, + }; if saw_impl { Some(trait_) } else { None } }) diff --git a/src/test/rustdoc/intra-doc/generic-trait-impl.rs b/src/test/rustdoc/intra-doc/generic-trait-impl.rs new file mode 100644 index 0000000000000..c3d3f8b386607 --- /dev/null +++ b/src/test/rustdoc/intra-doc/generic-trait-impl.rs @@ -0,0 +1,19 @@ +#![deny(rustdoc::broken_intra_doc_links)] + +// Test intra-doc links on trait implementations with generics + +use std::marker::PhantomData; + +pub trait Bar<T> { + fn bar(&self); +} + +pub struct Foo<U>(PhantomData<U>); + +impl<T, U> Bar<T> for Foo<U> { + fn bar(&self) {} +} + +// @has generic_trait_impl/fn.main.html '//a[@href="struct.Foo.html#method.bar"]' 'Foo::bar' +/// link to [`Foo::bar`] +pub fn main() {} From ae20500d76e36b5e8692af499830c4707f713bc0 Mon Sep 17 00:00:00 2001 From: Mahdi Dibaiee <mdibaiee@pm.me> Date: Thu, 13 Jan 2022 08:10:08 +0000 Subject: [PATCH 24/24] rustdoc: add intra-doc trait impl test for extern types --- src/test/rustdoc/intra-doc/extern-type.rs | 24 +++++++++++++++++-- .../rustdoc/intra-doc/generic-trait-impl.rs | 1 + 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/test/rustdoc/intra-doc/extern-type.rs b/src/test/rustdoc/intra-doc/extern-type.rs index f37ae62dde1aa..ab088ab789d43 100644 --- a/src/test/rustdoc/intra-doc/extern-type.rs +++ b/src/test/rustdoc/intra-doc/extern-type.rs @@ -4,14 +4,34 @@ extern { pub type ExternType; } +pub trait T { + fn test(&self) {} +} + +pub trait G<N> { + fn g(&self, n: N) {} +} + impl ExternType { - pub fn f(&self) { + pub fn f(&self) {} +} - } +impl T for ExternType { + fn test(&self) {} +} + +impl G<usize> for ExternType { + fn g(&self, n: usize) {} } // @has 'extern_type/foreigntype.ExternType.html' // @has 'extern_type/fn.links_to_extern_type.html' \ // 'href="foreigntype.ExternType.html#method.f"' +// @has 'extern_type/fn.links_to_extern_type.html' \ +// 'href="foreigntype.ExternType.html#method.test"' +// @has 'extern_type/fn.links_to_extern_type.html' \ +// 'href="foreigntype.ExternType.html#method.g"' /// See also [ExternType::f] +/// See also [ExternType::test] +/// See also [ExternType::g] pub fn links_to_extern_type() {} diff --git a/src/test/rustdoc/intra-doc/generic-trait-impl.rs b/src/test/rustdoc/intra-doc/generic-trait-impl.rs index c3d3f8b386607..ba8595abfa959 100644 --- a/src/test/rustdoc/intra-doc/generic-trait-impl.rs +++ b/src/test/rustdoc/intra-doc/generic-trait-impl.rs @@ -1,6 +1,7 @@ #![deny(rustdoc::broken_intra_doc_links)] // Test intra-doc links on trait implementations with generics +// regression test for issue #92662 use std::marker::PhantomData;