From fc52b47e8013da325fc0a0f5d3897143243ba206 Mon Sep 17 00:00:00 2001 From: Jamie Cunliffe Date: Tue, 30 Jun 2020 09:57:59 +0100 Subject: [PATCH 01/23] va_args implementation for AAPCS. Implement the va args in codegen for AAPCS, this will be used as the default va_args implementation for AArch64 rather than the va_args llvm-ir as it currently is. Copyright (c) 2020, Arm Limited. --- src/librustc_codegen_llvm/builder.rs | 7 +- src/librustc_codegen_llvm/va_arg.rs | 82 ++++++++++++++++++- .../c-link-to-rust-va-list-fn/checkrust.rs | 55 +++++++++++++ .../c-link-to-rust-va-list-fn/test.c | 10 +++ 4 files changed, 151 insertions(+), 3 deletions(-) diff --git a/src/librustc_codegen_llvm/builder.rs b/src/librustc_codegen_llvm/builder.rs index ba285b5ef38d1..01c2ab32d6ada 100644 --- a/src/librustc_codegen_llvm/builder.rs +++ b/src/librustc_codegen_llvm/builder.rs @@ -1279,7 +1279,12 @@ impl Builder<'a, 'll, 'tcx> { self.call(lifetime_intrinsic, &[self.cx.const_u64(size), ptr], None); } - fn phi(&mut self, ty: &'ll Type, vals: &[&'ll Value], bbs: &[&'ll BasicBlock]) -> &'ll Value { + pub(crate) fn phi( + &mut self, + ty: &'ll Type, + vals: &[&'ll Value], + bbs: &[&'ll BasicBlock], + ) -> &'ll Value { assert_eq!(vals.len(), bbs.len()); let phi = unsafe { llvm::LLVMBuildPhi(self.llbuilder, ty, UNNAMED) }; unsafe { diff --git a/src/librustc_codegen_llvm/va_arg.rs b/src/librustc_codegen_llvm/va_arg.rs index 8bc3579800ea8..8c0d256df2357 100644 --- a/src/librustc_codegen_llvm/va_arg.rs +++ b/src/librustc_codegen_llvm/va_arg.rs @@ -3,8 +3,9 @@ use crate::type_::Type; use crate::type_of::LayoutLlvmExt; use crate::value::Value; use rustc_codegen_ssa::mir::operand::OperandRef; -use rustc_codegen_ssa::traits::{ - BaseTypeMethods, BuilderMethods, ConstMethods, DerivedTypeMethods, +use rustc_codegen_ssa::{ + common::IntPredicate, + traits::{BaseTypeMethods, BuilderMethods, ConstMethods, DerivedTypeMethods}, }; use rustc_middle::ty::layout::HasTyCtxt; use rustc_middle::ty::Ty; @@ -89,6 +90,82 @@ fn emit_ptr_va_arg( } } +fn emit_aapcs_va_arg( + bx: &mut Builder<'a, 'll, 'tcx>, + list: OperandRef<'tcx, &'ll Value>, + target_ty: Ty<'tcx>, +) -> &'ll Value { + // Implementation of the AAPCS64 calling convention for va_args see + // https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst + let va_list_addr = list.immediate(); + let layout = bx.cx.layout_of(target_ty); + + let mut maybe_reg = bx.build_sibling_block("va_arg.maybe_reg"); + let mut in_reg = bx.build_sibling_block("va_arg.in_reg"); + let mut on_stack = bx.build_sibling_block("va_arg.on_stack"); + let mut end = bx.build_sibling_block("va_arg.end"); + let zero = bx.const_i32(0); + let offset_align = Align::from_bytes(4).unwrap(); + assert!(&*bx.tcx().sess.target.target.target_endian == "little"); + + let gr_type = target_ty.is_any_ptr() || target_ty.is_integral(); + let (reg_off, reg_top_index, slot_size) = if gr_type { + let gr_offs = bx.struct_gep(va_list_addr, 7); + let nreg = (layout.size.bytes() + 7) / 8; + (gr_offs, 3, nreg * 8) + } else { + let vr_off = bx.struct_gep(va_list_addr, 9); + let nreg = (layout.size.bytes() + 15) / 16; + (vr_off, 5, nreg * 16) + }; + + // if the offset >= 0 then the value will be on the stack + let mut reg_off_v = bx.load(reg_off, offset_align); + let use_stack = bx.icmp(IntPredicate::IntSGE, reg_off_v, zero); + bx.cond_br(use_stack, &on_stack.llbb(), &maybe_reg.llbb()); + + // The value at this point might be in a register, but there is a chance that + // it could be on the stack so we have to update the offset and then check + // the offset again. + + if layout.align.abi.bytes() > 8 { + assert!(layout.align.abi.bytes() <= 16); + reg_off_v = maybe_reg.add(reg_off_v, bx.const_i32(15)); + reg_off_v = maybe_reg.and(reg_off_v, bx.const_i32(-16)); + } + let new_reg_off_v = maybe_reg.add(reg_off_v, bx.const_i32(slot_size as i32)); + + maybe_reg.store(new_reg_off_v, reg_off, offset_align); + + // Check to see if we have overflowed the registers as a result of this. + // If we have then we need to use the stack for this value + let use_stack = maybe_reg.icmp(IntPredicate::IntSGT, new_reg_off_v, zero); + maybe_reg.cond_br(use_stack, &on_stack.llbb(), &in_reg.llbb()); + + let top = in_reg.struct_gep(va_list_addr, reg_top_index); + let top = in_reg.load(top, bx.tcx().data_layout.pointer_align.abi); + + // reg_value = *(@top + reg_off_v); + let top = in_reg.gep(top, &[reg_off_v]); + let top = in_reg.bitcast(top, bx.cx.type_ptr_to(layout.llvm_type(bx))); + let reg_value = in_reg.load(top, layout.align.abi); + in_reg.br(&end.llbb()); + + // On Stack block + let stack_value = + emit_ptr_va_arg(&mut on_stack, list, target_ty, false, Align::from_bytes(8).unwrap(), true); + on_stack.br(&end.llbb()); + + let val = end.phi( + layout.immediate_llvm_type(bx), + &[reg_value, stack_value], + &[&in_reg.llbb(), &on_stack.llbb()], + ); + + *bx = end; + val +} + pub(super) fn emit_va_arg( bx: &mut Builder<'a, 'll, 'tcx>, addr: OperandRef<'tcx, &'ll Value>, @@ -115,6 +192,7 @@ pub(super) fn emit_va_arg( ("aarch64", _) if target.target_os == "ios" => { emit_ptr_va_arg(bx, addr, target_ty, false, Align::from_bytes(8).unwrap(), true) } + ("aarch64", _) => emit_aapcs_va_arg(bx, addr, target_ty), // Windows x86_64 ("x86_64", true) => { let target_ty_size = bx.cx.size_of(target_ty).bytes(); diff --git a/src/test/run-make-fulldeps/c-link-to-rust-va-list-fn/checkrust.rs b/src/test/run-make-fulldeps/c-link-to-rust-va-list-fn/checkrust.rs index a0a5b141ec0e1..5830ef033d389 100644 --- a/src/test/run-make-fulldeps/c-link-to-rust-va-list-fn/checkrust.rs +++ b/src/test/run-make-fulldeps/c-link-to-rust-va-list-fn/checkrust.rs @@ -91,3 +91,58 @@ pub unsafe extern "C" fn check_varargs_1(_: c_int, mut ap: ...) -> usize { pub unsafe extern "C" fn check_varargs_2(_: c_int, _ap: ...) -> usize { 0 } + +#[no_mangle] +pub unsafe extern "C" fn check_varargs_3(_: c_int, mut ap: ...) -> usize { + continue_if!(ap.arg::() == 1); + continue_if!(ap.arg::() == 2); + continue_if!(ap.arg::() == 3); + continue_if!(ap.arg::() == 4); + continue_if!(ap.arg::() == 5); + continue_if!(ap.arg::() == 6); + continue_if!(ap.arg::() == 7); + continue_if!(ap.arg::() == 8); + continue_if!(ap.arg::() == 9); + continue_if!(ap.arg::() == 10); + 0 +} + +#[no_mangle] +pub unsafe extern "C" fn check_varargs_4(_: c_double, mut ap: ...) -> usize { + continue_if!(ap.arg::() == 1.0); + continue_if!(ap.arg::() == 2.0); + continue_if!(ap.arg::() == 3.0); + continue_if!(ap.arg::() == 4.0); + continue_if!(ap.arg::() == 5.0); + continue_if!(ap.arg::() == 6.0); + continue_if!(ap.arg::() == 7.0); + continue_if!(ap.arg::() == 8.0); + continue_if!(ap.arg::() == 9.0); + continue_if!(ap.arg::() == 10.0); + 0 +} + +#[no_mangle] +pub unsafe extern "C" fn check_varargs_5(_: c_int, mut ap: ...) -> usize { + continue_if!(ap.arg::() == 1.0); + continue_if!(ap.arg::() == 1); + continue_if!(ap.arg::() == 2.0); + continue_if!(ap.arg::() == 2); + continue_if!(ap.arg::() == 3.0); + continue_if!(ap.arg::() == 3); + continue_if!(ap.arg::() == 4.0); + continue_if!(ap.arg::() == 4); + continue_if!(ap.arg::() == 5); + continue_if!(ap.arg::() == 5.0); + continue_if!(ap.arg::() == 6); + continue_if!(ap.arg::() == 6.0); + continue_if!(ap.arg::() == 7); + continue_if!(ap.arg::() == 7.0); + continue_if!(ap.arg::() == 8); + continue_if!(ap.arg::() == 8.0); + continue_if!(ap.arg::() == 9); + continue_if!(ap.arg::() == 9.0); + continue_if!(ap.arg::() == 10); + continue_if!(ap.arg::() == 10.0); + 0 +} diff --git a/src/test/run-make-fulldeps/c-link-to-rust-va-list-fn/test.c b/src/test/run-make-fulldeps/c-link-to-rust-va-list-fn/test.c index 91b060dce26f4..5bdb51680a656 100644 --- a/src/test/run-make-fulldeps/c-link-to-rust-va-list-fn/test.c +++ b/src/test/run-make-fulldeps/c-link-to-rust-va-list-fn/test.c @@ -11,6 +11,9 @@ extern size_t check_list_copy_0(va_list ap); extern size_t check_varargs_0(int fixed, ...); extern size_t check_varargs_1(int fixed, ...); extern size_t check_varargs_2(int fixed, ...); +extern size_t check_varargs_3(int fixed, ...); +extern size_t check_varargs_4(double fixed, ...); +extern size_t check_varargs_5(int fixed, ...); int test_rust(size_t (*fn)(va_list), ...) { size_t ret = 0; @@ -36,5 +39,12 @@ int main(int argc, char* argv[]) { assert(check_varargs_2(0, "All", "of", "these", "are", "ignored", ".") == 0); + assert(check_varargs_3(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10) == 0); + + assert(check_varargs_4(0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0) == 0); + + assert(check_varargs_5(0, 1.0, 1, 2.0, 2, 3.0, 3, 4.0, 4, 5, 5.0, 6, 6.0, 7, 7.0, 8, 8.0, + 9, 9.0, 10, 10.0) == 0); + return 0; } From 31c7aae1136b4d80f0256e269ca8086fd3fbdddd Mon Sep 17 00:00:00 2001 From: Andrew Paverd Date: Tue, 14 Jul 2020 15:27:42 +0100 Subject: [PATCH 02/23] Stabilize control-flow-guard codegen option --- src/bootstrap/builder.rs | 2 +- src/doc/rustc/src/codegen-options/index.md | 12 ++++++++++++ src/librustc_codegen_llvm/context.rs | 2 +- src/librustc_codegen_ssa/back/link.rs | 2 +- src/librustc_interface/tests.rs | 2 +- src/librustc_session/config.rs | 2 +- src/librustc_session/options.rs | 4 ++-- src/test/codegen/cfguard-checks.rs | 2 +- src/test/codegen/cfguard-disabled.rs | 2 +- src/test/codegen/cfguard-nochecks.rs | 2 +- src/test/codegen/cfguard-non-msvc.rs | 2 +- src/test/ui/cfguard-run.rs | 2 +- 12 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 557fb1aa550a5..276e7ac558e31 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -1231,7 +1231,7 @@ impl<'a> Builder<'a> { && self.config.control_flow_guard && compiler.stage >= 1 { - rustflags.arg("-Zcontrol-flow-guard"); + rustflags.arg("-Ccontrol-flow-guard"); } // For `cargo doc` invocations, make rustdoc print the Rust version into the docs diff --git a/src/doc/rustc/src/codegen-options/index.md b/src/doc/rustc/src/codegen-options/index.md index 0b4bb05c1db23..35904e15d3fee 100644 --- a/src/doc/rustc/src/codegen-options/index.md +++ b/src/doc/rustc/src/codegen-options/index.md @@ -42,6 +42,18 @@ generated code, but may be slower to compile. The default value, if not specified, is 16 for non-incremental builds. For incremental builds the default is 256 which allows caching to be more granular. +## control-flow-guard + +This flag controls whether LLVM enables the Windows [Control Flow +Guard](https://docs.microsoft.com/en-us/windows/win32/secbp/control-flow-guard) +platform security feature. This flag is currently ignored for non-Windows targets. +It takes one of the following values: + +* `y`, `yes`, `on`, `checks`, or no value: enable Control Flow Guard. +* `nochecks`: emit Control Flow Guard metadata without runtime enforcement checks (this +should only be used for testing purposes as it does not provide security enforcement). +* `n`, `no`, `off`: do not enable Control Flow Guard (the default). + ## debug-assertions This flag lets you turn `cfg(debug_assertions)` [conditional diff --git a/src/librustc_codegen_llvm/context.rs b/src/librustc_codegen_llvm/context.rs index 21ba97d15a485..a07f6c64edcb1 100644 --- a/src/librustc_codegen_llvm/context.rs +++ b/src/librustc_codegen_llvm/context.rs @@ -190,7 +190,7 @@ pub unsafe fn create_module( // Control Flow Guard is currently only supported by the MSVC linker on Windows. if sess.target.target.options.is_like_msvc { - match sess.opts.debugging_opts.control_flow_guard { + match sess.opts.cg.control_flow_guard { CFGuard::Disabled => {} CFGuard::NoChecks => { // Set `cfguard=1` module flag to emit metadata only. diff --git a/src/librustc_codegen_ssa/back/link.rs b/src/librustc_codegen_ssa/back/link.rs index 3adaa07db91b0..7f3aa4be9fdb6 100644 --- a/src/librustc_codegen_ssa/back/link.rs +++ b/src/librustc_codegen_ssa/back/link.rs @@ -1700,7 +1700,7 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( } // OBJECT-FILES-NO, AUDIT-ORDER - if sess.opts.debugging_opts.control_flow_guard != CFGuard::Disabled { + if sess.opts.cg.control_flow_guard != CFGuard::Disabled { cmd.control_flow_guard(); } diff --git a/src/librustc_interface/tests.rs b/src/librustc_interface/tests.rs index 651a77912c6d0..dd09ce9e3dddf 100644 --- a/src/librustc_interface/tests.rs +++ b/src/librustc_interface/tests.rs @@ -420,6 +420,7 @@ fn test_codegen_options_tracking_hash() { // Make sure that changing a [TRACKED] option changes the hash. // This list is in alphabetical order. tracked!(code_model, Some(CodeModel::Large)); + tracked!(control_flow_guard, CFGuard::Checks); tracked!(debug_assertions, Some(true)); tracked!(debuginfo, 0xdeadbeef); tracked!(embed_bitcode, false); @@ -537,7 +538,6 @@ fn test_debugging_options_tracking_hash() { tracked!(binary_dep_depinfo, true); tracked!(chalk, true); tracked!(codegen_backend, Some("abc".to_string())); - tracked!(control_flow_guard, CFGuard::Checks); tracked!(crate_attr, vec!["abc".to_string()]); tracked!(debug_macros, true); tracked!(dep_info_omit_d_target, true); diff --git a/src/librustc_session/config.rs b/src/librustc_session/config.rs index c5a866817cb4a..dbf5b7ece08fe 100644 --- a/src/librustc_session/config.rs +++ b/src/librustc_session/config.rs @@ -103,7 +103,7 @@ pub enum Strip { Symbols, } -/// The different settings that the `-Z control-flow-guard` flag can have. +/// The different settings that the `-C control-flow-guard` flag can have. #[derive(Clone, Copy, PartialEq, Hash, Debug)] pub enum CFGuard { /// Do not emit Control Flow Guard metadata or checks. diff --git a/src/librustc_session/options.rs b/src/librustc_session/options.rs index 2ad7d09cbf415..62f6fca97466d 100644 --- a/src/librustc_session/options.rs +++ b/src/librustc_session/options.rs @@ -692,6 +692,8 @@ options! {CodegenOptions, CodegenSetter, basic_codegen_options, "choose the code model to use (`rustc --print code-models` for details)"), codegen_units: Option = (None, parse_opt_uint, [UNTRACKED], "divide crate into N units to optimize in parallel"), + control_flow_guard: CFGuard = (CFGuard::Disabled, parse_cfguard, [TRACKED], + "use Windows Control Flow Guard (default: no)"), debug_assertions: Option = (None, parse_opt_bool, [TRACKED], "explicitly enable the `cfg(debug_assertions)` directive"), debuginfo: usize = (0, parse_uint, [TRACKED], @@ -809,8 +811,6 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "enable the experimental Chalk-based trait solving engine"), codegen_backend: Option = (None, parse_opt_string, [TRACKED], "the backend to use"), - control_flow_guard: CFGuard = (CFGuard::Disabled, parse_cfguard, [TRACKED], - "use Windows Control Flow Guard (default: no)"), crate_attr: Vec = (Vec::new(), parse_string_push, [TRACKED], "inject the given attribute in the crate"), debug_macros: bool = (false, parse_bool, [TRACKED], diff --git a/src/test/codegen/cfguard-checks.rs b/src/test/codegen/cfguard-checks.rs index 96a0a321199a3..571a2654bcbfd 100644 --- a/src/test/codegen/cfguard-checks.rs +++ b/src/test/codegen/cfguard-checks.rs @@ -1,4 +1,4 @@ -// compile-flags: -Z control-flow-guard=checks +// compile-flags: -C control-flow-guard=checks // only-msvc #![crate_type = "lib"] diff --git a/src/test/codegen/cfguard-disabled.rs b/src/test/codegen/cfguard-disabled.rs index 925e4e8e2d155..c3f8f4116819c 100644 --- a/src/test/codegen/cfguard-disabled.rs +++ b/src/test/codegen/cfguard-disabled.rs @@ -1,4 +1,4 @@ -// compile-flags: -Z control-flow-guard=no +// compile-flags: -C control-flow-guard=no // only-msvc #![crate_type = "lib"] diff --git a/src/test/codegen/cfguard-nochecks.rs b/src/test/codegen/cfguard-nochecks.rs index d7dc3d7e89eea..3847c3e81ed7a 100644 --- a/src/test/codegen/cfguard-nochecks.rs +++ b/src/test/codegen/cfguard-nochecks.rs @@ -1,4 +1,4 @@ -// compile-flags: -Z control-flow-guard=nochecks +// compile-flags: -C control-flow-guard=nochecks // only-msvc #![crate_type = "lib"] diff --git a/src/test/codegen/cfguard-non-msvc.rs b/src/test/codegen/cfguard-non-msvc.rs index 4008f0187a0b0..6278a951e35f1 100644 --- a/src/test/codegen/cfguard-non-msvc.rs +++ b/src/test/codegen/cfguard-non-msvc.rs @@ -1,4 +1,4 @@ -// compile-flags: -Z control-flow-guard +// compile-flags: -C control-flow-guard // ignore-msvc #![crate_type = "lib"] diff --git a/src/test/ui/cfguard-run.rs b/src/test/ui/cfguard-run.rs index 21368fad3b058..3c4f9a1f5ee2c 100644 --- a/src/test/ui/cfguard-run.rs +++ b/src/test/ui/cfguard-run.rs @@ -1,5 +1,5 @@ // run-pass -// compile-flags: -Z control-flow-guard +// compile-flags: -C control-flow-guard pub fn main() { println!("hello, world"); From 5300ca38d88b924f7108274c50d918728c53aecd Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Fri, 17 Jul 2020 21:12:47 +0200 Subject: [PATCH 03/23] cleanup ty_is_~non~_local_constructor --- .../traits/coherence.rs | 59 +++++++------------ 1 file changed, 21 insertions(+), 38 deletions(-) diff --git a/src/librustc_trait_selection/traits/coherence.rs b/src/librustc_trait_selection/traits/coherence.rs index 3ec7fe2bf25c6..3f286d733ed6d 100644 --- a/src/librustc_trait_selection/traits/coherence.rs +++ b/src/librustc_trait_selection/traits/coherence.rs @@ -418,7 +418,7 @@ fn orphan_check_trait_ref<'tcx>( .substs .types() .flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate)) - .find(|ty| ty_is_non_local_constructor(ty, in_crate).is_none()); + .find(|ty| ty_is_local_constructor(ty, in_crate)); debug!("orphan_check_trait_ref: uncovered ty local_type: `{:?}`", local_type); @@ -435,20 +435,16 @@ fn orphan_check_trait_ref<'tcx>( Err(OrphanCheckErr::NonLocalInputType(non_local_spans)) } +// FIXME: Return a `Vec` without `Option` here. fn ty_is_non_local(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, in_crate: InCrate) -> Option>> { - match ty_is_non_local_constructor(ty, in_crate) { - Some(ty) => { - if let Some(inner_tys) = fundamental_ty_inner_tys(tcx, ty) { - let tys: Vec<_> = inner_tys - .filter_map(|ty| ty_is_non_local(tcx, ty, in_crate)) - .flatten() - .collect(); - if tys.is_empty() { None } else { Some(tys) } - } else { - Some(vec![ty]) - } - } - None => None, + if ty_is_local_constructor(ty, in_crate) { + None + } else if let Some(inner_tys) = fundamental_ty_inner_tys(tcx, ty) { + let tys: Vec<_> = + inner_tys.filter_map(|ty| ty_is_non_local(tcx, ty, in_crate)).flatten().collect(); + if tys.is_empty() { None } else { Some(tys) } + } else { + Some(vec![ty]) } } @@ -493,8 +489,7 @@ fn def_id_is_local(def_id: DefId, in_crate: InCrate) -> bool { } } -// FIXME(eddyb) this can just return `bool` as it always returns `Some(ty)` or `None`. -fn ty_is_non_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> Option> { +fn ty_is_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> bool { debug!("ty_is_non_local_constructor({:?})", ty); match ty.kind { @@ -513,29 +508,17 @@ fn ty_is_non_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> Option> | ty::Never | ty::Tuple(..) | ty::Param(..) - | ty::Projection(..) => Some(ty), + | ty::Projection(..) => false, ty::Placeholder(..) | ty::Bound(..) | ty::Infer(..) => match in_crate { - InCrate::Local => Some(ty), + InCrate::Local => false, // The inference variable might be unified with a local // type in that remote crate. - InCrate::Remote => None, + InCrate::Remote => true, }, - ty::Adt(def, _) => { - if def_id_is_local(def.did, in_crate) { - None - } else { - Some(ty) - } - } - ty::Foreign(did) => { - if def_id_is_local(did, in_crate) { - None - } else { - Some(ty) - } - } + ty::Adt(def, _) => def_id_is_local(def.did, in_crate), + ty::Foreign(did) => def_id_is_local(did, in_crate), ty::Opaque(..) => { // This merits some explanation. // Normally, opaque types are not involed when performing @@ -553,7 +536,7 @@ fn ty_is_non_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> Option> // the underlying type *within the same crate*. When an // opaque type is used from outside the module // where it is declared, it should be impossible to observe - // anyything about it other than the traits that it implements. + // anything about it other than the traits that it implements. // // The alternative would be to look at the underlying type // to determine whether or not the opaque type itself should @@ -562,18 +545,18 @@ fn ty_is_non_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> Option> // to a remote type. This would violate the rule that opaque // types should be completely opaque apart from the traits // that they implement, so we don't use this behavior. - Some(ty) + false } ty::Dynamic(ref tt, ..) => { if let Some(principal) = tt.principal() { - if def_id_is_local(principal.def_id(), in_crate) { None } else { Some(ty) } + def_id_is_local(principal.def_id(), in_crate) } else { - Some(ty) + false } } - ty::Error(_) => None, + ty::Error(_) => true, ty::Closure(..) | ty::Generator(..) | ty::GeneratorWitness(..) => { bug!("ty_is_local invoked on unexpected type: {:?}", ty) From 1ac3713f256dd379d8fe24b09bc8ba6643ea41b4 Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Fri, 17 Jul 2020 21:40:47 +0200 Subject: [PATCH 04/23] refactor ty_is_non_local --- .../traits/coherence.rs | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/librustc_trait_selection/traits/coherence.rs b/src/librustc_trait_selection/traits/coherence.rs index 3f286d733ed6d..b3218f97e42b5 100644 --- a/src/librustc_trait_selection/traits/coherence.rs +++ b/src/librustc_trait_selection/traits/coherence.rs @@ -389,7 +389,7 @@ fn orphan_check_trait_ref<'tcx>( ) -> Vec> { // FIXME(eddyb) figure out if this is redundant with `ty_is_non_local`, // or maybe if this should be calling `ty_is_non_local_constructor`. - if ty_is_non_local(tcx, ty, in_crate).is_some() { + if !contained_non_local_types(tcx, ty, in_crate).is_empty() { if let Some(inner_tys) = fundamental_ty_inner_tys(tcx, ty) { return inner_tys .flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate)) @@ -408,8 +408,8 @@ fn orphan_check_trait_ref<'tcx>( .enumerate() { debug!("orphan_check_trait_ref: check ty `{:?}`", input_ty); - let non_local_tys = ty_is_non_local(tcx, input_ty, in_crate); - if non_local_tys.is_none() { + let non_local_tys = contained_non_local_types(tcx, input_ty, in_crate); + if non_local_tys.is_empty() { debug!("orphan_check_trait_ref: ty_is_local `{:?}`", input_ty); return Ok(()); } else if let ty::Param(_) = input_ty.kind { @@ -424,10 +424,9 @@ fn orphan_check_trait_ref<'tcx>( return Err(OrphanCheckErr::UncoveredTy(input_ty, local_type)); } - if let Some(non_local_tys) = non_local_tys { - for input_ty in non_local_tys { - non_local_spans.push((input_ty, i == 0)); - } + + for input_ty in non_local_tys { + non_local_spans.push((input_ty, i == 0)); } } // If we exit above loop, never found a local type. @@ -435,16 +434,20 @@ fn orphan_check_trait_ref<'tcx>( Err(OrphanCheckErr::NonLocalInputType(non_local_spans)) } -// FIXME: Return a `Vec` without `Option` here. -fn ty_is_non_local(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, in_crate: InCrate) -> Option>> { +/// Returns a list of relevant non-local types for `ty`. +/// +/// This is just `ty` itself unless `ty` is `#[fundamental]`, +/// in which case we recursively look into this type. +fn contained_non_local_types(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, in_crate: InCrate) -> Vec> { if ty_is_local_constructor(ty, in_crate) { - None - } else if let Some(inner_tys) = fundamental_ty_inner_tys(tcx, ty) { - let tys: Vec<_> = - inner_tys.filter_map(|ty| ty_is_non_local(tcx, ty, in_crate)).flatten().collect(); - if tys.is_empty() { None } else { Some(tys) } + Vec::new() } else { - Some(vec![ty]) + match fundamental_ty_inner_tys(tcx, ty) { + Some(inner_tys) => { + inner_tys.flat_map(|ty| contained_non_local_types(tcx, ty, in_crate)).collect() + } + None => vec![ty], + } } } From cfa3a330149b78c4d827b10d9ac4ff8d381e7342 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 11 Jul 2020 08:45:26 +0000 Subject: [PATCH 05/23] compiletest: Rewrite extract_lldb_version function This makes extract_lldb_version has the same version type like extract_gdb_version. This is technically a breaking change for rustc-dev users. But note that rustc-dev is a nightly component. --- src/tools/compiletest/src/common.rs | 2 +- src/tools/compiletest/src/header.rs | 23 +++----- src/tools/compiletest/src/main.rs | 90 ++++++++++------------------- src/tools/compiletest/src/tests.rs | 11 ++++ 4 files changed, 50 insertions(+), 76 deletions(-) diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index 703b87634cec3..4abb1db35a02f 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -268,7 +268,7 @@ pub struct Config { pub gdb_native_rust: bool, /// Version of LLDB - pub lldb_version: Option, + pub lldb_version: Option, /// Whether LLDB has native rust support pub lldb_native_rust: bool, diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index d6e28e93c9667..0ee11608dc5b3 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -188,16 +188,17 @@ impl EarlyProps { } fn ignore_lldb(config: &Config, line: &str) -> bool { - if let Some(ref actual_version) = config.lldb_version { - if line.starts_with("min-lldb-version") { - let min_version = line - .trim_end() - .rsplit(' ') - .next() - .expect("Malformed lldb version directive"); + if let Some(actual_version) = config.lldb_version { + if let Some(min_version) = line.strip_prefix("min-lldb-version:").map(str::trim) { + let min_version = min_version.parse().unwrap_or_else(|e| { + panic!( + "Unexpected format of LLDB version string: {}\n{:?}", + min_version, e + ); + }); // Ignore if actual version is smaller the minimum required // version - lldb_version_to_int(actual_version) < lldb_version_to_int(min_version) + actual_version < min_version } else if line.starts_with("rust-lldb") && !config.lldb_native_rust { true } else { @@ -944,12 +945,6 @@ impl Config { } } -pub fn lldb_version_to_int(version_string: &str) -> isize { - let error_string = - format!("Encountered LLDB version string with unexpected format: {}", version_string); - version_string.parse().expect(&error_string) -} - fn expand_variables(mut value: String, config: &Config) -> String { const CWD: &'static str = "{{cwd}}"; const SRC_BASE: &'static str = "{{src-base}}"; diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 97272f1a9c1b6..2b0ff0da9f5c5 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -165,8 +165,12 @@ pub fn parse_config(args: Vec) -> Config { let cdb = analyze_cdb(matches.opt_str("cdb"), &target); let (gdb, gdb_version, gdb_native_rust) = analyze_gdb(matches.opt_str("gdb"), &target, &android_cross_path); - let (lldb_version, lldb_native_rust) = extract_lldb_version(matches.opt_str("lldb-version")); - + let (lldb_version, lldb_native_rust) = matches + .opt_str("lldb-version") + .as_deref() + .and_then(extract_lldb_version) + .map(|(v, b)| (Some(v), b)) + .unwrap_or((None, false)); let color = match matches.opt_str("color").as_ref().map(|x| &**x) { Some("auto") | None => ColorConfig::AutoColor, Some("always") => ColorConfig::AlwaysColor, @@ -400,17 +404,14 @@ fn configure_lldb(config: &Config) -> Option { return None; } - if let Some(lldb_version) = config.lldb_version.as_ref() { - if lldb_version == "350" { - println!( - "WARNING: The used version of LLDB ({}) has a \ - known issue that breaks debuginfo tests. See \ - issue #32520 for more information. Skipping all \ - LLDB-based tests!", - lldb_version - ); - return None; - } + if let Some(350) = config.lldb_version { + println!( + "WARNING: The used version of LLDB (350) has a \ + known issue that breaks debuginfo tests. See \ + issue #32520 for more information. Skipping all \ + LLDB-based tests!", + ); + return None; } // Some older versions of LLDB seem to have problems with multiple @@ -908,7 +909,7 @@ fn extract_gdb_version(full_version_line: &str) -> Option { } /// Returns (LLDB version, LLDB is rust-enabled) -fn extract_lldb_version(full_version_line: Option) -> (Option, bool) { +fn extract_lldb_version(full_version_line: &str) -> Option<(u32, bool)> { // Extract the major LLDB version from the given version string. // LLDB version strings are different for Apple and non-Apple platforms. // The Apple variant looks like this: @@ -917,7 +918,7 @@ fn extract_lldb_version(full_version_line: Option) -> (Option, b // lldb-300.2.51 (new versions) // // We are only interested in the major version number, so this function - // will return `Some("179")` and `Some("300")` respectively. + // will return `Some(179)` and `Some(300)` respectively. // // Upstream versions look like: // lldb version 6.0.1 @@ -929,53 +930,20 @@ fn extract_lldb_version(full_version_line: Option) -> (Option, b // normally fine because the only non-Apple version we test is // rust-enabled. - if let Some(ref full_version_line) = full_version_line { - if !full_version_line.trim().is_empty() { - let full_version_line = full_version_line.trim(); - - for (pos, l) in full_version_line.char_indices() { - if l != 'l' && l != 'L' { - continue; - } - if pos + 5 >= full_version_line.len() { - continue; - } - let l = full_version_line[pos + 1..].chars().next().unwrap(); - if l != 'l' && l != 'L' { - continue; - } - let d = full_version_line[pos + 2..].chars().next().unwrap(); - if d != 'd' && d != 'D' { - continue; - } - let b = full_version_line[pos + 3..].chars().next().unwrap(); - if b != 'b' && b != 'B' { - continue; - } - let dash = full_version_line[pos + 4..].chars().next().unwrap(); - if dash != '-' { - continue; - } - - let vers = full_version_line[pos + 5..] - .chars() - .take_while(|c| c.is_digit(10)) - .collect::(); - if !vers.is_empty() { - return (Some(vers), full_version_line.contains("rust-enabled")); - } - } + let full_version_line = full_version_line.trim(); - if full_version_line.starts_with("lldb version ") { - let vers = full_version_line[13..] - .chars() - .take_while(|c| c.is_digit(10)) - .collect::(); - if !vers.is_empty() { - return (Some(vers + "00"), full_version_line.contains("rust-enabled")); - } - } + if let Some(apple_ver) = + full_version_line.strip_prefix("LLDB-").or_else(|| full_version_line.strip_prefix("lldb-")) + { + if let Some(idx) = apple_ver.find(|c: char| !c.is_digit(10)) { + let version: u32 = apple_ver[..idx].parse().unwrap(); + return Some((version, full_version_line.contains("rust-enabled"))); + } + } else if let Some(lldb_ver) = full_version_line.strip_prefix("lldb version ") { + if let Some(idx) = lldb_ver.find(|c: char| !c.is_digit(10)) { + let version: u32 = lldb_ver[..idx].parse().unwrap(); + return Some((version * 100, full_version_line.contains("rust-enabled"))); } } - (None, false) + None } diff --git a/src/tools/compiletest/src/tests.rs b/src/tools/compiletest/src/tests.rs index 31c151d29e916..7669ec53bf72f 100644 --- a/src/tools/compiletest/src/tests.rs +++ b/src/tools/compiletest/src/tests.rs @@ -41,6 +41,17 @@ fn test_extract_gdb_version() { } } +#[test] +fn test_extract_lldb_version() { + // Apple variants + assert_eq!(extract_lldb_version("LLDB-179.5"), Some((179, false))); + assert_eq!(extract_lldb_version("lldb-300.2.51"), Some((300, false))); + + // Upstream versions + assert_eq!(extract_lldb_version("lldb version 6.0.1"), Some((600, false))); + assert_eq!(extract_lldb_version("lldb version 9.0.0"), Some((900, false))); +} + #[test] fn is_test_test() { assert_eq!(true, is_test(&OsString::from("a_test.rs"))); From d778f326c385b2df7053b84fb5e3f89361b5fc3a Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 11 Jul 2020 08:45:26 +0000 Subject: [PATCH 06/23] compiletest: Rewrite extract_gdb_version function --- src/tools/compiletest/src/main.rs | 89 +++++++++--------------------- src/tools/compiletest/src/tests.rs | 2 +- 2 files changed, 28 insertions(+), 63 deletions(-) diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 2b0ff0da9f5c5..80f611d2ad6a4 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -841,71 +841,36 @@ fn extract_gdb_version(full_version_line: &str) -> Option { // This particular form is documented in the GNU coding standards: // https://www.gnu.org/prep/standards/html_node/_002d_002dversion.html#g_t_002d_002dversion - // don't start parsing in the middle of a number - let mut prev_was_digit = false; - let mut in_parens = false; - for (pos, c) in full_version_line.char_indices() { - if in_parens { - if c == ')' { - in_parens = false; - } - continue; - } else if c == '(' { - in_parens = true; - continue; - } - - if prev_was_digit || !c.is_digit(10) { - prev_was_digit = c.is_digit(10); - continue; + let mut splits = full_version_line.rsplit(' '); + let version_string = splits.next().unwrap(); + + let mut splits = version_string.split('.'); + let major = splits.next().unwrap(); + let minor = splits.next().unwrap(); + let patch = splits.next(); + + let major: u32 = major.parse().unwrap(); + let (minor, patch): (u32, u32) = match minor.find(|c: char| !c.is_digit(10)) { + None => { + let minor = minor.parse().unwrap(); + let patch: u32 = match patch { + Some(patch) => match patch.find(|c: char| !c.is_digit(10)) { + None => patch.parse().unwrap(), + Some(idx) if idx > 3 => 0, + Some(idx) => patch[..idx].parse().unwrap(), + }, + None => 0, + }; + (minor, patch) } - - prev_was_digit = true; - - let line = &full_version_line[pos..]; - - let next_split = match line.find(|c: char| !c.is_digit(10)) { - Some(idx) => idx, - None => continue, // no minor version - }; - - if line.as_bytes()[next_split] != b'.' { - continue; // no minor version + // There is no patch version after minor-date (e.g. "4-2012"). + Some(idx) => { + let minor = minor[..idx].parse().unwrap(); + (minor, 0) } + }; - let major = &line[..next_split]; - let line = &line[next_split + 1..]; - - let (minor, patch) = match line.find(|c: char| !c.is_digit(10)) { - Some(idx) => { - if line.as_bytes()[idx] == b'.' { - let patch = &line[idx + 1..]; - - let patch_len = - patch.find(|c: char| !c.is_digit(10)).unwrap_or_else(|| patch.len()); - let patch = &patch[..patch_len]; - let patch = if patch_len > 3 || patch_len == 0 { None } else { Some(patch) }; - - (&line[..idx], patch) - } else { - (&line[..idx], None) - } - } - None => (line, None), - }; - - if minor.is_empty() { - continue; - } - - let major: u32 = major.parse().unwrap(); - let minor: u32 = minor.parse().unwrap(); - let patch: u32 = patch.unwrap_or("0").parse().unwrap(); - - return Some(((major * 1000) + minor) * 1000 + patch); - } - - None + Some(((major * 1000) + minor) * 1000 + patch) } /// Returns (LLDB version, LLDB is rust-enabled) diff --git a/src/tools/compiletest/src/tests.rs b/src/tools/compiletest/src/tests.rs index 7669ec53bf72f..237bb756597a3 100644 --- a/src/tools/compiletest/src/tests.rs +++ b/src/tools/compiletest/src/tests.rs @@ -2,7 +2,7 @@ use super::*; #[test] fn test_extract_gdb_version() { - macro_rules! test { ($($expectation:tt: $input:tt,)*) => {{$( + macro_rules! test { ($($expectation:literal: $input:literal,)*) => {{$( assert_eq!(extract_gdb_version($input), Some($expectation)); )*}}} From 07d56cba8ffd854f1c8b91bb1372130e5abe6169 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 11 Jul 2020 13:48:21 +0000 Subject: [PATCH 07/23] Fix panic as passing wrong format to `extract_gdb_version` --- src/tools/compiletest/src/header.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 0ee11608dc5b3..dc561352642b3 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -132,32 +132,29 @@ impl EarlyProps { fn ignore_gdb(config: &Config, line: &str) -> bool { if let Some(actual_version) = config.gdb_version { - if line.starts_with("min-gdb-version") { - let (start_ver, end_ver) = extract_gdb_version_range(line); + if let Some(rest) = line.strip_prefix("min-gdb-version:").map(str::trim) { + let (start_ver, end_ver) = extract_gdb_version_range(rest); if start_ver != end_ver { panic!("Expected single GDB version") } // Ignore if actual version is smaller the minimum required // version - actual_version < start_ver - } else if line.starts_with("ignore-gdb-version") { - let (min_version, max_version) = extract_gdb_version_range(line); + return actual_version < start_ver; + } else if let Some(rest) = line.strip_prefix("ignore-gdb-version:").map(str::trim) { + let (min_version, max_version) = extract_gdb_version_range(rest); if max_version < min_version { panic!("Malformed GDB version range: max < min") } - actual_version >= min_version && actual_version <= max_version - } else { - false + return actual_version >= min_version && actual_version <= max_version; } - } else { - false } + false } - // Takes a directive of the form "ignore-gdb-version [- ]", + // Takes a directive of the form " [- ]", // returns the numeric representation of and as // tuple: ( as u32, as u32) // If the part is omitted, the second component of the tuple From 79d5cbbf867a888000b8bbd47194bc57b343b72a Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 11 Jul 2020 08:45:26 +0000 Subject: [PATCH 08/23] Use Option::as_deref --- src/tools/compiletest/src/main.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 80f611d2ad6a4..3929ce28ab0a6 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -171,7 +171,7 @@ pub fn parse_config(args: Vec) -> Config { .and_then(extract_lldb_version) .map(|(v, b)| (Some(v), b)) .unwrap_or((None, false)); - let color = match matches.opt_str("color").as_ref().map(|x| &**x) { + let color = match matches.opt_str("color").as_deref() { Some("auto") | None => ColorConfig::AutoColor, Some("always") => ColorConfig::AlwaysColor, Some("never") => ColorConfig::NeverColor, @@ -255,7 +255,7 @@ pub fn log_config(config: &Config) { logv(c, format!("stage_id: {}", config.stage_id)); logv(c, format!("mode: {}", config.mode)); logv(c, format!("run_ignored: {}", config.run_ignored)); - logv(c, format!("filter: {}", opt_str(&config.filter.as_ref().map(|re| re.to_owned())))); + logv(c, format!("filter: {}", opt_str(&config.filter))); logv(c, format!("filter_exact: {}", config.filter_exact)); logv( c, @@ -723,9 +723,7 @@ fn make_test_closure( let config = config.clone(); let testpaths = testpaths.clone(); let revision = revision.cloned(); - test::DynTestFn(Box::new(move || { - runtest::run(config, &testpaths, revision.as_ref().map(|s| s.as_str())) - })) + test::DynTestFn(Box::new(move || runtest::run(config, &testpaths, revision.as_deref()))) } /// Returns `true` if the given target is an Android target for the From 75caee076d41eca5163a4094e44ce6d48f6b4f1b Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 11 Jul 2020 09:06:59 +0000 Subject: [PATCH 09/23] Extract closure to function --- src/tools/compiletest/src/main.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 3929ce28ab0a6..049bf44d6fd96 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -848,11 +848,11 @@ fn extract_gdb_version(full_version_line: &str) -> Option { let patch = splits.next(); let major: u32 = major.parse().unwrap(); - let (minor, patch): (u32, u32) = match minor.find(|c: char| !c.is_digit(10)) { + let (minor, patch): (u32, u32) = match minor.find(not_a_digit) { None => { let minor = minor.parse().unwrap(); let patch: u32 = match patch { - Some(patch) => match patch.find(|c: char| !c.is_digit(10)) { + Some(patch) => match patch.find(not_a_digit) { None => patch.parse().unwrap(), Some(idx) if idx > 3 => 0, Some(idx) => patch[..idx].parse().unwrap(), @@ -898,15 +898,19 @@ fn extract_lldb_version(full_version_line: &str) -> Option<(u32, bool)> { if let Some(apple_ver) = full_version_line.strip_prefix("LLDB-").or_else(|| full_version_line.strip_prefix("lldb-")) { - if let Some(idx) = apple_ver.find(|c: char| !c.is_digit(10)) { + if let Some(idx) = apple_ver.find(not_a_digit) { let version: u32 = apple_ver[..idx].parse().unwrap(); return Some((version, full_version_line.contains("rust-enabled"))); } } else if let Some(lldb_ver) = full_version_line.strip_prefix("lldb version ") { - if let Some(idx) = lldb_ver.find(|c: char| !c.is_digit(10)) { + if let Some(idx) = lldb_ver.find(not_a_digit) { let version: u32 = lldb_ver[..idx].parse().unwrap(); return Some((version * 100, full_version_line.contains("rust-enabled"))); } } None } + +fn not_a_digit(c: char) -> bool { + !c.is_digit(10) +} From 5aa33b11fc88d0f81c7b2dc586db1439fcf2b664 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 11 Jul 2020 13:50:46 +0000 Subject: [PATCH 10/23] Use subslice pattern --- src/tools/compiletest/src/header.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index dc561352642b3..e155a9249902f 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -170,14 +170,14 @@ impl EarlyProps { .take(3) // 3 or more = invalid, so take at most 3. .collect::>>(); - match range_components.len() { - 1 => { - let v = range_components[0].unwrap(); + match *range_components { + [v] => { + let v = v.unwrap(); (v, v) } - 2 => { - let v_min = range_components[0].unwrap(); - let v_max = range_components[1].expect(ERROR_MESSAGE); + [min, max] => { + let v_min = min.unwrap(); + let v_max = max.expect(ERROR_MESSAGE); (v_min, v_max) } _ => panic!(ERROR_MESSAGE), From 5e5d8163233df1421925c7351377ade2306742de Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 18 Jul 2020 03:17:30 +0000 Subject: [PATCH 11/23] Add missing : after min-gdb-version --- src/test/debuginfo/function-call.rs | 2 +- src/test/debuginfo/pretty-huge-vec.rs | 2 +- src/test/debuginfo/pretty-std-collections.rs | 2 +- src/test/debuginfo/pretty-std.rs | 2 +- src/test/debuginfo/pretty-uninitialized-vec.rs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/debuginfo/function-call.rs b/src/test/debuginfo/function-call.rs index 98ad8983a60a0..a5d5942b53953 100644 --- a/src/test/debuginfo/function-call.rs +++ b/src/test/debuginfo/function-call.rs @@ -1,5 +1,5 @@ // This test does not passed with gdb < 8.0. See #53497. -// min-gdb-version 8.0 +// min-gdb-version: 8.0 // compile-flags:-g diff --git a/src/test/debuginfo/pretty-huge-vec.rs b/src/test/debuginfo/pretty-huge-vec.rs index 2616c9465246e..cbd2278f7e27c 100644 --- a/src/test/debuginfo/pretty-huge-vec.rs +++ b/src/test/debuginfo/pretty-huge-vec.rs @@ -2,7 +2,7 @@ // ignore-freebsd: gdb package too new // ignore-android: FIXME(#10381) // compile-flags:-g -// min-gdb-version 8.1 +// min-gdb-version: 8.1 // min-lldb-version: 310 // === GDB TESTS =================================================================================== diff --git a/src/test/debuginfo/pretty-std-collections.rs b/src/test/debuginfo/pretty-std-collections.rs index 4e95a028e0749..a4fbff5725c97 100644 --- a/src/test/debuginfo/pretty-std-collections.rs +++ b/src/test/debuginfo/pretty-std-collections.rs @@ -6,7 +6,7 @@ // The pretty printers being tested here require the patch from // https://sourceware.org/bugzilla/show_bug.cgi?id=21763 -// min-gdb-version 8.1 +// min-gdb-version: 8.1 // min-lldb-version: 310 diff --git a/src/test/debuginfo/pretty-std.rs b/src/test/debuginfo/pretty-std.rs index 57721ce103c39..7ae82d522b09d 100644 --- a/src/test/debuginfo/pretty-std.rs +++ b/src/test/debuginfo/pretty-std.rs @@ -2,7 +2,7 @@ // only-cdb // "Temporarily" ignored on GDB/LLDB due to debuginfo tests being disabled, see PR 47155 // ignore-android: FIXME(#10381) // compile-flags:-g -// min-gdb-version 7.7 +// min-gdb-version: 7.7 // min-lldb-version: 310 // === GDB TESTS =================================================================================== diff --git a/src/test/debuginfo/pretty-uninitialized-vec.rs b/src/test/debuginfo/pretty-uninitialized-vec.rs index 7ce004681e100..61791f48f4db7 100644 --- a/src/test/debuginfo/pretty-uninitialized-vec.rs +++ b/src/test/debuginfo/pretty-uninitialized-vec.rs @@ -2,7 +2,7 @@ // ignore-freebsd: gdb package too new // ignore-android: FIXME(#10381) // compile-flags:-g -// min-gdb-version 8.1 +// min-gdb-version: 8.1 // min-lldb-version: 310 // === GDB TESTS =================================================================================== From 2bcefa8d81fdf46f8ad9d893d271788477ccf95b Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sun, 19 Jul 2020 10:45:31 +0000 Subject: [PATCH 12/23] Add missing : after *llvm-version --- src/test/codegen/abi-efiapi.rs | 2 +- src/test/codegen/force-unwind-tables.rs | 2 +- src/test/ui/sanitize/new-llvm-pass-manager-thin-lto.rs | 2 +- src/tools/compiletest/src/header/tests.rs | 8 ++++---- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/test/codegen/abi-efiapi.rs b/src/test/codegen/abi-efiapi.rs index 8aeee5859d0ad..7c61b7809901f 100644 --- a/src/test/codegen/abi-efiapi.rs +++ b/src/test/codegen/abi-efiapi.rs @@ -2,7 +2,7 @@ // revisions:x86_64 i686 arm -// min-llvm-version 9.0 +// min-llvm-version: 9.0 //[x86_64] compile-flags: --target x86_64-unknown-uefi //[i686] compile-flags: --target i686-unknown-linux-musl diff --git a/src/test/codegen/force-unwind-tables.rs b/src/test/codegen/force-unwind-tables.rs index fbaf38d69df7f..eba4a7469f930 100644 --- a/src/test/codegen/force-unwind-tables.rs +++ b/src/test/codegen/force-unwind-tables.rs @@ -1,4 +1,4 @@ -// min-llvm-version 8.0 +// min-llvm-version: 8.0 // compile-flags: -C no-prepopulate-passes -C force-unwind-tables=y #![crate_type="lib"] diff --git a/src/test/ui/sanitize/new-llvm-pass-manager-thin-lto.rs b/src/test/ui/sanitize/new-llvm-pass-manager-thin-lto.rs index 64d6ccf340916..9439df266d59b 100644 --- a/src/test/ui/sanitize/new-llvm-pass-manager-thin-lto.rs +++ b/src/test/ui/sanitize/new-llvm-pass-manager-thin-lto.rs @@ -2,7 +2,7 @@ // being run when compiling with new LLVM pass manager and ThinLTO. // Note: The issue occurred only on non-zero opt-level. // -// min-llvm-version 9.0 +// min-llvm-version: 9.0 // needs-sanitizer-support // needs-sanitizer-address // diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index 72af34d78260b..58f73a4d7e060 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -120,16 +120,16 @@ fn llvm_version() { let mut config = config(); config.llvm_version = Some("8.1.2-rust".to_owned()); - assert!(parse_rs(&config, "// min-llvm-version 9.0").ignore); + assert!(parse_rs(&config, "// min-llvm-version: 9.0").ignore); config.llvm_version = Some("9.0.1-rust-1.43.0-dev".to_owned()); - assert!(parse_rs(&config, "// min-llvm-version 9.2").ignore); + assert!(parse_rs(&config, "// min-llvm-version: 9.2").ignore); config.llvm_version = Some("9.3.1-rust-1.43.0-dev".to_owned()); - assert!(!parse_rs(&config, "// min-llvm-version 9.2").ignore); + assert!(!parse_rs(&config, "// min-llvm-version: 9.2").ignore); config.llvm_version = Some("10.0.0-rust".to_owned()); - assert!(!parse_rs(&config, "// min-llvm-version 9.0").ignore); + assert!(!parse_rs(&config, "// min-llvm-version: 9.0").ignore); } #[test] From 99e3a3cdeaf9d6b42d329254b5d2752833c0c562 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sun, 19 Jul 2020 10:58:06 +0000 Subject: [PATCH 13/23] Extract extract_version_range --- src/tools/compiletest/src/header.rs | 66 +++++++++++++++-------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index e155a9249902f..3d9c875499136 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -133,7 +133,7 @@ impl EarlyProps { fn ignore_gdb(config: &Config, line: &str) -> bool { if let Some(actual_version) = config.gdb_version { if let Some(rest) = line.strip_prefix("min-gdb-version:").map(str::trim) { - let (start_ver, end_ver) = extract_gdb_version_range(rest); + let (start_ver, end_ver) = extract_version_range(rest, extract_gdb_version); if start_ver != end_ver { panic!("Expected single GDB version") @@ -142,7 +142,8 @@ impl EarlyProps { // version return actual_version < start_ver; } else if let Some(rest) = line.strip_prefix("ignore-gdb-version:").map(str::trim) { - let (min_version, max_version) = extract_gdb_version_range(rest); + let (min_version, max_version) = + extract_version_range(rest, extract_gdb_version); if max_version < min_version { panic!("Malformed GDB version range: max < min") @@ -154,36 +155,6 @@ impl EarlyProps { false } - // Takes a directive of the form " [- ]", - // returns the numeric representation of and as - // tuple: ( as u32, as u32) - // If the part is omitted, the second component of the tuple - // is the same as . - fn extract_gdb_version_range(line: &str) -> (u32, u32) { - const ERROR_MESSAGE: &'static str = "Malformed GDB version directive"; - - let range_components = line - .split(&[' ', '-'][..]) - .filter(|word| !word.is_empty()) - .map(extract_gdb_version) - .skip_while(Option::is_none) - .take(3) // 3 or more = invalid, so take at most 3. - .collect::>>(); - - match *range_components { - [v] => { - let v = v.unwrap(); - (v, v) - } - [min, max] => { - let v_min = min.unwrap(); - let v_max = max.expect(ERROR_MESSAGE); - (v_min, v_max) - } - _ => panic!(ERROR_MESSAGE), - } - } - fn ignore_lldb(config: &Config, line: &str) -> bool { if let Some(actual_version) = config.lldb_version { if let Some(min_version) = line.strip_prefix("min-lldb-version:").map(str::trim) { @@ -982,3 +953,34 @@ fn parse_normalization_string(line: &mut &str) -> Option { *line = &line[end + 1..]; Some(result) } + +// Takes a directive of the form " [- ]", +// returns the numeric representation of and as +// tuple: ( as u32, as u32) +// If the part is omitted, the second component of the tuple +// is the same as . +fn extract_version_range(line: &str, parse: F) -> (u32, u32) +where + F: Fn(&str) -> Option, +{ + let range_components = line + .split(&[' ', '-'][..]) + .filter(|word| !word.is_empty()) + .map(parse) + .skip_while(Option::is_none) + .take(3) // 3 or more = invalid, so take at most 3. + .collect::>>(); + + match *range_components { + [v] => { + let v = v.unwrap(); + (v, v) + } + [min, max] => { + let v_min = min.unwrap(); + let v_max = max.expect("Malformed version directive"); + (v_min, v_max) + } + _ => panic!("Malformed version directive"), + } +} From 60fac34c2048fe5e4fdb5c4516987b948d9f5192 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sun, 19 Jul 2020 11:10:38 +0000 Subject: [PATCH 14/23] Rewrite extract_llvm_version --- src/tools/compiletest/src/common.rs | 2 +- src/tools/compiletest/src/header.rs | 83 +++++++++-------------- src/tools/compiletest/src/header/tests.rs | 8 +-- src/tools/compiletest/src/main.rs | 4 +- src/tools/compiletest/src/tests.rs | 9 +++ 5 files changed, 49 insertions(+), 57 deletions(-) diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index 4abb1db35a02f..c1c07e0bc00a0 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -274,7 +274,7 @@ pub struct Config { pub lldb_native_rust: bool, /// Version of LLVM - pub llvm_version: Option, + pub llvm_version: Option, /// Is LLVM a system LLVM pub system_llvm: bool, diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 3d9c875499136..4b5c44bca875b 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -181,48 +181,28 @@ impl EarlyProps { if config.system_llvm && line.starts_with("no-system-llvm") { return true; } - if let Some(ref actual_version) = config.llvm_version { - let actual_version = version_to_int(actual_version); - if line.starts_with("min-llvm-version") { - let min_version = line - .trim_end() - .rsplit(' ') - .next() - .expect("Malformed llvm version directive"); + if let Some(actual_version) = config.llvm_version { + if let Some(rest) = line.strip_prefix("min-llvm-version:").map(str::trim) { + let min_version = extract_llvm_version(rest).unwrap(); // Ignore if actual version is smaller the minimum required // version - actual_version < version_to_int(min_version) - } else if line.starts_with("min-system-llvm-version") { - let min_version = line - .trim_end() - .rsplit(' ') - .next() - .expect("Malformed llvm version directive"); + actual_version < min_version + } else if let Some(rest) = + line.strip_prefix("min-system-llvm-version:").map(str::trim) + { + let min_version = extract_llvm_version(rest).unwrap(); // Ignore if using system LLVM and actual version // is smaller the minimum required version - config.system_llvm && actual_version < version_to_int(min_version) - } else if line.starts_with("ignore-llvm-version") { - // Syntax is: "ignore-llvm-version [- ]" - let range_components = line - .split(' ') - .skip(1) // Skip the directive. - .map(|s| s.trim()) - .filter(|word| !word.is_empty() && word != &"-") - .take(3) // 3 or more = invalid, so take at most 3. - .collect::>(); - match range_components.len() { - 1 => actual_version == version_to_int(range_components[0]), - 2 => { - let v_min = version_to_int(range_components[0]); - let v_max = version_to_int(range_components[1]); - if v_max < v_min { - panic!("Malformed LLVM version range: max < min") - } - // Ignore if version lies inside of range. - actual_version >= v_min && actual_version <= v_max - } - _ => panic!("Malformed LLVM version directive"), + config.system_llvm && actual_version < min_version + } else if let Some(rest) = line.strip_prefix("ignore-llvm-version:").map(str::trim) + { + // Syntax is: "ignore-llvm-version: [- ]" + let (v_min, v_max) = extract_version_range(rest, extract_llvm_version); + if v_max < v_min { + panic!("Malformed LLVM version range: max < min") } + // Ignore if version lies inside of range. + actual_version >= v_min && actual_version <= v_max } else { false } @@ -230,20 +210,6 @@ impl EarlyProps { false } } - - fn version_to_int(version: &str) -> u32 { - let version_without_suffix = version.trim_end_matches("git").split('-').next().unwrap(); - let components: Vec = version_without_suffix - .split('.') - .map(|s| s.parse().expect("Malformed version component")) - .collect(); - match components.len() { - 1 => components[0] * 10000, - 2 => components[0] * 10000 + components[1] * 100, - 3 => components[0] * 10000 + components[1] * 100 + components[2], - _ => panic!("Malformed version"), - } - } } } @@ -954,6 +920,21 @@ fn parse_normalization_string(line: &mut &str) -> Option { Some(result) } +pub fn extract_llvm_version(version: &str) -> Option { + let version_without_suffix = version.trim_end_matches("git").split('-').next().unwrap(); + let components: Vec = version_without_suffix + .split('.') + .map(|s| s.parse().expect("Malformed version component")) + .collect(); + let version = match *components { + [a] => a * 10_000, + [a, b] => a * 10_000 + b * 100, + [a, b, c] => a * 10_000 + b * 100 + c, + _ => panic!("Malformed version"), + }; + Some(version) +} + // Takes a directive of the form " [- ]", // returns the numeric representation of and as // tuple: ( as u32, as u32) diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index 58f73a4d7e060..adcd8691d0f58 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -119,16 +119,16 @@ fn no_system_llvm() { fn llvm_version() { let mut config = config(); - config.llvm_version = Some("8.1.2-rust".to_owned()); + config.llvm_version = Some(80102); assert!(parse_rs(&config, "// min-llvm-version: 9.0").ignore); - config.llvm_version = Some("9.0.1-rust-1.43.0-dev".to_owned()); + config.llvm_version = Some(90001); assert!(parse_rs(&config, "// min-llvm-version: 9.2").ignore); - config.llvm_version = Some("9.3.1-rust-1.43.0-dev".to_owned()); + config.llvm_version = Some(90301); assert!(!parse_rs(&config, "// min-llvm-version: 9.2").ignore); - config.llvm_version = Some("10.0.0-rust".to_owned()); + config.llvm_version = Some(100000); assert!(!parse_rs(&config, "// min-llvm-version: 9.0").ignore); } diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 049bf44d6fd96..0b46aace49944 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -177,6 +177,8 @@ pub fn parse_config(args: Vec) -> Config { Some("never") => ColorConfig::NeverColor, Some(x) => panic!("argument for --color must be auto, always, or never, but found `{}`", x), }; + let llvm_version = + matches.opt_str("llvm-version").as_deref().and_then(header::extract_llvm_version); let src_base = opt_path(matches, "src-base"); let run_ignored = matches.opt_present("ignored"); @@ -217,7 +219,7 @@ pub fn parse_config(args: Vec) -> Config { gdb_native_rust, lldb_version, lldb_native_rust, - llvm_version: matches.opt_str("llvm-version"), + llvm_version, system_llvm: matches.opt_present("system-llvm"), android_cross_path, adb_path: opt_str2(matches.opt_str("adb-path")), diff --git a/src/tools/compiletest/src/tests.rs b/src/tools/compiletest/src/tests.rs index 237bb756597a3..ea9bc1c1a5b7f 100644 --- a/src/tools/compiletest/src/tests.rs +++ b/src/tools/compiletest/src/tests.rs @@ -1,3 +1,4 @@ +use super::header::extract_llvm_version; use super::*; #[test] @@ -60,3 +61,11 @@ fn is_test_test() { assert_eq!(false, is_test(&OsString::from("#a_dog_gif"))); assert_eq!(false, is_test(&OsString::from("~a_temp_file"))); } + +#[test] +fn test_extract_llvm_version() { + assert_eq!(extract_llvm_version("8.1.2-rust"), Some(80102)); + assert_eq!(extract_llvm_version("9.0.1-rust-1.43.0-dev"), Some(90001)); + assert_eq!(extract_llvm_version("9.3.1-rust-1.43.0-dev"), Some(90301)); + assert_eq!(extract_llvm_version("10.0.0-rust"), Some(100000)); +} From 1314d31fb5974e8c32aa8d695a02d37c4ab4bd7e Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sun, 19 Jul 2020 15:08:35 +0000 Subject: [PATCH 15/23] Rewrite extract_version_range --- src/tools/compiletest/src/header.rs | 54 +++++++++++++---------- src/tools/compiletest/src/header/tests.rs | 15 +++++++ 2 files changed, 46 insertions(+), 23 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 4b5c44bca875b..2ab764eb9207c 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -133,7 +133,10 @@ impl EarlyProps { fn ignore_gdb(config: &Config, line: &str) -> bool { if let Some(actual_version) = config.gdb_version { if let Some(rest) = line.strip_prefix("min-gdb-version:").map(str::trim) { - let (start_ver, end_ver) = extract_version_range(rest, extract_gdb_version); + let (start_ver, end_ver) = extract_version_range(rest, extract_gdb_version) + .unwrap_or_else(|| { + panic!("couldn't parse version range: {:?}", rest); + }); if start_ver != end_ver { panic!("Expected single GDB version") @@ -143,7 +146,9 @@ impl EarlyProps { return actual_version < start_ver; } else if let Some(rest) = line.strip_prefix("ignore-gdb-version:").map(str::trim) { let (min_version, max_version) = - extract_version_range(rest, extract_gdb_version); + extract_version_range(rest, extract_gdb_version).unwrap_or_else(|| { + panic!("couldn't parse version range: {:?}", rest); + }); if max_version < min_version { panic!("Malformed GDB version range: max < min") @@ -197,7 +202,10 @@ impl EarlyProps { } else if let Some(rest) = line.strip_prefix("ignore-llvm-version:").map(str::trim) { // Syntax is: "ignore-llvm-version: [- ]" - let (v_min, v_max) = extract_version_range(rest, extract_llvm_version); + let (v_min, v_max) = extract_version_range(rest, extract_llvm_version) + .unwrap_or_else(|| { + panic!("couldn't parse version range: {:?}", rest); + }); if v_max < v_min { panic!("Malformed LLVM version range: max < min") } @@ -940,28 +948,28 @@ pub fn extract_llvm_version(version: &str) -> Option { // tuple: ( as u32, as u32) // If the part is omitted, the second component of the tuple // is the same as . -fn extract_version_range(line: &str, parse: F) -> (u32, u32) +fn extract_version_range(line: &str, parse: F) -> Option<(u32, u32)> where F: Fn(&str) -> Option, { - let range_components = line - .split(&[' ', '-'][..]) - .filter(|word| !word.is_empty()) - .map(parse) - .skip_while(Option::is_none) - .take(3) // 3 or more = invalid, so take at most 3. - .collect::>>(); - - match *range_components { - [v] => { - let v = v.unwrap(); - (v, v) - } - [min, max] => { - let v_min = min.unwrap(); - let v_max = max.expect("Malformed version directive"); - (v_min, v_max) - } - _ => panic!("Malformed version directive"), + let mut splits = line.splitn(2, "- ").map(str::trim); + let min = splits.next().unwrap(); + if min.ends_with('-') { + return None; + } + + let max = splits.next(); + + if min.is_empty() { + return None; } + + let min = parse(min)?; + let max = match max { + Some(max) if max.is_empty() => return None, + Some(max) => parse(max)?, + _ => min, + }; + + Some((min, max)) } diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index adcd8691d0f58..1f82b137ee6cf 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -220,3 +220,18 @@ fn sanitizers() { assert!(parse_rs(&config, "// needs-sanitizer-memory").ignore); assert!(parse_rs(&config, "// needs-sanitizer-thread").ignore); } + +#[test] +fn test_extract_version_range() { + use super::{extract_llvm_version, extract_version_range}; + + assert_eq!(extract_version_range("1.2.3 - 4.5.6", extract_llvm_version), Some((10203, 40506))); + assert_eq!(extract_version_range("0 - 4.5.6", extract_llvm_version), Some((0, 40506))); + assert_eq!(extract_version_range("1.2.3 -", extract_llvm_version), None); + assert_eq!(extract_version_range("1.2.3 - ", extract_llvm_version), None); + assert_eq!(extract_version_range("- 4.5.6", extract_llvm_version), None); + assert_eq!(extract_version_range("-", extract_llvm_version), None); + assert_eq!(extract_version_range(" - 4.5.6", extract_llvm_version), None); + assert_eq!(extract_version_range(" - 4.5.6", extract_llvm_version), None); + assert_eq!(extract_version_range("0 -", extract_llvm_version), None); +} From 40df8fd0fad99db880bee40fce7c855922e7f0f9 Mon Sep 17 00:00:00 2001 From: aticu <15schnic@gmail.com> Date: Mon, 20 Jul 2020 20:39:17 +0200 Subject: [PATCH 16/23] Apply #66379 to `*mut T` `as_ref` --- src/libcore/ptr/mut_ptr.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/libcore/ptr/mut_ptr.rs b/src/libcore/ptr/mut_ptr.rs index 96856e7512cab..915dc3e45f644 100644 --- a/src/libcore/ptr/mut_ptr.rs +++ b/src/libcore/ptr/mut_ptr.rs @@ -47,17 +47,22 @@ impl *mut T { /// operation because the returned value could be pointing to invalid /// memory. /// - /// When calling this method, you have to ensure that if the pointer is - /// non-NULL, then it is properly aligned, dereferenceable (for the whole - /// size of `T`) and points to an initialized instance of `T`. This applies - /// even if the result of this method is unused! + /// When calling this method, you have to ensure that *either* the pointer is NULL *or* + /// all of the following is true: + /// - it is properly aligned + /// - it must point to an initialized instance of T; in particular, the pointer must be + /// "dereferencable" in the sense defined [here]. + /// + /// This applies even if the result of this method is unused! /// (The part about being initialized is not yet fully decided, but until /// it is, the only safe approach is to ensure that they are indeed initialized.) /// /// Additionally, the lifetime `'a` returned is arbitrarily chosen and does - /// not necessarily reflect the actual lifetime of the data. It is up to the - /// caller to ensure that for the duration of this lifetime, the memory this - /// pointer points to does not get written to outside of `UnsafeCell`. + /// not necessarily reflect the actual lifetime of the data. *You* must enforce + /// Rust's aliasing rules. In particular, for the duration of this lifetime, + /// the memory the pointer points to must not get mutated (except inside `UnsafeCell`). + /// + /// [here]: crate::ptr#safety /// /// # Examples /// From cf52d5fb87dc53b69f04416cc4080de807b7fd76 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 20 Jul 2020 16:29:58 -0300 Subject: [PATCH 17/23] Use forge links for prioritization procedure --- triagebot.toml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/triagebot.toml b/triagebot.toml index 51a29553fdb3d..ce4ea895400ac 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -81,7 +81,7 @@ topic = "I-prioritize #{number} {title}" message_on_add = """\ @*WG-prioritization/alerts* issue #{number} has been requested for prioritization. -# [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#Unprioritized-I-prioritize) +# [Procedure](https://forge.rust-lang.org/compiler/prioritization/procedure.html#assign-priority-to-unprioritized-issues-with-i-prioritize-label) - Priority? - Regression? - Notify people/groups? @@ -96,7 +96,7 @@ topic = "I-nominated #{number} {title}" message_on_add = """\ @*WG-prioritization/alerts* #{number} has been nominated for discussion in `T-compiler` meeting. -# [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#I-nominated) +# [Procedure](https://forge.rust-lang.org/compiler/prioritization/procedure.html#summarize-i-nominated-issues) - Already discussed? - Worth the meeting time? - Add agenda entry: @@ -113,7 +113,7 @@ topic = "Backport #{number} {title}" message_on_add = """\ @*WG-prioritization/alerts* PR #{number} has been requested for beta backport. -# [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#StableBeta-nominations) +# [Procedure](https://forge.rust-lang.org/compiler/prioritization/procedure.html#summarize-stablebeta-nominations) Prepare agenda entry: - Why nominated? - Author, assignee? @@ -127,7 +127,7 @@ topic = "Backport #{number} {title}" message_on_add = """\ @*WG-prioritization/alerts* PR #{number} has been requested for stable backport. -# [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#StableBeta-nominations) +# [Procedure](https://forge.rust-lang.org/compiler/prioritization/procedure.html#summarize-stablebeta-nominations) Prepare agenda entry: - Why nominated? - Author, assignee? @@ -142,7 +142,7 @@ topic = "S-waiting-on-team #{number} {title}" message_on_add = """\ @*WG-prioritization/alerts* PR #{number} is waiting on `T-compiler`. -# [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#PR%E2%80%99s-waiting-on-team) +# [Procedure](https://forge.rust-lang.org/compiler/prioritization/procedure.html#summarize-prs-waiting-on-team) - Prepare agenda entry: - What is it waiting for? - Important details? @@ -156,7 +156,7 @@ topic = "P-critical #{number} {title}" message_on_add = """\ @*WG-prioritization/alerts* issue #{number} has been assigned `P-critical`. -# [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#P-critical-and-Unassigned-P-high-regressions) +# [Procedure](https://forge.rust-lang.org/compiler/prioritization/procedure.html#summarize-p-critical-and-unassigned-p-high-regressions) - Notify people/groups? - Assign if possible? - Add to agenda: @@ -172,7 +172,7 @@ topic = "P-high regression #{number} {title}" message_on_add = """\ @*WG-prioritization/alerts* issue #{number} has been assigned `P-high` and is a regression. -# [Procedure](https://hackmd.io/WJ0G17DHTHGgv0OW9I2PxA?view#P-critical-and-Unassigned-P-high-regressions) +# [Procedure](https://forge.rust-lang.org/compiler/prioritization/procedure.html#summarize-p-critical-and-unassigned-p-high-regressions) Is issue assigned? If not: - Try to find an assignee? - Otherwise add to agenda: From c71b196f66d110291a67c3bbc0f8fdb11a261f1f Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Mon, 20 Jul 2020 22:34:26 +0200 Subject: [PATCH 18/23] review --- src/librustc_trait_selection/traits/coherence.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/librustc_trait_selection/traits/coherence.rs b/src/librustc_trait_selection/traits/coherence.rs index b3218f97e42b5..047552b6aeb6d 100644 --- a/src/librustc_trait_selection/traits/coherence.rs +++ b/src/librustc_trait_selection/traits/coherence.rs @@ -438,6 +438,15 @@ fn orphan_check_trait_ref<'tcx>( /// /// This is just `ty` itself unless `ty` is `#[fundamental]`, /// in which case we recursively look into this type. +/// +/// If `ty` is local itself, this method returns an empty `Vec`. +/// +/// # Examples +/// +/// - `u32` is not local, so this returns `[u32]`. +/// - for `Foo`, where `Foo` is a local type, this returns `[]`. +/// - `&mut u32` returns `[u32]`, as `&mut` is a fundamental type, similar to `Box`. +/// - `Box>` returns `[]`, as `Box` is a fundamental type and `Foo` is local. fn contained_non_local_types(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, in_crate: InCrate) -> Vec> { if ty_is_local_constructor(ty, in_crate) { Vec::new() @@ -493,7 +502,7 @@ fn def_id_is_local(def_id: DefId, in_crate: InCrate) -> bool { } fn ty_is_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> bool { - debug!("ty_is_non_local_constructor({:?})", ty); + debug!("ty_is_local_constructor({:?})", ty); match ty.kind { ty::Bool From cfcbca6c697895e86a70127b317c24c1750c8f89 Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Mon, 20 Jul 2020 23:18:06 +0200 Subject: [PATCH 19/23] update coherence docs --- src/librustc_trait_selection/traits/coherence.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/librustc_trait_selection/traits/coherence.rs b/src/librustc_trait_selection/traits/coherence.rs index 047552b6aeb6d..b06cf4411d053 100644 --- a/src/librustc_trait_selection/traits/coherence.rs +++ b/src/librustc_trait_selection/traits/coherence.rs @@ -289,11 +289,11 @@ pub fn orphan_check(tcx: TyCtxt<'_>, impl_def_id: DefId) -> Result<(), OrphanChe /// - but (knowing that `Vec` is non-fundamental, and assuming it's /// not local), `Vec` is bad, because `Vec<->` is between /// the local type and the type parameter. -/// 3. Every type parameter before the local key parameter is fully known in C. -/// - e.g., `impl T: Trait` is bad, because `T` might be -/// an unknown type. -/// - but `impl LocalType: Trait` is OK, because `LocalType` -/// occurs before `T`. +/// 3. Before this local type, no generic type parameter of the impl must +/// be reachable through fundamental types. +/// - e.g. `impl Trait for Vec` is fine, as `Vec` is not fundamental. +/// - while `impl Trait` results in an error, as `T` is +/// reachable through the fundamental type `Box`. /// 4. Every type in the local key parameter not known in C, going /// through the parameter's type tree, must appear only as a subtree of /// a type local to C, with only fundamental types between the type @@ -387,8 +387,8 @@ fn orphan_check_trait_ref<'tcx>( ty: Ty<'tcx>, in_crate: InCrate, ) -> Vec> { - // FIXME(eddyb) figure out if this is redundant with `ty_is_non_local`, - // or maybe if this should be calling `ty_is_non_local_constructor`. + // FIXME: this is currently somewhat overly complicated, + // but fixing this requires a more complicated refactor. if !contained_non_local_types(tcx, ty, in_crate).is_empty() { if let Some(inner_tys) = fundamental_ty_inner_tys(tcx, ty) { return inner_tys From 7f3e2c0d374ba7982ef09499c7de6bb53866abf4 Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Sun, 19 Jul 2020 13:03:35 -0400 Subject: [PATCH 20/23] refactor and reword intra-doc link errors This commit refactors intra-doc link error reporting to deduplicate code and decouple error construction from the type of error. This greatly improves flexibility at each error construction site, while reducing the complexity of the diagnostic creation. This commit also rewords the diagnostics for clarity and style: - Diagnostics should not end in periods. - It's unnecessary to say "ignoring it". Since this is a warning by default, it's already clear that the link is ignored. --- .../passes/collect_intra_doc_links.rs | 354 +++++++++--------- .../deny-intra-link-resolution-failure.stderr | 6 +- .../rustdoc-ui/intra-doc-alias-ice.stderr | 6 +- .../rustdoc-ui/intra-link-span-ice-55723.rs | 2 +- .../intra-link-span-ice-55723.stderr | 6 +- src/test/rustdoc-ui/intra-links-anchors.rs | 8 +- .../rustdoc-ui/intra-links-anchors.stderr | 16 +- .../intra-links-private.public.stderr | 2 +- src/test/rustdoc-ui/intra-links-private.rs | 2 +- .../rustdoc-ui/intra-links-warning-crlf.rs | 8 +- .../intra-links-warning-crlf.stderr | 24 +- src/test/rustdoc-ui/intra-links-warning.rs | 38 +- .../rustdoc-ui/intra-links-warning.stderr | 104 ++--- src/test/rustdoc-ui/issue-74134.public.stderr | 2 +- src/test/rustdoc-ui/issue-74134.rs | 2 +- src/test/rustdoc-ui/lint-group.rs | 2 +- src/test/rustdoc-ui/lint-group.stderr | 6 +- 17 files changed, 290 insertions(+), 298 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index e187b9251f71e..6f221da1168c6 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1,5 +1,5 @@ use rustc_ast::ast; -use rustc_errors::Applicability; +use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_expand::base::SyntaxExtensionKind; use rustc_feature::UnstableFeatures; use rustc_hir as hir; @@ -45,7 +45,17 @@ pub fn collect_intra_doc_links(krate: Crate, cx: &DocContext<'_>) -> Crate { enum ErrorKind { ResolutionFailure, - AnchorFailure(&'static str), + AnchorFailure(AnchorFailure), +} + +enum AnchorFailure { + MultipleAnchors, + Primitive, + Variant, + AssocConstant, + AssocType, + Field, + Method, } struct LinkCollector<'a, 'tcx> { @@ -197,9 +207,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // Not a trait item; just return what we found. Res::PrimTy(..) => { if extra_fragment.is_some() { - return Err(ErrorKind::AnchorFailure( - "primitive types cannot be followed by anchors", - )); + return Err(ErrorKind::AnchorFailure(AnchorFailure::Primitive)); } return Ok((res, Some(path_str.to_owned()))); } @@ -209,9 +217,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { if disambiguator == Some("type") { if let Some(prim) = is_primitive(path_str, ns) { if extra_fragment.is_some() { - return Err(ErrorKind::AnchorFailure( - "primitive types cannot be followed by anchors", - )); + return Err(ErrorKind::AnchorFailure(AnchorFailure::Primitive)); } return Ok((prim, Some(path_str.to_owned()))); } @@ -228,9 +234,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } } else if let Some(prim) = is_primitive(path_str, ns) { if extra_fragment.is_some() { - return Err(ErrorKind::AnchorFailure( - "primitive types cannot be followed by anchors", - )); + return Err(ErrorKind::AnchorFailure(AnchorFailure::Primitive)); } return Ok((prim, Some(path_str.to_owned()))); } else { @@ -338,9 +342,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }; if extra_fragment.is_some() { Err(ErrorKind::AnchorFailure(if item.kind == ty::AssocKind::Fn { - "methods cannot be followed by anchors" + AnchorFailure::Method } else { - "associated constants cannot be followed by anchors" + AnchorFailure::AssocConstant })) } else { Ok((ty_res, Some(format!("{}.{}", out, item_name)))) @@ -358,9 +362,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } { if extra_fragment.is_some() { Err(ErrorKind::AnchorFailure(if def.is_enum() { - "enum variants cannot be followed by anchors" + AnchorFailure::Variant } else { - "struct fields cannot be followed by anchors" + AnchorFailure::Field })) } else { Ok(( @@ -404,11 +408,11 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { if extra_fragment.is_some() { Err(ErrorKind::AnchorFailure(if item.kind == ty::AssocKind::Const { - "associated constants cannot be followed by anchors" + AnchorFailure::AssocConstant } else if item.kind == ty::AssocKind::Type { - "associated types cannot be followed by anchors" + AnchorFailure::AssocType } else { - "methods cannot be followed by anchors" + AnchorFailure::Method })) } else { Ok((ty_res, Some(format!("{}.{}", kind, item_name)))) @@ -559,16 +563,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { let link = ori_link.replace("`", ""); let parts = link.split('#').collect::>(); let (link, extra_fragment) = if parts.len() > 2 { - build_diagnostic( - cx, - &item, - &link, - &dox, - link_range, - "has an issue with the link anchor.", - "only one `#` is allowed in a link", - None, - ); + anchor_failure(cx, &item, &link, &dox, link_range, AnchorFailure::MultipleAnchors); continue; } else if parts.len() == 2 { if parts[0].trim().is_empty() { @@ -803,21 +798,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { && (item.visibility == Visibility::Public) && !self.cx.render_options.document_private { - let item_name = item.name.as_deref().unwrap_or(""); - let err_msg = format!( - "public documentation for `{}` links to a private item", - item_name - ); - build_diagnostic( - cx, - &item, - path_str, - &dox, - link_range, - &err_msg, - "this item is private", - None, - ); + privacy_error(cx, &item, &path_str, &dox, link_range); continue; } } @@ -851,24 +832,33 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } } -fn build_diagnostic( +/// Reports a diagnostic for an intra-doc link. +/// +/// If no link range is provided, or the source span of the link cannot be determined, the span of +/// the entire documentation block is used for the lint. If a range is provided but the span +/// calculation fails, a note is added to the diagnostic pointing to the link in the markdown. +/// +/// The `decorate` callback is invoked in all cases to allow further customization of the +/// diagnostic before emission. If the span of the link was able to be determined, the second +/// parameter of the callback will contain it, and the primary span of the diagnostic will be set +/// to it. +fn report_diagnostic( cx: &DocContext<'_>, + msg: &str, item: &Item, - path_str: &str, dox: &str, link_range: Option>, - err_msg: &str, - short_err_msg: &str, - help_msg: Option<&str>, + decorate: impl FnOnce(&mut DiagnosticBuilder<'_>, Option), ) { let hir_id = match cx.as_local_hir_id(item.def_id) { Some(hir_id) => hir_id, None => { // If non-local, no need to check anything. - info!("ignoring warning from parent crate: {}", err_msg); + info!("ignoring warning from parent crate: {}", msg); return; } }; + let attrs = &item.attrs; let sp = span_of_attrs(attrs).unwrap_or(item.source.span()); @@ -877,12 +867,15 @@ fn build_diagnostic( hir_id, sp, |lint| { - let mut diag = lint.build(&format!("`[{}]` {}", path_str, err_msg)); + let mut diag = lint.build(msg); + + let span = link_range + .as_ref() + .and_then(|range| super::source_span_for_markdown_range(cx, dox, range, attrs)); + if let Some(link_range) = link_range { - if let Some(sp) = super::source_span_for_markdown_range(cx, dox, &link_range, attrs) - { + if let Some(sp) = span { diag.set_span(sp); - diag.span_label(sp, short_err_msg); } else { // blah blah blah\nblah\nblah [blah] blah blah\nblah blah // ^ ~~~~ @@ -902,20 +895,15 @@ fn build_diagnostic( found = link_range.len(), )); } - }; - if let Some(help_msg) = help_msg { - diag.help(help_msg); } + + decorate(&mut diag, span); + diag.emit(); }, ); } -/// Reports a resolution failure diagnostic. -/// -/// If we cannot find the exact source span of the resolution failure, we use the span of the -/// documentation attributes themselves. This is a little heavy-handed, so we display the markdown -/// line containing the failure as a note as well. fn resolution_failure( cx: &DocContext<'_>, item: &Item, @@ -923,15 +911,19 @@ fn resolution_failure( dox: &str, link_range: Option>, ) { - build_diagnostic( + report_diagnostic( cx, + &format!("unresolved link to `{}`", path_str), item, - path_str, dox, link_range, - "cannot be resolved, ignoring it.", - "cannot be resolved, ignoring", - Some("to escape `[` and `]` characters, just add '\\' before them like `\\[` or `\\]`"), + |diag, sp| { + if let Some(sp) = sp { + diag.span_label(sp, "unresolved link"); + } + + diag.help(r#"to escape `[` and `]` characters, add '\' before them like `\[` or `\]`"#); + }, ); } @@ -941,18 +933,39 @@ fn anchor_failure( path_str: &str, dox: &str, link_range: Option>, - msg: &str, + failure: AnchorFailure, ) { - build_diagnostic( - cx, - item, - path_str, - dox, - link_range, - "has an issue with the link anchor.", - msg, - None, - ); + let msg = match failure { + AnchorFailure::MultipleAnchors => format!("`{}` contains multiple anchors", path_str), + AnchorFailure::Primitive + | AnchorFailure::Variant + | AnchorFailure::AssocConstant + | AnchorFailure::AssocType + | AnchorFailure::Field + | AnchorFailure::Method => { + let kind = match failure { + AnchorFailure::Primitive => "primitive type", + AnchorFailure::Variant => "enum variant", + AnchorFailure::AssocConstant => "associated constant", + AnchorFailure::AssocType => "associated type", + AnchorFailure::Field => "struct field", + AnchorFailure::Method => "method", + AnchorFailure::MultipleAnchors => unreachable!("should be handled already"), + }; + + format!( + "`{}` contains an anchor, but links to {kind}s are already anchored", + path_str, + kind = kind + ) + } + }; + + report_diagnostic(cx, &msg, item, dox, link_range, |diag, sp| { + if let Some(sp) = sp { + diag.span_label(sp, "contains invalid anchor"); + } + }); } fn ambiguity_error( @@ -963,121 +976,100 @@ fn ambiguity_error( link_range: Option>, candidates: PerNS>, ) { - let hir_id = match cx.as_local_hir_id(item.def_id) { - Some(hir_id) => hir_id, - None => { - // If non-local, no need to check anything. - return; + let mut msg = format!("`{}` is ", path_str); + + let candidates = [TypeNS, ValueNS, MacroNS] + .iter() + .filter_map(|&ns| candidates[ns].map(|res| (res, ns))) + .collect::>(); + match candidates.as_slice() { + [(first_def, _), (second_def, _)] => { + msg += &format!( + "both {} {} and {} {}", + first_def.article(), + first_def.descr(), + second_def.article(), + second_def.descr(), + ); } - }; - let attrs = &item.attrs; - let sp = span_of_attrs(attrs).unwrap_or(item.source.span()); - - cx.tcx.struct_span_lint_hir( - lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE, - hir_id, - sp, - |lint| { - let mut msg = format!("`{}` is ", path_str); - - let candidates = [TypeNS, ValueNS, MacroNS] - .iter() - .filter_map(|&ns| candidates[ns].map(|res| (res, ns))) - .collect::>(); - match candidates.as_slice() { - [(first_def, _), (second_def, _)] => { - msg += &format!( - "both {} {} and {} {}", - first_def.article(), - first_def.descr(), - second_def.article(), - second_def.descr(), - ); - } - _ => { - let mut candidates = candidates.iter().peekable(); - while let Some((res, _)) = candidates.next() { - if candidates.peek().is_some() { - msg += &format!("{} {}, ", res.article(), res.descr()); - } else { - msg += &format!("and {} {}", res.article(), res.descr()); - } - } + _ => { + let mut candidates = candidates.iter().peekable(); + while let Some((res, _)) = candidates.next() { + if candidates.peek().is_some() { + msg += &format!("{} {}, ", res.article(), res.descr()); + } else { + msg += &format!("and {} {}", res.article(), res.descr()); } } + } + } - let mut diag = lint.build(&msg); + report_diagnostic(cx, &msg, item, dox, link_range.clone(), |diag, sp| { + if let Some(sp) = sp { + diag.span_label(sp, "ambiguous link"); - if let Some(link_range) = link_range { - if let Some(sp) = super::source_span_for_markdown_range(cx, dox, &link_range, attrs) - { - diag.set_span(sp); - diag.span_label(sp, "ambiguous link"); + let link_range = link_range.expect("must have a link range if we have a span"); - for (res, ns) in candidates { - let (action, mut suggestion) = match res { - Res::Def(DefKind::AssocFn | DefKind::Fn, _) => { - ("add parentheses", format!("{}()", path_str)) - } - Res::Def(DefKind::Macro(MacroKind::Bang), _) => { - ("add an exclamation mark", format!("{}!", path_str)) - } - _ => { - let type_ = match (res, ns) { - (Res::Def(DefKind::Const, _), _) => "const", - (Res::Def(DefKind::Static, _), _) => "static", - (Res::Def(DefKind::Struct, _), _) => "struct", - (Res::Def(DefKind::Enum, _), _) => "enum", - (Res::Def(DefKind::Union, _), _) => "union", - (Res::Def(DefKind::Trait, _), _) => "trait", - (Res::Def(DefKind::Mod, _), _) => "module", - (_, TypeNS) => "type", - (_, ValueNS) => "value", - (Res::Def(DefKind::Macro(MacroKind::Derive), _), MacroNS) => { - "derive" - } - (_, MacroNS) => "macro", - }; - - // FIXME: if this is an implied shortcut link, it's bad style to suggest `@` - ("prefix with the item type", format!("{}@{}", type_, path_str)) - } + for (res, ns) in candidates { + let (action, mut suggestion) = match res { + Res::Def(DefKind::AssocFn | DefKind::Fn, _) => { + ("add parentheses", format!("{}()", path_str)) + } + Res::Def(DefKind::Macro(MacroKind::Bang), _) => { + ("add an exclamation mark", format!("{}!", path_str)) + } + _ => { + let type_ = match (res, ns) { + (Res::Def(DefKind::Const, _), _) => "const", + (Res::Def(DefKind::Static, _), _) => "static", + (Res::Def(DefKind::Struct, _), _) => "struct", + (Res::Def(DefKind::Enum, _), _) => "enum", + (Res::Def(DefKind::Union, _), _) => "union", + (Res::Def(DefKind::Trait, _), _) => "trait", + (Res::Def(DefKind::Mod, _), _) => "module", + (_, TypeNS) => "type", + (_, ValueNS) => "value", + (Res::Def(DefKind::Macro(MacroKind::Derive), _), MacroNS) => "derive", + (_, MacroNS) => "macro", }; - if dox.bytes().nth(link_range.start) == Some(b'`') { - suggestion = format!("`{}`", suggestion); - } - - diag.span_suggestion( - sp, - &format!("to link to the {}, {}", res.descr(), action), - suggestion, - Applicability::MaybeIncorrect, - ); + // FIXME: if this is an implied shortcut link, it's bad style to suggest `@` + ("prefix with the item type", format!("{}@{}", type_, path_str)) } - } else { - // blah blah blah\nblah\nblah [blah] blah blah\nblah blah - // ^ ~~~~ - // | link_range - // last_new_line_offset - let last_new_line_offset = - dox[..link_range.start].rfind('\n').map_or(0, |n| n + 1); - let line = dox[last_new_line_offset..].lines().next().unwrap_or(""); + }; - // Print the line containing the `link_range` and manually mark it with '^'s. - diag.note(&format!( - "the link appears in this line:\n\n{line}\n\ - {indicator: , + item: &Item, + path_str: &str, + dox: &str, + link_range: Option>, +) { + let item_name = item.name.as_deref().unwrap_or(""); + let msg = + format!("public documentation for `{}` links to private item `{}`", item_name, path_str); + + report_diagnostic(cx, &msg, item, dox, link_range, |diag, sp| { + if let Some(sp) = sp { + diag.span_label(sp, "this item is private"); + } + }); } /// Given an enum variant's res, return the res of its enum and the associated fragment. @@ -1089,7 +1081,7 @@ fn handle_variant( use rustc_middle::ty::DefIdTree; if extra_fragment.is_some() { - return Err(ErrorKind::AnchorFailure("variants cannot be followed by anchors")); + return Err(ErrorKind::AnchorFailure(AnchorFailure::Variant)); } let parent = if let Some(parent) = cx.tcx.parent(res.def_id()) { parent diff --git a/src/test/rustdoc-ui/deny-intra-link-resolution-failure.stderr b/src/test/rustdoc-ui/deny-intra-link-resolution-failure.stderr index bc21cfd47c5d1..894518faa3168 100644 --- a/src/test/rustdoc-ui/deny-intra-link-resolution-failure.stderr +++ b/src/test/rustdoc-ui/deny-intra-link-resolution-failure.stderr @@ -1,15 +1,15 @@ -error: `[v2]` cannot be resolved, ignoring it. +error: unresolved link to `v2` --> $DIR/deny-intra-link-resolution-failure.rs:3:6 | LL | /// [v2] - | ^^ cannot be resolved, ignoring + | ^^ unresolved link | note: the lint level is defined here --> $DIR/deny-intra-link-resolution-failure.rs:1:9 | LL | #![deny(intra_doc_link_resolution_failure)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: aborting due to previous error diff --git a/src/test/rustdoc-ui/intra-doc-alias-ice.stderr b/src/test/rustdoc-ui/intra-doc-alias-ice.stderr index cf26675163054..d2b2b90a4e50d 100644 --- a/src/test/rustdoc-ui/intra-doc-alias-ice.stderr +++ b/src/test/rustdoc-ui/intra-doc-alias-ice.stderr @@ -1,15 +1,15 @@ -error: `[TypeAlias::hoge]` cannot be resolved, ignoring it. +error: unresolved link to `TypeAlias::hoge` --> $DIR/intra-doc-alias-ice.rs:5:30 | LL | /// [broken cross-reference](TypeAlias::hoge) - | ^^^^^^^^^^^^^^^ cannot be resolved, ignoring + | ^^^^^^^^^^^^^^^ unresolved link | note: the lint level is defined here --> $DIR/intra-doc-alias-ice.rs:1:9 | LL | #![deny(intra_doc_link_resolution_failure)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: aborting due to previous error diff --git a/src/test/rustdoc-ui/intra-link-span-ice-55723.rs b/src/test/rustdoc-ui/intra-link-span-ice-55723.rs index 44997c90f5932..95388003f8470 100644 --- a/src/test/rustdoc-ui/intra-link-span-ice-55723.rs +++ b/src/test/rustdoc-ui/intra-link-span-ice-55723.rs @@ -7,7 +7,7 @@ /// ## For example: /// /// (arr[i]) -//~^ ERROR `[i]` cannot be resolved, ignoring it. +//~^ ERROR `i` pub fn test_ice() { unimplemented!(); } diff --git a/src/test/rustdoc-ui/intra-link-span-ice-55723.stderr b/src/test/rustdoc-ui/intra-link-span-ice-55723.stderr index ce31eb3a8a378..156e214a79ff0 100644 --- a/src/test/rustdoc-ui/intra-link-span-ice-55723.stderr +++ b/src/test/rustdoc-ui/intra-link-span-ice-55723.stderr @@ -1,15 +1,15 @@ -error: `[i]` cannot be resolved, ignoring it. +error: unresolved link to `i` --> $DIR/intra-link-span-ice-55723.rs:9:10 | LL | /// (arr[i]) - | ^ cannot be resolved, ignoring + | ^ unresolved link | note: the lint level is defined here --> $DIR/intra-link-span-ice-55723.rs:1:9 | LL | #![deny(intra_doc_link_resolution_failure)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: aborting due to previous error diff --git a/src/test/rustdoc-ui/intra-links-anchors.rs b/src/test/rustdoc-ui/intra-links-anchors.rs index 7f8a8dd3c45e0..7e61bd725359a 100644 --- a/src/test/rustdoc-ui/intra-links-anchors.rs +++ b/src/test/rustdoc-ui/intra-links-anchors.rs @@ -23,23 +23,23 @@ pub enum Enum { /// Like [Foo#hola]. /// /// Or maybe [Foo::f#hola]. -//~^ ERROR `[Foo::f#hola]` has an issue with the link anchor. +//~^ ERROR `Foo::f#hola` contains an anchor pub fn foo() {} /// Empty. /// /// Another anchor error: [hello#people#!]. -//~^ ERROR `[hello#people#!]` has an issue with the link anchor. +//~^ ERROR `hello#people#!` contains multiple anchors pub fn bar() {} /// Empty? /// /// Damn enum's variants: [Enum::A#whatever]. -//~^ ERROR `[Enum::A#whatever]` has an issue with the link anchor. +//~^ ERROR `Enum::A#whatever` contains an anchor pub fn enum_link() {} /// Primitives? /// /// [u32#hello] -//~^ ERROR `[u32#hello]` has an issue with the link anchor. +//~^ ERROR `u32#hello` contains an anchor pub fn x() {} diff --git a/src/test/rustdoc-ui/intra-links-anchors.stderr b/src/test/rustdoc-ui/intra-links-anchors.stderr index 11dee5547db4f..ef33d8f3e06fe 100644 --- a/src/test/rustdoc-ui/intra-links-anchors.stderr +++ b/src/test/rustdoc-ui/intra-links-anchors.stderr @@ -1,8 +1,8 @@ -error: `[Foo::f#hola]` has an issue with the link anchor. +error: `Foo::f#hola` contains an anchor, but links to struct fields are already anchored --> $DIR/intra-links-anchors.rs:25:15 | LL | /// Or maybe [Foo::f#hola]. - | ^^^^^^^^^^^ struct fields cannot be followed by anchors + | ^^^^^^^^^^^ contains invalid anchor | note: the lint level is defined here --> $DIR/intra-links-anchors.rs:1:9 @@ -10,23 +10,23 @@ note: the lint level is defined here LL | #![deny(intra_doc_link_resolution_failure)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: `[hello#people#!]` has an issue with the link anchor. +error: `hello#people#!` contains multiple anchors --> $DIR/intra-links-anchors.rs:31:28 | LL | /// Another anchor error: [hello#people#!]. - | ^^^^^^^^^^^^^^ only one `#` is allowed in a link + | ^^^^^^^^^^^^^^ contains invalid anchor -error: `[Enum::A#whatever]` has an issue with the link anchor. +error: `Enum::A#whatever` contains an anchor, but links to enum variants are already anchored --> $DIR/intra-links-anchors.rs:37:28 | LL | /// Damn enum's variants: [Enum::A#whatever]. - | ^^^^^^^^^^^^^^^^ variants cannot be followed by anchors + | ^^^^^^^^^^^^^^^^ contains invalid anchor -error: `[u32#hello]` has an issue with the link anchor. +error: `u32#hello` contains an anchor, but links to primitive types are already anchored --> $DIR/intra-links-anchors.rs:43:6 | LL | /// [u32#hello] - | ^^^^^^^^^ primitive types cannot be followed by anchors + | ^^^^^^^^^ contains invalid anchor error: aborting due to 4 previous errors diff --git a/src/test/rustdoc-ui/intra-links-private.public.stderr b/src/test/rustdoc-ui/intra-links-private.public.stderr index 0a8dafdaf9466..a124435b08a99 100644 --- a/src/test/rustdoc-ui/intra-links-private.public.stderr +++ b/src/test/rustdoc-ui/intra-links-private.public.stderr @@ -1,4 +1,4 @@ -warning: `[DontDocMe]` public documentation for `DocMe` links to a private item +warning: public documentation for `DocMe` links to private item `DontDocMe` --> $DIR/intra-links-private.rs:6:11 | LL | /// docs [DontDocMe] diff --git a/src/test/rustdoc-ui/intra-links-private.rs b/src/test/rustdoc-ui/intra-links-private.rs index 86cf9fed3dab4..1b97f6e61bd23 100644 --- a/src/test/rustdoc-ui/intra-links-private.rs +++ b/src/test/rustdoc-ui/intra-links-private.rs @@ -4,7 +4,7 @@ #![cfg_attr(private, deny(intra_doc_link_resolution_failure))] /// docs [DontDocMe] -//[public]~^ WARNING `[DontDocMe]` public documentation for `DocMe` links to a private item +//[public]~^ WARNING public documentation for `DocMe` links to private item `DontDocMe` // FIXME: for [private] we should also make sure the link was actually generated pub struct DocMe; struct DontDocMe; diff --git a/src/test/rustdoc-ui/intra-links-warning-crlf.rs b/src/test/rustdoc-ui/intra-links-warning-crlf.rs index 18c9837b0bb45..a19c33b53be09 100644 --- a/src/test/rustdoc-ui/intra-links-warning-crlf.rs +++ b/src/test/rustdoc-ui/intra-links-warning-crlf.rs @@ -6,16 +6,16 @@ /// [error] pub struct A; -//~^^ WARNING `[error]` cannot be resolved +//~^^ WARNING `error` /// /// docs [error1] -//~^ WARNING `[error1]` cannot be resolved +//~^ WARNING `error1` /// docs [error2] /// pub struct B; -//~^^^ WARNING `[error2]` cannot be resolved +//~^^^ WARNING `error2` /** * This is a multi-line comment. @@ -23,4 +23,4 @@ pub struct B; * It also has an [error]. */ pub struct C; -//~^^^ WARNING `[error]` cannot be resolved +//~^^^ WARNING `error` diff --git a/src/test/rustdoc-ui/intra-links-warning-crlf.stderr b/src/test/rustdoc-ui/intra-links-warning-crlf.stderr index ac8691a8743ba..bc31264c170ea 100644 --- a/src/test/rustdoc-ui/intra-links-warning-crlf.stderr +++ b/src/test/rustdoc-ui/intra-links-warning-crlf.stderr @@ -1,35 +1,35 @@ -warning: `[error]` cannot be resolved, ignoring it. +warning: unresolved link to `error` --> $DIR/intra-links-warning-crlf.rs:7:6 | LL | /// [error] - | ^^^^^ cannot be resolved, ignoring + | ^^^^^ unresolved link | = note: `#[warn(intra_doc_link_resolution_failure)]` on by default - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[error1]` cannot be resolved, ignoring it. +warning: unresolved link to `error1` --> $DIR/intra-links-warning-crlf.rs:12:11 | LL | /// docs [error1] - | ^^^^^^ cannot be resolved, ignoring + | ^^^^^^ unresolved link | - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[error2]` cannot be resolved, ignoring it. +warning: unresolved link to `error2` --> $DIR/intra-links-warning-crlf.rs:15:11 | LL | /// docs [error2] - | ^^^^^^ cannot be resolved, ignoring + | ^^^^^^ unresolved link | - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[error]` cannot be resolved, ignoring it. +warning: unresolved link to `error` --> $DIR/intra-links-warning-crlf.rs:23:20 | LL | * It also has an [error]. - | ^^^^^ cannot be resolved, ignoring + | ^^^^^ unresolved link | - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: 4 warnings emitted diff --git a/src/test/rustdoc-ui/intra-links-warning.rs b/src/test/rustdoc-ui/intra-links-warning.rs index 623dcc320bb8d..eab1f03480460 100644 --- a/src/test/rustdoc-ui/intra-links-warning.rs +++ b/src/test/rustdoc-ui/intra-links-warning.rs @@ -1,37 +1,37 @@ // check-pass //! Test with [Foo::baz], [Bar::foo], ... -//~^ WARNING `[Foo::baz]` cannot be resolved -//~| WARNING `[Bar::foo]` cannot be resolved +//~^ WARNING `Foo::baz` +//~| WARNING `Bar::foo` //! , [Uniooon::X] and [Qux::Z]. -//~^ WARNING `[Uniooon::X]` cannot be resolved -//~| WARNING `[Qux::Z]` cannot be resolved +//~^ WARNING `Uniooon::X` +//~| WARNING `Qux::Z` //! //! , [Uniooon::X] and [Qux::Z]. -//~^ WARNING `[Uniooon::X]` cannot be resolved -//~| WARNING `[Qux::Z]` cannot be resolved +//~^ WARNING `Uniooon::X` +//~| WARNING `Qux::Z` /// [Qux:Y] -//~^ WARNING `[Qux:Y]` cannot be resolved +//~^ WARNING `Qux:Y` pub struct Foo { pub bar: usize, } /// Foo -/// bar [BarA] bar //~ WARNING `[BarA]` cannot be resolved +/// bar [BarA] bar //~ WARNING `BarA` /// baz pub fn a() {} /** * Foo - * bar [BarB] bar //~ WARNING `[BarB]` cannot be resolved + * bar [BarB] bar //~ WARNING `BarB` * baz */ pub fn b() {} /** Foo -bar [BarC] bar //~ WARNING `[BarC]` cannot be resolved +bar [BarC] bar //~ WARNING `BarC` baz let bar_c_1 = 0; @@ -42,12 +42,12 @@ baz */ pub fn c() {} -#[doc = "Foo\nbar [BarD] bar\nbaz"] //~ WARNING `[BarD]` cannot be resolved +#[doc = "Foo\nbar [BarD] bar\nbaz"] //~ WARNING `BarD` pub fn d() {} macro_rules! f { ($f:expr) => { - #[doc = $f] //~ WARNING `[BarF]` cannot be resolved + #[doc = $f] //~ WARNING `BarF` pub fn f() {} } } @@ -55,30 +55,30 @@ f!("Foo\nbar [BarF] bar\nbaz"); /** # for example, * - * time to introduce a link [error]*/ //~ WARNING `[error]` cannot be resolved + * time to introduce a link [error]*/ //~ WARNING `error` pub struct A; /** * # for example, * - * time to introduce a link [error] //~ WARNING `[error]` cannot be resolved + * time to introduce a link [error] //~ WARNING `error` */ pub struct B; -#[doc = "single line [error]"] //~ WARNING `[error]` cannot be resolved +#[doc = "single line [error]"] //~ WARNING `error` pub struct C; -#[doc = "single line with \"escaping\" [error]"] //~ WARNING `[error]` cannot be resolved +#[doc = "single line with \"escaping\" [error]"] //~ WARNING `error` pub struct D; -/// Item docs. //~ WARNING `[error]` cannot be resolved +/// Item docs. //~ WARNING `error` #[doc="Hello there!"] /// [error] pub struct E; /// -/// docs [error1] //~ WARNING `[error1]` cannot be resolved +/// docs [error1] //~ WARNING `error1` -/// docs [error2] //~ WARNING `[error2]` cannot be resolved +/// docs [error2] //~ WARNING `error2` /// pub struct F; diff --git a/src/test/rustdoc-ui/intra-links-warning.stderr b/src/test/rustdoc-ui/intra-links-warning.stderr index 914a19fc536c7..81931399c240a 100644 --- a/src/test/rustdoc-ui/intra-links-warning.stderr +++ b/src/test/rustdoc-ui/intra-links-warning.stderr @@ -1,77 +1,77 @@ -warning: `[Foo::baz]` cannot be resolved, ignoring it. +warning: unresolved link to `Foo::baz` --> $DIR/intra-links-warning.rs:3:23 | LL | //! Test with [Foo::baz], [Bar::foo], ... - | ^^^^^^^^ cannot be resolved, ignoring + | ^^^^^^^^ unresolved link | = note: `#[warn(intra_doc_link_resolution_failure)]` on by default - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[Bar::foo]` cannot be resolved, ignoring it. +warning: unresolved link to `Bar::foo` --> $DIR/intra-links-warning.rs:3:35 | LL | //! Test with [Foo::baz], [Bar::foo], ... - | ^^^^^^^^ cannot be resolved, ignoring + | ^^^^^^^^ unresolved link | - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[Uniooon::X]` cannot be resolved, ignoring it. +warning: unresolved link to `Uniooon::X` --> $DIR/intra-links-warning.rs:6:13 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^^^^^ cannot be resolved, ignoring + | ^^^^^^^^^^ unresolved link | - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[Qux::Z]` cannot be resolved, ignoring it. +warning: unresolved link to `Qux::Z` --> $DIR/intra-links-warning.rs:6:30 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^ cannot be resolved, ignoring + | ^^^^^^ unresolved link | - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[Uniooon::X]` cannot be resolved, ignoring it. +warning: unresolved link to `Uniooon::X` --> $DIR/intra-links-warning.rs:10:14 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^^^^^ cannot be resolved, ignoring + | ^^^^^^^^^^ unresolved link | - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[Qux::Z]` cannot be resolved, ignoring it. +warning: unresolved link to `Qux::Z` --> $DIR/intra-links-warning.rs:10:31 | LL | //! , [Uniooon::X] and [Qux::Z]. - | ^^^^^^ cannot be resolved, ignoring + | ^^^^^^ unresolved link | - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[Qux:Y]` cannot be resolved, ignoring it. +warning: unresolved link to `Qux:Y` --> $DIR/intra-links-warning.rs:14:13 | LL | /// [Qux:Y] - | ^^^^^ cannot be resolved, ignoring + | ^^^^^ unresolved link | - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[error]` cannot be resolved, ignoring it. +warning: unresolved link to `error` --> $DIR/intra-links-warning.rs:58:30 | LL | * time to introduce a link [error]*/ - | ^^^^^ cannot be resolved, ignoring + | ^^^^^ unresolved link | - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[error]` cannot be resolved, ignoring it. +warning: unresolved link to `error` --> $DIR/intra-links-warning.rs:64:30 | LL | * time to introduce a link [error] - | ^^^^^ cannot be resolved, ignoring + | ^^^^^ unresolved link | - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[error]` cannot be resolved, ignoring it. +warning: unresolved link to `error` --> $DIR/intra-links-warning.rs:68:1 | LL | #[doc = "single line [error]"] @@ -81,9 +81,9 @@ LL | #[doc = "single line [error]"] single line [error] ^^^^^ - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[error]` cannot be resolved, ignoring it. +warning: unresolved link to `error` --> $DIR/intra-links-warning.rs:71:1 | LL | #[doc = "single line with \"escaping\" [error]"] @@ -93,9 +93,9 @@ LL | #[doc = "single line with \"escaping\" [error]"] single line with "escaping" [error] ^^^^^ - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[error]` cannot be resolved, ignoring it. +warning: unresolved link to `error` --> $DIR/intra-links-warning.rs:74:1 | LL | / /// Item docs. @@ -107,49 +107,49 @@ LL | | /// [error] [error] ^^^^^ - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[error1]` cannot be resolved, ignoring it. +warning: unresolved link to `error1` --> $DIR/intra-links-warning.rs:80:11 | LL | /// docs [error1] - | ^^^^^^ cannot be resolved, ignoring + | ^^^^^^ unresolved link | - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[error2]` cannot be resolved, ignoring it. +warning: unresolved link to `error2` --> $DIR/intra-links-warning.rs:82:11 | LL | /// docs [error2] - | ^^^^^^ cannot be resolved, ignoring + | ^^^^^^ unresolved link | - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[BarA]` cannot be resolved, ignoring it. +warning: unresolved link to `BarA` --> $DIR/intra-links-warning.rs:21:10 | LL | /// bar [BarA] bar - | ^^^^ cannot be resolved, ignoring + | ^^^^ unresolved link | - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[BarB]` cannot be resolved, ignoring it. +warning: unresolved link to `BarB` --> $DIR/intra-links-warning.rs:27:9 | LL | * bar [BarB] bar - | ^^^^ cannot be resolved, ignoring + | ^^^^ unresolved link | - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[BarC]` cannot be resolved, ignoring it. +warning: unresolved link to `BarC` --> $DIR/intra-links-warning.rs:34:6 | LL | bar [BarC] bar - | ^^^^ cannot be resolved, ignoring + | ^^^^ unresolved link | - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[BarD]` cannot be resolved, ignoring it. +warning: unresolved link to `BarD` --> $DIR/intra-links-warning.rs:45:1 | LL | #[doc = "Foo\nbar [BarD] bar\nbaz"] @@ -159,9 +159,9 @@ LL | #[doc = "Foo\nbar [BarD] bar\nbaz"] bar [BarD] bar ^^^^ - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` -warning: `[BarF]` cannot be resolved, ignoring it. +warning: unresolved link to `BarF` --> $DIR/intra-links-warning.rs:50:9 | LL | #[doc = $f] @@ -174,7 +174,7 @@ LL | f!("Foo\nbar [BarF] bar\nbaz"); bar [BarF] bar ^^^^ - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) warning: 19 warnings emitted diff --git a/src/test/rustdoc-ui/issue-74134.public.stderr b/src/test/rustdoc-ui/issue-74134.public.stderr index 03f95f19d326e..3c41f7e63e637 100644 --- a/src/test/rustdoc-ui/issue-74134.public.stderr +++ b/src/test/rustdoc-ui/issue-74134.public.stderr @@ -1,4 +1,4 @@ -warning: `[PrivateType]` public documentation for `public_item` links to a private item +warning: public documentation for `public_item` links to private item `PrivateType` --> $DIR/issue-74134.rs:19:10 | LL | /// [`PrivateType`] diff --git a/src/test/rustdoc-ui/issue-74134.rs b/src/test/rustdoc-ui/issue-74134.rs index d561c2dd89015..fe484b43fb6fc 100644 --- a/src/test/rustdoc-ui/issue-74134.rs +++ b/src/test/rustdoc-ui/issue-74134.rs @@ -17,7 +17,7 @@ pub struct PublicType; pub struct Public { /// [`PublicType`] /// [`PrivateType`] - //[public]~^ WARNING public documentation for `public_item` links to a private + //[public]~^ WARNING public documentation for `public_item` links to private item `PrivateType` pub public_item: u32, /// [`PublicType`] diff --git a/src/test/rustdoc-ui/lint-group.rs b/src/test/rustdoc-ui/lint-group.rs index 06766db5335a1..e58c8b12f68cb 100644 --- a/src/test/rustdoc-ui/lint-group.rs +++ b/src/test/rustdoc-ui/lint-group.rs @@ -11,7 +11,7 @@ /// ``` /// println!("sup"); /// ``` -pub fn link_error() {} //~^^^^^ ERROR cannot be resolved, ignoring it +pub fn link_error() {} //~^^^^^ ERROR unresolved link to `error` /// wait, this doesn't have a doctest? pub fn no_doctest() {} //~^ ERROR missing code example in this documentation diff --git a/src/test/rustdoc-ui/lint-group.stderr b/src/test/rustdoc-ui/lint-group.stderr index 852c9120e0bf9..14d72e9aad3ba 100644 --- a/src/test/rustdoc-ui/lint-group.stderr +++ b/src/test/rustdoc-ui/lint-group.stderr @@ -15,11 +15,11 @@ LL | #![deny(rustdoc)] | ^^^^^^^ = note: `#[deny(private_doc_tests)]` implied by `#[deny(rustdoc)]` -error: `[error]` cannot be resolved, ignoring it. +error: unresolved link to `error` --> $DIR/lint-group.rs:9:29 | LL | /// what up, let's make an [error] - | ^^^^^ cannot be resolved, ignoring + | ^^^^^ unresolved link | note: the lint level is defined here --> $DIR/lint-group.rs:7:9 @@ -27,7 +27,7 @@ note: the lint level is defined here LL | #![deny(rustdoc)] | ^^^^^^^ = note: `#[deny(intra_doc_link_resolution_failure)]` implied by `#[deny(rustdoc)]` - = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]` + = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: missing code example in this documentation --> $DIR/lint-group.rs:16:1 From 8b58eb9dc49254926c91690380c065403b10fbb8 Mon Sep 17 00:00:00 2001 From: Jamie Cunliffe Date: Tue, 21 Jul 2020 13:46:22 +0100 Subject: [PATCH 21/23] Remove the assert on alignment check. Also the alignment should only be done on general register types as per the AAPCS so fixed that issue. Copyright (c) 2020, Arm Limited. --- src/librustc_codegen_llvm/va_arg.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/librustc_codegen_llvm/va_arg.rs b/src/librustc_codegen_llvm/va_arg.rs index 8c0d256df2357..54efa05aee81e 100644 --- a/src/librustc_codegen_llvm/va_arg.rs +++ b/src/librustc_codegen_llvm/va_arg.rs @@ -128,8 +128,7 @@ fn emit_aapcs_va_arg( // it could be on the stack so we have to update the offset and then check // the offset again. - if layout.align.abi.bytes() > 8 { - assert!(layout.align.abi.bytes() <= 16); + if gr_type && layout.align.abi.bytes() > 8 { reg_off_v = maybe_reg.add(reg_off_v, bx.const_i32(15)); reg_off_v = maybe_reg.and(reg_off_v, bx.const_i32(-16)); } From 3eed7da1f93960871f16f8d6558a7b4356267f57 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 21 Jul 2020 07:58:59 -0700 Subject: [PATCH 22/23] Update books --- src/doc/book | 2 +- src/doc/edition-guide | 2 +- src/doc/reference | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/doc/book b/src/doc/book index 84a31397b34f9..a914f2c7e5cdb 160000 --- a/src/doc/book +++ b/src/doc/book @@ -1 +1 @@ -Subproject commit 84a31397b34f9d405df44f2899ff17a4828dba18 +Subproject commit a914f2c7e5cdb771fa465de142381a51c53b580e diff --git a/src/doc/edition-guide b/src/doc/edition-guide index 82bec5877c77c..bd6e4a9f59c5c 160000 --- a/src/doc/edition-guide +++ b/src/doc/edition-guide @@ -1 +1 @@ -Subproject commit 82bec5877c77cfad530ca11095db4456d757f668 +Subproject commit bd6e4a9f59c5c1545f572266af77f5c7a5bad6d1 diff --git a/src/doc/reference b/src/doc/reference index 0ea7bc494f128..b329ce3742487 160000 --- a/src/doc/reference +++ b/src/doc/reference @@ -1 +1 @@ -Subproject commit 0ea7bc494f1289234d8800bb9185021e0ad946f0 +Subproject commit b329ce37424874ad4db94f829a55807c6e21d2cb From 96225b15e5480dae3c7e0f3df044553502c82d66 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 22 Jul 2020 16:28:47 +0200 Subject: [PATCH 23/23] Fix tooltip position if the documentation starts with a code block --- src/librustdoc/html/static/rustdoc.css | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/librustdoc/html/static/rustdoc.css b/src/librustdoc/html/static/rustdoc.css index 38709b445efae..85c125e5efdd4 100644 --- a/src/librustdoc/html/static/rustdoc.css +++ b/src/librustdoc/html/static/rustdoc.css @@ -1291,6 +1291,16 @@ h4 > .important-traits { /* Media Queries */ +@media (min-width: 701px) { + /* In case there is no documentation before a code block, we need to add some margin at the top + to prevent an overlay between the "collapse toggle" and the information tooltip. + However, it's needed needed with smaller screen width because the doc/code block is always put + "one line" below. */ + .information:first-child > .tooltip { + margin-top: 16px; + } +} + @media (max-width: 700px) { body { padding-top: 0px;