From 6ef203388483688b0a4598efbc63ee7295bff975 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Wed, 18 May 2022 03:51:52 +0100 Subject: [PATCH 01/24] Fix FFI-unwind unsoundness with mixed panic mode --- compiler/rustc_interface/src/passes.rs | 1 + compiler/rustc_lint_defs/src/builtin.rs | 40 +++++ compiler/rustc_metadata/src/creader.rs | 2 +- .../rustc_metadata/src/dependency_format.rs | 11 +- compiler/rustc_metadata/src/rmeta/decoder.rs | 2 +- compiler/rustc_metadata/src/rmeta/encoder.rs | 2 +- compiler/rustc_metadata/src/rmeta/mod.rs | 2 +- compiler/rustc_middle/src/query/mod.rs | 11 +- .../src/ffi_unwind_calls.rs | 148 ++++++++++++++++++ compiler/rustc_mir_transform/src/lib.rs | 5 + library/std/src/lib.rs | 2 + 11 files changed, 212 insertions(+), 14 deletions(-) create mode 100644 compiler/rustc_mir_transform/src/ffi_unwind_calls.rs diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 3c867e308c40e..ef9c7859dbb7a 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -944,6 +944,7 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> { if !tcx.sess.opts.debugging_opts.thir_unsafeck { rustc_mir_transform::check_unsafety::check_unsafety(tcx, def_id); } + tcx.ensure().has_ffi_unwind_calls(def_id); if tcx.hir().body_const_context(def_id).is_some() { tcx.ensure() diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index a067534b18938..48d89a785c117 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -3230,6 +3230,7 @@ declare_lint_pass! { UNEXPECTED_CFGS, DEPRECATED_WHERE_CLAUSE_LOCATION, TEST_UNSTABLE_LINT, + FFI_UNWIND_CALLS, ] } @@ -3895,3 +3896,42 @@ declare_lint! { "this unstable lint is only for testing", @feature_gate = sym::test_unstable_lint; } + +declare_lint! { + /// The `ffi_unwind_calls` lint detects calls to foreign functions or function pointers with + /// `C-unwind` or other FFI-unwind ABIs. + /// + /// ### Example + /// + /// ```rust,ignore (need FFI) + /// #![feature(ffi_unwind_calls)] + /// #![feature(c_unwind)] + /// + /// # mod impl { + /// # #[no_mangle] + /// # pub fn "C-unwind" fn foo() {} + /// # } + /// + /// extern "C-unwind" { + /// fn foo(); + /// } + /// + /// fn bar() { + /// unsafe { foo(); } + /// let ptr: unsafe extern "C-unwind" fn() = foo; + /// unsafe { ptr(); } + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// For crates containing such calls, if they are compiled with `-C panic=unwind` then the + /// produced library cannot be linked with crates compiled with `-C panic=abort`. For crates + /// that desire this ability it is therefore necessary to avoid such calls. + pub FFI_UNWIND_CALLS, + Allow, + "call to foreign functions or function pointers with FFI-unwind ABI", + @feature_gate = sym::c_unwind; +} diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 947d563ae3cd1..3f6d1f050056d 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -744,7 +744,7 @@ impl<'a> CrateLoader<'a> { if !data.is_panic_runtime() { self.sess.err(&format!("the crate `{}` is not a panic runtime", name)); } - if data.panic_strategy() != desired_strategy { + if data.panic_strategy() != Some(desired_strategy) { self.sess.err(&format!( "the crate `{}` does not have the panic \ strategy `{}`", diff --git a/compiler/rustc_metadata/src/dependency_format.rs b/compiler/rustc_metadata/src/dependency_format.rs index 245b2076ebca9..349ff08124cf6 100644 --- a/compiler/rustc_metadata/src/dependency_format.rs +++ b/compiler/rustc_metadata/src/dependency_format.rs @@ -60,7 +60,6 @@ use rustc_middle::ty::TyCtxt; use rustc_session::config::CrateType; use rustc_session::cstore::CrateDepKind; use rustc_session::cstore::LinkagePreference::{self, RequireDynamic, RequireStatic}; -use rustc_target::spec::PanicStrategy; pub(crate) fn calculate(tcx: TyCtxt<'_>) -> Dependencies { tcx.sess @@ -367,7 +366,7 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { prev_name, cur_name )); } - panic_runtime = Some((cnum, tcx.panic_strategy(cnum))); + panic_runtime = Some((cnum, tcx.panic_strategy(cnum).unwrap())); } } @@ -397,18 +396,14 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { if let Linkage::NotLinked = *linkage { continue; } - if desired_strategy == PanicStrategy::Abort { - continue; - } let cnum = CrateNum::new(i + 1); if tcx.is_compiler_builtins(cnum) { continue; } - let found_strategy = tcx.panic_strategy(cnum); - if desired_strategy != found_strategy { + if let Some(found_strategy) = tcx.panic_strategy(cnum) && desired_strategy != found_strategy { sess.err(&format!( - "the crate `{}` is compiled with the \ + "the crate `{}` requires \ panic strategy `{}` which is \ incompatible with this crate's \ strategy of `{}`", diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 03ac82b467b8e..658c51bf62043 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -1759,7 +1759,7 @@ impl CrateMetadata { self.dep_kind.with_lock(|dep_kind| *dep_kind = f(*dep_kind)) } - pub(crate) fn panic_strategy(&self) -> PanicStrategy { + pub(crate) fn panic_strategy(&self) -> Option { self.root.panic_strategy } diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index 8867e008e42fa..cf685a7bd6217 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -665,7 +665,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { triple: tcx.sess.opts.target_triple.clone(), hash: tcx.crate_hash(LOCAL_CRATE), stable_crate_id: tcx.def_path_hash(LOCAL_CRATE.as_def_id()).stable_crate_id(), - panic_strategy: tcx.sess.panic_strategy(), + panic_strategy: tcx.required_panic_strategy(()), panic_in_drop_strategy: tcx.sess.opts.debugging_opts.panic_in_drop, edition: tcx.sess.edition(), has_global_allocator: tcx.has_global_allocator(LOCAL_CRATE), diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs index 04f0847f5cccc..60510e535b244 100644 --- a/compiler/rustc_metadata/src/rmeta/mod.rs +++ b/compiler/rustc_metadata/src/rmeta/mod.rs @@ -217,7 +217,7 @@ pub(crate) struct CrateRoot { extra_filename: String, hash: Svh, stable_crate_id: StableCrateId, - panic_strategy: PanicStrategy, + panic_strategy: Option, panic_in_drop_strategy: PanicStrategy, edition: Edition, has_global_allocator: bool, diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 5b48f164016f7..5d0d11a1b784a 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1365,9 +1365,16 @@ rustc_queries! { desc { "query a crate is `#![profiler_runtime]`" } separate_provide_extern } - query panic_strategy(_: CrateNum) -> PanicStrategy { + query has_ffi_unwind_calls(key: LocalDefId) -> bool { + desc { |tcx| "check if `{}` contains FFI-unwind calls", tcx.def_path_str(key.to_def_id()) } + cache_on_disk_if { true } + } + query required_panic_strategy(_: ()) -> Option { + desc { "compute the required panic strategy for the current crate" } + } + query panic_strategy(_: CrateNum) -> Option { fatal_cycle - desc { "query a crate's configured panic strategy" } + desc { "query a crate's required panic strategy" } separate_provide_extern } query panic_in_drop_strategy(_: CrateNum) -> PanicStrategy { diff --git a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs new file mode 100644 index 0000000000000..e6f8ef5759a11 --- /dev/null +++ b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs @@ -0,0 +1,148 @@ +use rustc_hir::def::DefKind; +use rustc_hir::def_id::LocalDefId; +use rustc_middle::mir::*; +use rustc_middle::ty::layout; +use rustc_middle::ty::query::Providers; +use rustc_middle::ty::{self, TyCtxt}; +use rustc_session::lint::builtin::FFI_UNWIND_CALLS; +use rustc_target::spec::abi::Abi; +use rustc_target::spec::PanicStrategy; + +fn abi_can_unwind(abi: Abi) -> bool { + use Abi::*; + match abi { + C { unwind } + | System { unwind } + | Cdecl { unwind } + | Stdcall { unwind } + | Fastcall { unwind } + | Vectorcall { unwind } + | Thiscall { unwind } + | Aapcs { unwind } + | Win64 { unwind } + | SysV64 { unwind } => unwind, + PtxKernel + | Msp430Interrupt + | X86Interrupt + | AmdGpuKernel + | EfiApi + | AvrInterrupt + | AvrNonBlockingInterrupt + | CCmseNonSecureCall + | Wasm + | RustIntrinsic + | PlatformIntrinsic + | Unadjusted => false, + Rust | RustCall | RustCold => true, + } +} + +// Check if the body of this def_id can possibly leak a foreign unwind into Rust code. +fn has_ffi_unwind_calls(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> bool { + debug!("has_ffi_unwind_calls({local_def_id:?})"); + + // Only perform check on functions because constants cannot call FFI functions. + let def_id = local_def_id.to_def_id(); + let kind = tcx.def_kind(def_id); + let is_function = match kind { + DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(..) => true, + _ => tcx.is_closure(def_id), + }; + if !is_function { + return false; + } + + let body = &*tcx.mir_built(ty::WithOptConstParam::unknown(local_def_id)).borrow(); + + let body_ty = tcx.type_of(def_id); + let body_abi = match body_ty.kind() { + ty::FnDef(..) => body_ty.fn_sig(tcx).abi(), + ty::Closure(..) => Abi::RustCall, + ty::Generator(..) => Abi::Rust, + _ => span_bug!(body.span, "unexpected body ty: {:?}", body_ty), + }; + let body_can_unwind = layout::fn_can_unwind(tcx, Some(def_id), body_abi); + + // Foreign unwinds cannot leak past functions that themselves cannot unwind. + if !body_can_unwind { + return false; + } + + let mut tainted = false; + + for block in body.basic_blocks() { + if block.is_cleanup { + continue; + } + let Some(terminator) = &block.terminator else { continue }; + let TerminatorKind::Call { func, .. } = &terminator.kind else { continue }; + + let ty = func.ty(body, tcx); + let sig = ty.fn_sig(tcx); + + // Rust calls cannot themselves create foreign unwinds. + if let Abi::Rust | Abi::RustCall | Abi::RustCold = sig.abi() { + continue; + }; + + let fn_def_id = match ty.kind() { + ty::FnPtr(_) => None, + &ty::FnDef(def_id, _) => { + // Rust calls cannot themselves create foreign unwinds. + if !tcx.is_foreign_item(def_id) { + continue; + } + Some(def_id) + } + _ => bug!("invalid callee of type {:?}", ty), + }; + + if layout::fn_can_unwind(tcx, fn_def_id, sig.abi()) && abi_can_unwind(sig.abi()) { + // We have detected a call that can possibly leak foreign unwind. + // + // Because the function body itself can unwind, we are not aborting this function call + // upon unwind, so this call can possibly leak foreign unwind into Rust code if the + // panic runtime linked is panic-abort. + + let lint_root = body.source_scopes[terminator.source_info.scope] + .local_data + .as_ref() + .assert_crate_local() + .lint_root; + let span = terminator.source_info.span; + + tcx.struct_span_lint_hir(FFI_UNWIND_CALLS, lint_root, span, |lint| { + let msg = match fn_def_id { + Some(_) => "call to foreign function with FFI-unwind ABI", + None => "call to function pointer with FFI-unwind ABI", + }; + let mut db = lint.build(msg); + db.span_label(span, msg); + db.emit(); + }); + + tainted = true; + } + } + + tainted +} + +fn required_panic_strategy(tcx: TyCtxt<'_>, (): ()) -> Option { + if tcx.sess.panic_strategy() == PanicStrategy::Abort { + return Some(PanicStrategy::Abort); + } + + for def_id in tcx.hir().body_owners() { + if tcx.has_ffi_unwind_calls(def_id) { + return Some(PanicStrategy::Unwind); + } + } + + // This crate can be linked with either runtime. + None +} + +pub(crate) fn provide(providers: &mut Providers) { + *providers = Providers { has_ffi_unwind_calls, required_panic_strategy, ..*providers }; +} diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 0e57c3c697999..e4591c3f23e71 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -57,6 +57,7 @@ mod dest_prop; pub mod dump_mir; mod early_otherwise_branch; mod elaborate_drops; +mod ffi_unwind_calls; mod function_item_references; mod generator; mod inline; @@ -96,6 +97,7 @@ pub fn provide(providers: &mut Providers) { check_unsafety::provide(providers); check_packed_ref::provide(providers); coverage::query::provide(providers); + ffi_unwind_calls::provide(providers); shim::provide(providers); *providers = Providers { mir_keys, @@ -221,6 +223,9 @@ fn mir_const<'tcx>( } } + // has_ffi_unwind_calls query uses the raw mir, so make sure it is run. + tcx.ensure().has_ffi_unwind_calls(def.did); + let mut body = tcx.mir_built(def).steal(); rustc_middle::mir::dump_mir(tcx, None, "mir_map", &0, &body, |_, _| Ok(())); diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index b1c68ec43bc99..fc2ff3635133c 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -210,6 +210,8 @@ #![allow(unused_lifetimes)] // Tell the compiler to link to either panic_abort or panic_unwind #![needs_panic_runtime] +// Ensure that std can be linked against panic_abort despite compiled with `-C panic=unwind` +#![cfg_attr(not(bootstrap), deny(ffi_unwind_calls))] // std may use features in a platform-specific way #![allow(unused_features)] #![cfg_attr(test, feature(internal_output_capture, print_internals, update_panic_count))] From 77fd0cc566b4eca108b3580312f1e298fc9af8df Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Thu, 19 May 2022 17:38:54 +0100 Subject: [PATCH 02/24] Handle panic runtime specially --- compiler/rustc_metadata/src/dependency_format.rs | 13 +++++++++---- .../rustc_mir_transform/src/ffi_unwind_calls.rs | 6 +++++- .../ui/panic-runtime/transitive-link-a-bunch.stderr | 6 ++---- src/test/ui/panic-runtime/want-unwind-got-abort.rs | 2 +- .../ui/panic-runtime/want-unwind-got-abort.stderr | 4 +--- .../ui/panic-runtime/want-unwind-got-abort2.stderr | 6 ++---- 6 files changed, 20 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_metadata/src/dependency_format.rs b/compiler/rustc_metadata/src/dependency_format.rs index 349ff08124cf6..e22c903bf336e 100644 --- a/compiler/rustc_metadata/src/dependency_format.rs +++ b/compiler/rustc_metadata/src/dependency_format.rs @@ -366,14 +366,19 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { prev_name, cur_name )); } - panic_runtime = Some((cnum, tcx.panic_strategy(cnum).unwrap())); + panic_runtime = Some(( + cnum, + tcx.panic_strategy(cnum).unwrap_or_else(|| { + bug!("cannot determine panic strategy of a panic runtime"); + }), + )); } } // If we found a panic runtime, then we know by this point that it's the // only one, but we perform validation here that all the panic strategy // compilation modes for the whole DAG are valid. - if let Some((cnum, found_strategy)) = panic_runtime { + if let Some((runtime_cnum, found_strategy)) = panic_runtime { let desired_strategy = sess.panic_strategy(); // First up, validate that our selected panic runtime is indeed exactly @@ -383,7 +388,7 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { "the linked panic runtime `{}` is \ not compiled with this crate's \ panic strategy `{}`", - tcx.crate_name(cnum), + tcx.crate_name(runtime_cnum), desired_strategy.desc() )); } @@ -397,7 +402,7 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { continue; } let cnum = CrateNum::new(i + 1); - if tcx.is_compiler_builtins(cnum) { + if cnum == runtime_cnum || tcx.is_compiler_builtins(cnum) { continue; } diff --git a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs index e6f8ef5759a11..78809b105359d 100644 --- a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs +++ b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs @@ -1,5 +1,5 @@ use rustc_hir::def::DefKind; -use rustc_hir::def_id::LocalDefId; +use rustc_hir::def_id::{LocalDefId, LOCAL_CRATE}; use rustc_middle::mir::*; use rustc_middle::ty::layout; use rustc_middle::ty::query::Providers; @@ -129,6 +129,10 @@ fn has_ffi_unwind_calls(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> bool { } fn required_panic_strategy(tcx: TyCtxt<'_>, (): ()) -> Option { + if tcx.is_panic_runtime(LOCAL_CRATE) { + return Some(tcx.sess.panic_strategy()); + } + if tcx.sess.panic_strategy() == PanicStrategy::Abort { return Some(PanicStrategy::Abort); } diff --git a/src/test/ui/panic-runtime/transitive-link-a-bunch.stderr b/src/test/ui/panic-runtime/transitive-link-a-bunch.stderr index 4af754c81f97c..7f4a8ed290ecf 100644 --- a/src/test/ui/panic-runtime/transitive-link-a-bunch.stderr +++ b/src/test/ui/panic-runtime/transitive-link-a-bunch.stderr @@ -2,9 +2,7 @@ error: cannot link together two panic runtimes: panic_runtime_unwind and panic_r error: the linked panic runtime `panic_runtime_abort` is not compiled with this crate's panic strategy `unwind` -error: the crate `wants_panic_runtime_abort` is compiled with the panic strategy `abort` which is incompatible with this crate's strategy of `unwind` +error: the crate `wants_panic_runtime_abort` requires panic strategy `abort` which is incompatible with this crate's strategy of `unwind` -error: the crate `panic_runtime_abort` is compiled with the panic strategy `abort` which is incompatible with this crate's strategy of `unwind` - -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors diff --git a/src/test/ui/panic-runtime/want-unwind-got-abort.rs b/src/test/ui/panic-runtime/want-unwind-got-abort.rs index c48caaf079077..23bfea6af15c1 100644 --- a/src/test/ui/panic-runtime/want-unwind-got-abort.rs +++ b/src/test/ui/panic-runtime/want-unwind-got-abort.rs @@ -1,6 +1,6 @@ // build-fail // needs-unwind -// error-pattern:is incompatible with this crate's strategy of `unwind` +// error-pattern:is not compiled with this crate's panic strategy `unwind` // aux-build:panic-runtime-abort.rs // aux-build:panic-runtime-lang-items.rs // ignore-wasm32-bare compiled with panic=abort by default diff --git a/src/test/ui/panic-runtime/want-unwind-got-abort.stderr b/src/test/ui/panic-runtime/want-unwind-got-abort.stderr index d4fd2cca81fdc..d306ce6c5ea28 100644 --- a/src/test/ui/panic-runtime/want-unwind-got-abort.stderr +++ b/src/test/ui/panic-runtime/want-unwind-got-abort.stderr @@ -1,6 +1,4 @@ error: the linked panic runtime `panic_runtime_abort` is not compiled with this crate's panic strategy `unwind` -error: the crate `panic_runtime_abort` is compiled with the panic strategy `abort` which is incompatible with this crate's strategy of `unwind` - -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/src/test/ui/panic-runtime/want-unwind-got-abort2.stderr b/src/test/ui/panic-runtime/want-unwind-got-abort2.stderr index 364a27a24eb70..014437b7f1b66 100644 --- a/src/test/ui/panic-runtime/want-unwind-got-abort2.stderr +++ b/src/test/ui/panic-runtime/want-unwind-got-abort2.stderr @@ -1,8 +1,6 @@ error: the linked panic runtime `panic_runtime_abort` is not compiled with this crate's panic strategy `unwind` -error: the crate `wants_panic_runtime_abort` is compiled with the panic strategy `abort` which is incompatible with this crate's strategy of `unwind` +error: the crate `wants_panic_runtime_abort` requires panic strategy `abort` which is incompatible with this crate's strategy of `unwind` -error: the crate `panic_runtime_abort` is compiled with the panic strategy `abort` which is incompatible with this crate's strategy of `unwind` - -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors From bc5afd9e347c79d7e0c910eb4f452410579ca4b6 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Thu, 19 May 2022 23:37:26 +0100 Subject: [PATCH 03/24] Ensure ffi_unwind_calls lint is gated behind c_unwind --- .../ui/unwind-abis/feature-gate-c-unwind.rs | 4 +++ .../unwind-abis/feature-gate-c-unwind.stderr | 25 +++++++++++++++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/test/ui/unwind-abis/feature-gate-c-unwind.rs b/src/test/ui/unwind-abis/feature-gate-c-unwind.rs index f02a368d4e097..ba72f74f20ce6 100644 --- a/src/test/ui/unwind-abis/feature-gate-c-unwind.rs +++ b/src/test/ui/unwind-abis/feature-gate-c-unwind.rs @@ -1,6 +1,10 @@ // Test that the "C-unwind" ABI is feature-gated, and cannot be used when the // `c_unwind` feature gate is not used. +#![allow(ffi_unwind_calls)] +//~^ WARNING unknown lint: `ffi_unwind_calls` +//~| WARNING unknown lint: `ffi_unwind_calls` + extern "C-unwind" fn f() {} //~^ ERROR C-unwind ABI is experimental and subject to change [E0658] diff --git a/src/test/ui/unwind-abis/feature-gate-c-unwind.stderr b/src/test/ui/unwind-abis/feature-gate-c-unwind.stderr index f4c785a235f67..a67f46cd2e3b6 100644 --- a/src/test/ui/unwind-abis/feature-gate-c-unwind.stderr +++ b/src/test/ui/unwind-abis/feature-gate-c-unwind.stderr @@ -1,5 +1,16 @@ +warning: unknown lint: `ffi_unwind_calls` + --> $DIR/feature-gate-c-unwind.rs:4:1 + | +LL | #![allow(ffi_unwind_calls)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(unknown_lints)]` on by default + = note: the `ffi_unwind_calls` lint is unstable + = note: see issue #74990 for more information + = help: add `#![feature(c_unwind)]` to the crate attributes to enable + error[E0658]: C-unwind ABI is experimental and subject to change - --> $DIR/feature-gate-c-unwind.rs:4:8 + --> $DIR/feature-gate-c-unwind.rs:8:8 | LL | extern "C-unwind" fn f() {} | ^^^^^^^^^^ @@ -7,6 +18,16 @@ LL | extern "C-unwind" fn f() {} = note: see issue #74990 for more information = help: add `#![feature(c_unwind)]` to the crate attributes to enable -error: aborting due to previous error +warning: unknown lint: `ffi_unwind_calls` + --> $DIR/feature-gate-c-unwind.rs:4:1 + | +LL | #![allow(ffi_unwind_calls)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the `ffi_unwind_calls` lint is unstable + = note: see issue #74990 for more information + = help: add `#![feature(c_unwind)]` to the crate attributes to enable + +error: aborting due to previous error; 2 warnings emitted For more information about this error, try `rustc --explain E0658`. From 1750a2f723a796b9c98b223df6013f3b1b0254df Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Fri, 20 May 2022 21:52:00 +0100 Subject: [PATCH 04/24] Add tests for mixed panic mode restriction and lints --- .../ui/panic-runtime/auxiliary/needs-abort.rs | 5 ++++ .../panic-runtime/auxiliary/needs-unwind.rs | 13 ++++++++++ .../ui/panic-runtime/need-abort-got-unwind.rs | 9 +++++++ .../need-abort-got-unwind.stderr | 4 +++ .../ui/panic-runtime/need-unwind-got-abort.rs | 8 ++++++ .../need-unwind-got-abort.stderr | 6 +++++ .../ui/unwind-abis/ffi-unwind-calls-lint.rs | 25 +++++++++++++++++++ .../unwind-abis/ffi-unwind-calls-lint.stderr | 20 +++++++++++++++ 8 files changed, 90 insertions(+) create mode 100644 src/test/ui/panic-runtime/auxiliary/needs-abort.rs create mode 100644 src/test/ui/panic-runtime/auxiliary/needs-unwind.rs create mode 100644 src/test/ui/panic-runtime/need-abort-got-unwind.rs create mode 100644 src/test/ui/panic-runtime/need-abort-got-unwind.stderr create mode 100644 src/test/ui/panic-runtime/need-unwind-got-abort.rs create mode 100644 src/test/ui/panic-runtime/need-unwind-got-abort.stderr create mode 100644 src/test/ui/unwind-abis/ffi-unwind-calls-lint.rs create mode 100644 src/test/ui/unwind-abis/ffi-unwind-calls-lint.stderr diff --git a/src/test/ui/panic-runtime/auxiliary/needs-abort.rs b/src/test/ui/panic-runtime/auxiliary/needs-abort.rs new file mode 100644 index 0000000000000..8fad49b5e9d32 --- /dev/null +++ b/src/test/ui/panic-runtime/auxiliary/needs-abort.rs @@ -0,0 +1,5 @@ +// compile-flags:-C panic=abort +// no-prefer-dynamic + +#![crate_type = "rlib"] +#![no_std] diff --git a/src/test/ui/panic-runtime/auxiliary/needs-unwind.rs b/src/test/ui/panic-runtime/auxiliary/needs-unwind.rs new file mode 100644 index 0000000000000..d555b531986b0 --- /dev/null +++ b/src/test/ui/panic-runtime/auxiliary/needs-unwind.rs @@ -0,0 +1,13 @@ +// compile-flags:-C panic=unwind +// no-prefer-dynamic + +#![crate_type = "rlib"] +#![no_std] +#![feature(c_unwind)] + +extern "C-unwind" fn foo() {} + +fn bar() { + let ptr: extern "C-unwind" fn() = foo; + ptr(); +} diff --git a/src/test/ui/panic-runtime/need-abort-got-unwind.rs b/src/test/ui/panic-runtime/need-abort-got-unwind.rs new file mode 100644 index 0000000000000..c72fb96e357f0 --- /dev/null +++ b/src/test/ui/panic-runtime/need-abort-got-unwind.rs @@ -0,0 +1,9 @@ +// build-fail +// needs-unwind +// error-pattern:is incompatible with this crate's strategy of `unwind` +// aux-build:needs-abort.rs +// ignore-wasm32-bare compiled with panic=abort by default + +extern crate needs_abort; + +fn main() {} diff --git a/src/test/ui/panic-runtime/need-abort-got-unwind.stderr b/src/test/ui/panic-runtime/need-abort-got-unwind.stderr new file mode 100644 index 0000000000000..d29c7875fd0ff --- /dev/null +++ b/src/test/ui/panic-runtime/need-abort-got-unwind.stderr @@ -0,0 +1,4 @@ +error: the crate `needs_abort` requires panic strategy `abort` which is incompatible with this crate's strategy of `unwind` + +error: aborting due to previous error + diff --git a/src/test/ui/panic-runtime/need-unwind-got-abort.rs b/src/test/ui/panic-runtime/need-unwind-got-abort.rs new file mode 100644 index 0000000000000..3bcc0aa39ac96 --- /dev/null +++ b/src/test/ui/panic-runtime/need-unwind-got-abort.rs @@ -0,0 +1,8 @@ +// build-fail +// error-pattern:is incompatible with this crate's strategy of `abort` +// aux-build:needs-unwind.rs +// compile-flags:-C panic=abort + +extern crate needs_unwind; + +fn main() {} diff --git a/src/test/ui/panic-runtime/need-unwind-got-abort.stderr b/src/test/ui/panic-runtime/need-unwind-got-abort.stderr new file mode 100644 index 0000000000000..a53b7ffe57485 --- /dev/null +++ b/src/test/ui/panic-runtime/need-unwind-got-abort.stderr @@ -0,0 +1,6 @@ +error: the linked panic runtime `panic_unwind` is not compiled with this crate's panic strategy `abort` + +error: the crate `needs_unwind` requires panic strategy `unwind` which is incompatible with this crate's strategy of `abort` + +error: aborting due to 2 previous errors + diff --git a/src/test/ui/unwind-abis/ffi-unwind-calls-lint.rs b/src/test/ui/unwind-abis/ffi-unwind-calls-lint.rs new file mode 100644 index 0000000000000..67c20e9eac54e --- /dev/null +++ b/src/test/ui/unwind-abis/ffi-unwind-calls-lint.rs @@ -0,0 +1,25 @@ +// build-pass + +#![feature(c_unwind)] +#![warn(ffi_unwind_calls)] + +mod foo { + #[no_mangle] + pub extern "C-unwind" fn foo() {} +} + +extern "C-unwind" { + fn foo(); +} + +fn main() { + // Call to Rust function is fine. + foo::foo(); + // Call to foreign function should warn. + unsafe { foo(); } + //~^ WARNING call to foreign function with FFI-unwind ABI + let ptr: extern "C-unwind" fn() = foo::foo; + // Call to function pointer should also warn. + ptr(); + //~^ WARNING call to function pointer with FFI-unwind ABI +} diff --git a/src/test/ui/unwind-abis/ffi-unwind-calls-lint.stderr b/src/test/ui/unwind-abis/ffi-unwind-calls-lint.stderr new file mode 100644 index 0000000000000..cf8a7782e35ee --- /dev/null +++ b/src/test/ui/unwind-abis/ffi-unwind-calls-lint.stderr @@ -0,0 +1,20 @@ +warning: call to foreign function with FFI-unwind ABI + --> $DIR/ffi-unwind-calls-lint.rs:19:14 + | +LL | unsafe { foo(); } + | ^^^^^ call to foreign function with FFI-unwind ABI + | +note: the lint level is defined here + --> $DIR/ffi-unwind-calls-lint.rs:4:9 + | +LL | #![warn(ffi_unwind_calls)] + | ^^^^^^^^^^^^^^^^ + +warning: call to function pointer with FFI-unwind ABI + --> $DIR/ffi-unwind-calls-lint.rs:23:5 + | +LL | ptr(); + | ^^^^^ call to function pointer with FFI-unwind ABI + +warning: 2 warnings emitted + From 9e6c044ee6860d8b97324c75cf3dfd6f47e2488e Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Mon, 23 May 2022 21:30:38 +0100 Subject: [PATCH 05/24] Use is_fn_like instead of matching on DefKind --- compiler/rustc_mir_transform/src/abort_unwinding_calls.rs | 7 +------ compiler/rustc_mir_transform/src/ffi_unwind_calls.rs | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs b/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs index 11980382ffdb2..2c8389e532dd7 100644 --- a/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs +++ b/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs @@ -1,6 +1,5 @@ use crate::MirPass; use rustc_ast::InlineAsmOptions; -use rustc_hir::def::DefKind; use rustc_middle::mir::*; use rustc_middle::ty::layout; use rustc_middle::ty::{self, TyCtxt}; @@ -31,11 +30,7 @@ impl<'tcx> MirPass<'tcx> for AbortUnwindingCalls { // We don't simplify the MIR of constants at this time because that // namely results in a cyclic query when we call `tcx.type_of` below. - let is_function = match kind { - DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(..) => true, - _ => tcx.is_closure(def_id), - }; - if !is_function { + if !kind.is_fn_like() { return; } diff --git a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs index 78809b105359d..b8ef68f608cc3 100644 --- a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs +++ b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs @@ -1,4 +1,3 @@ -use rustc_hir::def::DefKind; use rustc_hir::def_id::{LocalDefId, LOCAL_CRATE}; use rustc_middle::mir::*; use rustc_middle::ty::layout; @@ -44,11 +43,7 @@ fn has_ffi_unwind_calls(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> bool { // Only perform check on functions because constants cannot call FFI functions. let def_id = local_def_id.to_def_id(); let kind = tcx.def_kind(def_id); - let is_function = match kind { - DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(..) => true, - _ => tcx.is_closure(def_id), - }; - if !is_function { + if !kind.is_fn_like() { return false; } From 14d155a3dc42b35856e45fd9f4212ac0c432cd10 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Wed, 8 Jun 2022 21:32:17 +0100 Subject: [PATCH 06/24] Rename `panic_strategy` query to `required_panic_strategy` --- compiler/rustc_metadata/src/creader.rs | 2 +- compiler/rustc_metadata/src/dependency_format.rs | 4 ++-- compiler/rustc_metadata/src/rmeta/decoder.rs | 4 ++-- compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs | 2 +- compiler/rustc_metadata/src/rmeta/encoder.rs | 2 +- compiler/rustc_metadata/src/rmeta/mod.rs | 2 +- compiler/rustc_middle/src/query/mod.rs | 5 +---- compiler/rustc_mir_transform/src/ffi_unwind_calls.rs | 6 ++++-- 8 files changed, 13 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 3f6d1f050056d..907324f0e4fc4 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -744,7 +744,7 @@ impl<'a> CrateLoader<'a> { if !data.is_panic_runtime() { self.sess.err(&format!("the crate `{}` is not a panic runtime", name)); } - if data.panic_strategy() != Some(desired_strategy) { + if data.required_panic_strategy() != Some(desired_strategy) { self.sess.err(&format!( "the crate `{}` does not have the panic \ strategy `{}`", diff --git a/compiler/rustc_metadata/src/dependency_format.rs b/compiler/rustc_metadata/src/dependency_format.rs index e22c903bf336e..770d164894a73 100644 --- a/compiler/rustc_metadata/src/dependency_format.rs +++ b/compiler/rustc_metadata/src/dependency_format.rs @@ -368,7 +368,7 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { } panic_runtime = Some(( cnum, - tcx.panic_strategy(cnum).unwrap_or_else(|| { + tcx.required_panic_strategy(cnum).unwrap_or_else(|| { bug!("cannot determine panic strategy of a panic runtime"); }), )); @@ -406,7 +406,7 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { continue; } - if let Some(found_strategy) = tcx.panic_strategy(cnum) && desired_strategy != found_strategy { + if let Some(found_strategy) = tcx.required_panic_strategy(cnum) && desired_strategy != found_strategy { sess.err(&format!( "the crate `{}` requires \ panic strategy `{}` which is \ diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 658c51bf62043..b8efca19fb28b 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -1759,8 +1759,8 @@ impl CrateMetadata { self.dep_kind.with_lock(|dep_kind| *dep_kind = f(*dep_kind)) } - pub(crate) fn panic_strategy(&self) -> Option { - self.root.panic_strategy + pub(crate) fn required_panic_strategy(&self) -> Option { + self.root.required_panic_strategy } pub(crate) fn needs_panic_runtime(&self) -> bool { diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index e3581a7607fb8..1237ac4ec4780 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -246,7 +246,7 @@ provide! { <'tcx> tcx, def_id, other, cdata, has_global_allocator => { cdata.root.has_global_allocator } has_panic_handler => { cdata.root.has_panic_handler } is_profiler_runtime => { cdata.root.profiler_runtime } - panic_strategy => { cdata.root.panic_strategy } + required_panic_strategy => { cdata.root.required_panic_strategy } panic_in_drop_strategy => { cdata.root.panic_in_drop_strategy } extern_crate => { let r = *cdata.extern_crate.lock(); diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index cf685a7bd6217..12cc5a72aa736 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -665,7 +665,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { triple: tcx.sess.opts.target_triple.clone(), hash: tcx.crate_hash(LOCAL_CRATE), stable_crate_id: tcx.def_path_hash(LOCAL_CRATE.as_def_id()).stable_crate_id(), - panic_strategy: tcx.required_panic_strategy(()), + required_panic_strategy: tcx.required_panic_strategy(LOCAL_CRATE), panic_in_drop_strategy: tcx.sess.opts.debugging_opts.panic_in_drop, edition: tcx.sess.edition(), has_global_allocator: tcx.has_global_allocator(LOCAL_CRATE), diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs index 60510e535b244..900d9b9842631 100644 --- a/compiler/rustc_metadata/src/rmeta/mod.rs +++ b/compiler/rustc_metadata/src/rmeta/mod.rs @@ -217,7 +217,7 @@ pub(crate) struct CrateRoot { extra_filename: String, hash: Svh, stable_crate_id: StableCrateId, - panic_strategy: Option, + required_panic_strategy: Option, panic_in_drop_strategy: PanicStrategy, edition: Edition, has_global_allocator: bool, diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 5d0d11a1b784a..bcf7497b832b4 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1369,10 +1369,7 @@ rustc_queries! { desc { |tcx| "check if `{}` contains FFI-unwind calls", tcx.def_path_str(key.to_def_id()) } cache_on_disk_if { true } } - query required_panic_strategy(_: ()) -> Option { - desc { "compute the required panic strategy for the current crate" } - } - query panic_strategy(_: CrateNum) -> Option { + query required_panic_strategy(_: CrateNum) -> Option { fatal_cycle desc { "query a crate's required panic strategy" } separate_provide_extern diff --git a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs index b8ef68f608cc3..d09d2a0b26346 100644 --- a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs +++ b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs @@ -1,4 +1,4 @@ -use rustc_hir::def_id::{LocalDefId, LOCAL_CRATE}; +use rustc_hir::def_id::{CrateNum, LocalDefId, LOCAL_CRATE}; use rustc_middle::mir::*; use rustc_middle::ty::layout; use rustc_middle::ty::query::Providers; @@ -123,7 +123,9 @@ fn has_ffi_unwind_calls(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> bool { tainted } -fn required_panic_strategy(tcx: TyCtxt<'_>, (): ()) -> Option { +fn required_panic_strategy(tcx: TyCtxt<'_>, cnum: CrateNum) -> Option { + assert_eq!(cnum, LOCAL_CRATE); + if tcx.is_panic_runtime(LOCAL_CRATE) { return Some(tcx.sess.panic_strategy()); } From ce774e377881671b968d193440d99884e84039b6 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Wed, 8 Jun 2022 22:42:05 +0100 Subject: [PATCH 07/24] Add a explanation about required panic strategy computation --- .../src/ffi_unwind_calls.rs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs index d09d2a0b26346..7728fdaffb0dc 100644 --- a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs +++ b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs @@ -136,6 +136,27 @@ fn required_panic_strategy(tcx: TyCtxt<'_>, cnum: CrateNum) -> Option Date: Sun, 19 Jun 2022 23:21:14 +0400 Subject: [PATCH 08/24] remove `span_lint_and_sugg_for_edges` from clippy utils --- .../clippy_lints/src/methods/map_flatten.rs | 11 +-- .../internal_lints/metadata_collector.rs | 1 - .../clippy/clippy_utils/src/diagnostics.rs | 91 +------------------ src/tools/clippy/tests/ui/map_flatten.stderr | 25 ++--- .../clippy/tests/ui/map_flatten_fixable.fixed | 2 - .../tests/ui/map_flatten_fixable.stderr | 54 ++--------- 6 files changed, 23 insertions(+), 161 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/methods/map_flatten.rs b/src/tools/clippy/clippy_lints/src/methods/map_flatten.rs index f447940ea3b5a..8ae84dbb3dcf4 100644 --- a/src/tools/clippy/clippy_lints/src/methods/map_flatten.rs +++ b/src/tools/clippy/clippy_lints/src/methods/map_flatten.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::span_lint_and_sugg_for_edges; +use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::is_trait_method; use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::is_type_diagnostic_item; @@ -14,17 +14,14 @@ use super::MAP_FLATTEN; pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, map_arg: &Expr<'_>, map_span: Span) { if let Some((caller_ty_name, method_to_use)) = try_get_caller_ty_name_and_method_name(cx, expr, recv, map_arg) { let mut applicability = Applicability::MachineApplicable; - let help_msgs = [ - &format!("try replacing `map` with `{}`", method_to_use), - "and remove the `.flatten()`", - ]; + let closure_snippet = snippet_with_applicability(cx, map_arg.span, "..", &mut applicability); - span_lint_and_sugg_for_edges( + span_lint_and_sugg( cx, MAP_FLATTEN, expr.span.with_lo(map_span.lo()), &format!("called `map(..).flatten()` on `{}`", caller_ty_name), - &help_msgs, + &format!("try replacing `map` with `{}` and remove the `.flatten()`", method_to_use), format!("{}({})", method_to_use, closure_snippet), applicability, ); diff --git a/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs index 99e9e3275ab53..2564099f4dbca 100644 --- a/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs +++ b/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs @@ -112,7 +112,6 @@ const LINT_EMISSION_FUNCTIONS: [&[&str]; 8] = [ &["clippy_utils", "diagnostics", "span_lint_and_sugg"], &["clippy_utils", "diagnostics", "span_lint_and_then"], &["clippy_utils", "diagnostics", "span_lint_hir_and_then"], - &["clippy_utils", "diagnostics", "span_lint_and_sugg_for_edges"], ]; const SUGGESTION_DIAGNOSTIC_BUILDER_METHODS: [(&str, bool); 9] = [ ("span_suggestion", false), diff --git a/src/tools/clippy/clippy_utils/src/diagnostics.rs b/src/tools/clippy/clippy_utils/src/diagnostics.rs index 39595f589c70b..7f55db3b31f70 100644 --- a/src/tools/clippy/clippy_utils/src/diagnostics.rs +++ b/src/tools/clippy/clippy_utils/src/diagnostics.rs @@ -8,7 +8,7 @@ //! Thank you! //! ~The `INTERNAL_METADATA_COLLECTOR` lint -use rustc_errors::{emitter::MAX_SUGGESTION_HIGHLIGHT_LINES, Applicability, Diagnostic, MultiSpan}; +use rustc_errors::{Applicability, Diagnostic, MultiSpan}; use rustc_hir::HirId; use rustc_lint::{LateContext, Lint, LintContext}; use rustc_span::source_map::Span; @@ -219,95 +219,6 @@ pub fn span_lint_and_sugg<'a, T: LintContext>( }); } -/// Like [`span_lint_and_sugg`] with a focus on the edges. The output will either -/// emit single span or multispan suggestion depending on the number of its lines. -/// -/// If the given suggestion string has more lines than the maximum display length defined by -/// [`MAX_SUGGESTION_HIGHLIGHT_LINES`][`rustc_errors::emitter::MAX_SUGGESTION_HIGHLIGHT_LINES`], -/// this function will split the suggestion and span to showcase the change for the top and -/// bottom edge of the code. For normal suggestions, in one display window, the help message -/// will be combined with a colon. -/// -/// Multipart suggestions like the one being created here currently cannot be -/// applied by rustfix (See [rustfix#141](https://github.com/rust-lang/rustfix/issues/141)). -/// Testing rustfix with this lint emission function might require a file with -/// suggestions that can be fixed and those that can't. See -/// [clippy#8520](https://github.com/rust-lang/rust-clippy/pull/8520/files) for -/// an example and of this. -/// -/// # Example for a long suggestion -/// -/// ```text -/// error: called `map(..).flatten()` on `Option` -/// --> $DIR/map_flatten.rs:8:10 -/// | -/// LL | .map(|x| { -/// | __________^ -/// LL | | if x <= 5 { -/// LL | | Some(x) -/// LL | | } else { -/// ... | -/// LL | | }) -/// LL | | .flatten(); -/// | |__________________^ -/// | -/// = note: `-D clippy::map-flatten` implied by `-D warnings` -/// help: try replacing `map` with `and_then` -/// | -/// LL ~ .and_then(|x| { -/// LL + if x <= 5 { -/// LL + Some(x) -/// | -/// help: and remove the `.flatten()` -/// | -/// LL + None -/// LL + } -/// LL ~ }); -/// | -/// ``` -pub fn span_lint_and_sugg_for_edges( - cx: &LateContext<'_>, - lint: &'static Lint, - sp: Span, - msg: &str, - helps: &[&str; 2], - sugg: String, - applicability: Applicability, -) { - span_lint_and_then(cx, lint, sp, msg, |diag| { - let sugg_lines_count = sugg.lines().count(); - if sugg_lines_count > MAX_SUGGESTION_HIGHLIGHT_LINES { - let sm = cx.sess().source_map(); - if let (Ok(line_upper), Ok(line_bottom)) = - (sm.lookup_line(sp.lo()), sm.lookup_line(sp.hi())) - { - let split_idx = MAX_SUGGESTION_HIGHLIGHT_LINES / 2; - let span_upper = sm.span_until_char( - sp.with_hi(line_upper.sf.lines(|lines| lines[line_upper.line + split_idx])), - '\n', - ); - let span_bottom = sp.with_lo(line_bottom.sf.lines(|lines| lines[line_bottom.line - split_idx])); - - let sugg_lines_vec = sugg.lines().collect::>(); - let sugg_upper = sugg_lines_vec[..split_idx].join("\n"); - let sugg_bottom = sugg_lines_vec[sugg_lines_count - split_idx..].join("\n"); - - diag.span_suggestion(span_upper, helps[0], sugg_upper, applicability); - diag.span_suggestion(span_bottom, helps[1], sugg_bottom, applicability); - - return; - } - } - diag.span_suggestion_with_style( - sp, - &helps.join(", "), - sugg, - applicability, - rustc_errors::SuggestionStyle::ShowAlways, - ); - }); -} - /// Create a suggestion made from several `span → replacement`. /// /// Note: in the JSON format (used by `compiletest_rs`), the help message will diff --git a/src/tools/clippy/tests/ui/map_flatten.stderr b/src/tools/clippy/tests/ui/map_flatten.stderr index c9c60df838f67..4b2630d685847 100644 --- a/src/tools/clippy/tests/ui/map_flatten.stderr +++ b/src/tools/clippy/tests/ui/map_flatten.stderr @@ -12,14 +12,12 @@ LL | | .flatten(); | |__________________^ | = note: `-D clippy::map-flatten` implied by `-D warnings` -help: try replacing `map` with `and_then` +help: try replacing `map` with `and_then` and remove the `.flatten()` | LL ~ .and_then(|x| { LL + if x <= 5 { LL + Some(x) - | -help: and remove the `.flatten()` - | +LL + } else { LL + None LL + } LL ~ }); @@ -38,14 +36,12 @@ LL | | }) LL | | .flatten(); | |__________________^ | -help: try replacing `map` with `and_then` +help: try replacing `map` with `and_then` and remove the `.flatten()` | LL ~ .and_then(|x| { LL + if x == 1 { LL + Ok(x) - | -help: and remove the `.flatten()` - | +LL + } else { LL + Err(0) LL + } LL ~ }); @@ -64,14 +60,13 @@ LL | | }) LL | | .flatten(); | |__________________^ | -help: try replacing `map` with `and_then` +help: try replacing `map` with `and_then` and remove the `.flatten()` | LL ~ .and_then(|res| { LL + if res > 0 { LL + do_something(); - | -help: and remove the `.flatten()` - | +LL + Ok(res) +LL + } else { LL + Err(0) LL + } LL ~ }); @@ -90,14 +85,12 @@ LL | | }) LL | | .flatten() | |__________________^ | -help: try replacing `map` with `filter_map` +help: try replacing `map` with `filter_map` and remove the `.flatten()` | LL ~ .filter_map(|some_value| { LL + if some_value > 3 { LL + Some(some_value) - | -help: and remove the `.flatten()` - | +LL + } else { LL + None LL + } LL + }) diff --git a/src/tools/clippy/tests/ui/map_flatten_fixable.fixed b/src/tools/clippy/tests/ui/map_flatten_fixable.fixed index 928e5bd509c3f..e9b41354c58fa 100644 --- a/src/tools/clippy/tests/ui/map_flatten_fixable.fixed +++ b/src/tools/clippy/tests/ui/map_flatten_fixable.fixed @@ -59,8 +59,6 @@ fn issue8878() { .and_then(|_| { // we need some newlines // so that the span is big enough -// we need some newlines -// so that the span is big enough // for a splitted output of the diagnostic Some("") // whitespace beforehand is important as well diff --git a/src/tools/clippy/tests/ui/map_flatten_fixable.stderr b/src/tools/clippy/tests/ui/map_flatten_fixable.stderr index 828e24acaad6c..f3b82ad08d0fc 100644 --- a/src/tools/clippy/tests/ui/map_flatten_fixable.stderr +++ b/src/tools/clippy/tests/ui/map_flatten_fixable.stderr @@ -2,79 +2,45 @@ error: called `map(..).flatten()` on `Iterator` --> $DIR/map_flatten_fixable.rs:18:47 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().collect(); - | ^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(option_id)` | = note: `-D clippy::map-flatten` implied by `-D warnings` -help: try replacing `map` with `filter_map`, and remove the `.flatten()` - | -LL | let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(option_id).collect(); - | ~~~~~~~~~~~~~~~~~~~~~ error: called `map(..).flatten()` on `Iterator` --> $DIR/map_flatten_fixable.rs:19:47 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_ref).flatten().collect(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: try replacing `map` with `filter_map`, and remove the `.flatten()` - | -LL | let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(option_id_ref).collect(); - | ~~~~~~~~~~~~~~~~~~~~~~~~~ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(option_id_ref)` error: called `map(..).flatten()` on `Iterator` --> $DIR/map_flatten_fixable.rs:20:47 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_closure).flatten().collect(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: try replacing `map` with `filter_map`, and remove the `.flatten()` - | -LL | let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(option_id_closure).collect(); - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(option_id_closure)` error: called `map(..).flatten()` on `Iterator` --> $DIR/map_flatten_fixable.rs:21:47 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: try replacing `map` with `filter_map`, and remove the `.flatten()` - | -LL | let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(|x| x.checked_add(1)).collect(); - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(|x| x.checked_add(1))` error: called `map(..).flatten()` on `Iterator` --> $DIR/map_flatten_fixable.rs:24:47 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect(); - | ^^^^^^^^^^^^^^^^^^^^^^^ - | -help: try replacing `map` with `flat_map`, and remove the `.flatten()` - | -LL | let _: Vec<_> = vec![5_i8; 6].into_iter().flat_map(|x| 0..x).collect(); - | ~~~~~~~~~~~~~~~~~~ + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `flat_map` and remove the `.flatten()`: `flat_map(|x| 0..x)` error: called `map(..).flatten()` on `Option` --> $DIR/map_flatten_fixable.rs:27:40 | LL | let _: Option<_> = (Some(Some(1))).map(|x| x).flatten(); - | ^^^^^^^^^^^^^^^^^^^^ - | -help: try replacing `map` with `and_then`, and remove the `.flatten()` - | -LL | let _: Option<_> = (Some(Some(1))).and_then(|x| x); - | ~~~~~~~~~~~~~~~ + | ^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `and_then` and remove the `.flatten()`: `and_then(|x| x)` error: called `map(..).flatten()` on `Result` --> $DIR/map_flatten_fixable.rs:30:42 | LL | let _: Result<_, &str> = (Ok(Ok(1))).map(|x| x).flatten(); - | ^^^^^^^^^^^^^^^^^^^^ - | -help: try replacing `map` with `and_then`, and remove the `.flatten()` - | -LL | let _: Result<_, &str> = (Ok(Ok(1))).and_then(|x| x); - | ~~~~~~~~~~~~~~~ + | ^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `and_then` and remove the `.flatten()`: `and_then(|x| x)` error: called `map(..).flatten()` on `Option` --> $DIR/map_flatten_fixable.rs:59:10 @@ -89,14 +55,12 @@ LL | | }) LL | | .flatten(); | |__________________^ | -help: try replacing `map` with `and_then` +help: try replacing `map` with `and_then` and remove the `.flatten()` | LL ~ .and_then(|_| { LL + // we need some newlines LL + // so that the span is big enough - | -help: and remove the `.flatten()` - | +LL + // for a splitted output of the diagnostic LL + Some("") LL + // whitespace beforehand is important as well LL ~ }); From e2512477a9faf2a86ddeba9c8538c84d9c948384 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Mon, 20 Jun 2022 00:25:07 +0400 Subject: [PATCH 09/24] remove last use of MAX_SUGGESTION_HIGHLIGHT_LINES --- .../clippy_lints/src/methods/or_fun_call.rs | 13 ++---- src/tools/clippy/tests/ui/or_fun_call.fixed | 6 +-- src/tools/clippy/tests/ui/or_fun_call.stderr | 46 ++++++++++++++++--- 3 files changed, 45 insertions(+), 20 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs b/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs index 448dc4e6147ff..3d1208824fa34 100644 --- a/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs +++ b/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs @@ -4,7 +4,6 @@ use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_mac use clippy_utils::ty::{implements_trait, match_type}; use clippy_utils::{contains_return, is_trait_item, last_path_segment, paths}; use if_chain::if_chain; -use rustc_errors::emitter::MAX_SUGGESTION_HIGHLIGHT_LINES; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; @@ -33,7 +32,6 @@ pub(super) fn check<'tcx>( arg: &hir::Expr<'_>, or_has_args: bool, span: Span, - method_span: Span, ) -> bool { let is_default_default = || is_trait_item(cx, fun, sym::Default); @@ -56,19 +54,14 @@ pub(super) fn check<'tcx>( then { let mut applicability = Applicability::MachineApplicable; let hint = "unwrap_or_default()"; - let mut sugg_span = span; + let sugg_span = span; - let mut sugg: String = format!( + let sugg: String = format!( "{}.{}", snippet_with_applicability(cx, self_expr.span, "..", &mut applicability), hint ); - if sugg.lines().count() > MAX_SUGGESTION_HIGHLIGHT_LINES { - sugg_span = method_span.with_hi(span.hi()); - sugg = hint.to_string(); - } - span_lint_and_sugg( cx, OR_FUN_CALL, @@ -178,7 +171,7 @@ pub(super) fn check<'tcx>( match inner_arg.kind { hir::ExprKind::Call(fun, or_args) => { let or_has_args = !or_args.is_empty(); - if !check_unwrap_or_default(cx, name, fun, self_arg, arg, or_has_args, expr.span, method_span) { + if !check_unwrap_or_default(cx, name, fun, self_arg, arg, or_has_args, expr.span) { let fun_span = if or_has_args { None } else { Some(fun.span) }; check_general_case(cx, name, method_span, self_arg, arg, expr.span, fun_span); } diff --git a/src/tools/clippy/tests/ui/or_fun_call.fixed b/src/tools/clippy/tests/ui/or_fun_call.fixed index 3208048e0d53c..123aed40251e2 100644 --- a/src/tools/clippy/tests/ui/or_fun_call.fixed +++ b/src/tools/clippy/tests/ui/or_fun_call.fixed @@ -185,8 +185,7 @@ mod issue8239 { .reduce(|mut acc, f| { acc.push_str(&f); acc - }) - .unwrap_or_default(); + }).unwrap_or_default(); } fn more_to_max_suggestion_highest_lines_1() { @@ -198,8 +197,7 @@ mod issue8239 { let _ = ""; acc.push_str(&f); acc - }) - .unwrap_or_default(); + }).unwrap_or_default(); } fn equal_to_max_suggestion_highest_lines() { diff --git a/src/tools/clippy/tests/ui/or_fun_call.stderr b/src/tools/clippy/tests/ui/or_fun_call.stderr index 549b00ae3c459..dfe15654bc32c 100644 --- a/src/tools/clippy/tests/ui/or_fun_call.stderr +++ b/src/tools/clippy/tests/ui/or_fun_call.stderr @@ -109,16 +109,50 @@ LL | None.unwrap_or( unsafe { ptr_to_ref(s) } ); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| unsafe { ptr_to_ref(s) })` error: use of `unwrap_or` followed by a call to `new` - --> $DIR/or_fun_call.rs:189:14 + --> $DIR/or_fun_call.rs:182:9 + | +LL | / frames +LL | | .iter() +LL | | .map(|f: &String| f.to_lowercase()) +LL | | .reduce(|mut acc, f| { +... | +LL | | }) +LL | | .unwrap_or(String::new()); + | |_____________________________________^ + | +help: try this + | +LL ~ frames +LL + .iter() +LL + .map(|f: &String| f.to_lowercase()) +LL + .reduce(|mut acc, f| { +LL + acc.push_str(&f); +LL + acc +LL ~ }).unwrap_or_default(); | -LL | .unwrap_or(String::new()); - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()` error: use of `unwrap_or` followed by a call to `new` - --> $DIR/or_fun_call.rs:202:14 + --> $DIR/or_fun_call.rs:195:9 + | +LL | / iter.map(|f: &String| f.to_lowercase()) +LL | | .reduce(|mut acc, f| { +LL | | let _ = ""; +LL | | let _ = ""; +... | +LL | | }) +LL | | .unwrap_or(String::new()); + | |_____________________________________^ + | +help: try this + | +LL ~ iter.map(|f: &String| f.to_lowercase()) +LL + .reduce(|mut acc, f| { +LL + let _ = ""; +LL + let _ = ""; +LL + acc.push_str(&f); +LL + acc +LL ~ }).unwrap_or_default(); | -LL | .unwrap_or(String::new()); - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()` error: use of `unwrap_or` followed by a call to `new` --> $DIR/or_fun_call.rs:208:9 From ffb593bf4d0dfea864f8e47dfd30098e70b8e5f8 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Mon, 20 Jun 2022 00:25:51 +0400 Subject: [PATCH 10/24] remove MAX_SUGGESTION_HIGHLIGHT_LINES --- compiler/rustc_errors/src/emitter.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index a4cbc73978d5b..8b2a995f1c58e 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -656,11 +656,6 @@ impl Emitter for SilentEmitter { } } -/// Maximum number of lines we will print for a multiline suggestion; arbitrary. -/// -/// This should be replaced with a more involved mechanism to output multiline suggestions that -/// more closely mimics the regular diagnostic output, where irrelevant code lines are elided. -pub const MAX_SUGGESTION_HIGHLIGHT_LINES: usize = 6; /// Maximum number of suggestions to be shown /// /// Arbitrary, but taken from trait import suggestion limit From a0eba6634f1430e29637257fd1416f896bcf9edc Mon Sep 17 00:00:00 2001 From: Caio Date: Tue, 21 Jun 2022 10:56:26 -0300 Subject: [PATCH 11/24] [RFC 2011] Optimize non-consuming operators --- .../src/assert/context.rs | 96 ++++++++++-- .../rfc-2011-nicer-assert-messages/codegen.rs | 9 -- .../codegen.stdout | 29 ---- ...onsuming-methods-have-optimized-codegen.rs | 32 ++++ ...ming-methods-have-optimized-codegen.stdout | 147 ++++++++++++++++++ 5 files changed, 260 insertions(+), 53 deletions(-) delete mode 100644 src/test/ui/macros/rfc-2011-nicer-assert-messages/codegen.rs delete mode 100644 src/test/ui/macros/rfc-2011-nicer-assert-messages/codegen.stdout create mode 100644 src/test/ui/macros/rfc-2011-nicer-assert-messages/non-consuming-methods-have-optimized-codegen.rs create mode 100644 src/test/ui/macros/rfc-2011-nicer-assert-messages/non-consuming-methods-have-optimized-codegen.stdout diff --git a/compiler/rustc_builtin_macros/src/assert/context.rs b/compiler/rustc_builtin_macros/src/assert/context.rs index cad301812123b..9e50d33486cf2 100644 --- a/compiler/rustc_builtin_macros/src/assert/context.rs +++ b/compiler/rustc_builtin_macros/src/assert/context.rs @@ -1,11 +1,10 @@ -use crate::assert::expr_if_not; use rustc_ast::{ attr, ptr::P, token, tokenstream::{DelimSpan, TokenStream, TokenTree}, - BorrowKind, Expr, ExprKind, ItemKind, MacArgs, MacCall, MacDelimiter, Mutability, Path, - PathSegment, Stmt, StructRest, UseTree, UseTreeKind, DUMMY_NODE_ID, + BinOpKind, BorrowKind, Expr, ExprKind, ItemKind, MacArgs, MacCall, MacDelimiter, Mutability, + Path, PathSegment, Stmt, StructRest, UnOp, UseTree, UseTreeKind, DUMMY_NODE_ID, }; use rustc_ast_pretty::pprust; use rustc_data_structures::fx::FxHashSet; @@ -16,11 +15,19 @@ use rustc_span::{ }; pub(super) struct Context<'cx, 'a> { + // An optimization. + // + // Elements that aren't consumed (PartialEq, PartialOrd, ...) can be copied **after** the + // `assert!` expression fails rather than copied on-the-fly. + best_case_captures: Vec, // Top-level `let captureN = Capture::new()` statements capture_decls: Vec, cx: &'cx ExtCtxt<'a>, // Formatting string used for debugging fmt_string: String, + // If the current expression being visited consumes itself. Used to construct + // `best_case_captures`. + is_consumed: bool, // Top-level `let __local_bindN = &expr` statements local_bind_decls: Vec, // Used to avoid capturing duplicated paths @@ -36,9 +43,11 @@ pub(super) struct Context<'cx, 'a> { impl<'cx, 'a> Context<'cx, 'a> { pub(super) fn new(cx: &'cx ExtCtxt<'a>, span: Span) -> Self { Self { + best_case_captures: <_>::default(), capture_decls: <_>::default(), cx, fmt_string: <_>::default(), + is_consumed: true, local_bind_decls: <_>::default(), paths: <_>::default(), span, @@ -69,14 +78,22 @@ impl<'cx, 'a> Context<'cx, 'a> { self.manage_cond_expr(&mut cond_expr); let initial_imports = self.build_initial_imports(); let panic = self.build_panic(&expr_str, panic_path); + let cond_expr_with_unlikely = self.build_unlikely(cond_expr); + + let Self { best_case_captures, capture_decls, cx, local_bind_decls, span, .. } = self; - let Self { capture_decls, cx, local_bind_decls, span, .. } = self; + let mut assert_then_stmts = Vec::with_capacity(2); + assert_then_stmts.extend(best_case_captures); + assert_then_stmts.push(self.cx.stmt_expr(panic)); + let assert_then = self.cx.block(span, assert_then_stmts); let mut stmts = Vec::with_capacity(4); stmts.push(initial_imports); stmts.extend(capture_decls.into_iter().map(|c| c.decl)); stmts.extend(local_bind_decls); - stmts.push(cx.stmt_expr(expr_if_not(cx, span, cond_expr, panic, None))); + stmts.push( + cx.stmt_expr(cx.expr(span, ExprKind::If(cond_expr_with_unlikely, assert_then, None))), + ); cx.expr_block(cx.block(span, stmts)) } @@ -115,6 +132,16 @@ impl<'cx, 'a> Context<'cx, 'a> { ) } + /// Takes the conditional expression of `assert!` and then wraps it inside `unlikely` + fn build_unlikely(&self, cond_expr: P) -> P { + let unlikely_path = self.cx.std_path(&[sym::intrinsics, sym::unlikely]); + self.cx.expr_call( + self.span, + self.cx.expr_path(self.cx.path(self.span, unlikely_path)), + vec![self.cx.expr(self.span, ExprKind::Unary(UnOp::Not, cond_expr))], + ) + } + /// The necessary custom `panic!(...)` expression. /// /// panic!( @@ -167,17 +194,39 @@ impl<'cx, 'a> Context<'cx, 'a> { /// See [Self::manage_initial_capture] and [Self::manage_try_capture] fn manage_cond_expr(&mut self, expr: &mut P) { match (*expr).kind { - ExprKind::AddrOf(_, _, ref mut local_expr) => { - self.manage_cond_expr(local_expr); + ExprKind::AddrOf(_, mutability, ref mut local_expr) => { + self.with_is_consumed_management( + matches!(mutability, Mutability::Mut), + |this| this.manage_cond_expr(local_expr) + ); } ExprKind::Array(ref mut local_exprs) => { for local_expr in local_exprs { self.manage_cond_expr(local_expr); } } - ExprKind::Binary(_, ref mut lhs, ref mut rhs) => { - self.manage_cond_expr(lhs); - self.manage_cond_expr(rhs); + ExprKind::Binary(ref op, ref mut lhs, ref mut rhs) => { + self.with_is_consumed_management( + matches!( + op.node, + BinOpKind::Add + | BinOpKind::And + | BinOpKind::BitAnd + | BinOpKind::BitOr + | BinOpKind::BitXor + | BinOpKind::Div + | BinOpKind::Mul + | BinOpKind::Or + | BinOpKind::Rem + | BinOpKind::Shl + | BinOpKind::Shr + | BinOpKind::Sub + ), + |this| { + this.manage_cond_expr(lhs); + this.manage_cond_expr(rhs); + } + ); } ExprKind::Call(_, ref mut local_exprs) => { for local_expr in local_exprs { @@ -228,8 +277,11 @@ impl<'cx, 'a> Context<'cx, 'a> { self.manage_cond_expr(local_expr); } } - ExprKind::Unary(_, ref mut local_expr) => { - self.manage_cond_expr(local_expr); + ExprKind::Unary(un_op, ref mut local_expr) => { + self.with_is_consumed_management( + matches!(un_op, UnOp::Neg | UnOp::Not), + |this| this.manage_cond_expr(local_expr) + ); } // Expressions that are not worth or can not be captured. // @@ -337,9 +389,23 @@ impl<'cx, 'a> Context<'cx, 'a> { )) .add_trailing_semicolon(); let local_bind_path = self.cx.expr_path(Path::from_ident(local_bind)); - let ret = self.cx.stmt_expr(local_bind_path); - let block = self.cx.expr_block(self.cx.block(self.span, vec![try_capture_call, ret])); - *expr = self.cx.expr_deref(self.span, block); + let rslt = if self.is_consumed { + let ret = self.cx.stmt_expr(local_bind_path); + self.cx.expr_block(self.cx.block(self.span, vec![try_capture_call, ret])) + } else { + self.best_case_captures.push(try_capture_call); + local_bind_path + }; + *expr = self.cx.expr_deref(self.span, rslt); + } + + // Calls `f` with the internal `is_consumed` set to `curr_is_consumed` and then + // sets the internal `is_consumed` back to its original value. + fn with_is_consumed_management(&mut self, curr_is_consumed: bool, f: impl FnOnce(&mut Self)) { + let prev_is_consumed = self.is_consumed; + self.is_consumed = curr_is_consumed; + f(self); + self.is_consumed = prev_is_consumed; } } diff --git a/src/test/ui/macros/rfc-2011-nicer-assert-messages/codegen.rs b/src/test/ui/macros/rfc-2011-nicer-assert-messages/codegen.rs deleted file mode 100644 index 1db9d33c72aee..0000000000000 --- a/src/test/ui/macros/rfc-2011-nicer-assert-messages/codegen.rs +++ /dev/null @@ -1,9 +0,0 @@ -// check-pass -// compile-flags: -Z unpretty=expanded - -#![feature(core_intrinsics, generic_assert, generic_assert_internals)] - -fn main() { - let elem = 1i32; - assert!(elem == 1); -} diff --git a/src/test/ui/macros/rfc-2011-nicer-assert-messages/codegen.stdout b/src/test/ui/macros/rfc-2011-nicer-assert-messages/codegen.stdout deleted file mode 100644 index a590eb3223254..0000000000000 --- a/src/test/ui/macros/rfc-2011-nicer-assert-messages/codegen.stdout +++ /dev/null @@ -1,29 +0,0 @@ -#![feature(prelude_import)] -#![no_std] -// check-pass -// compile-flags: -Z unpretty=expanded - -#![feature(core_intrinsics, generic_assert, generic_assert_internals)] -#[prelude_import] -use ::std::prelude::rust_2015::*; -#[macro_use] -extern crate std; - -fn main() { - let elem = 1i32; - { - #[allow(unused_imports)] - use ::core::asserting::{TryCaptureGeneric, TryCapturePrintable}; - let mut __capture0 = ::core::asserting::Capture::new(); - let __local_bind0 = &elem; - if !(*{ - (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0); - __local_bind0 - } == 1) { - { - ::std::rt::panic_fmt(::core::fmt::Arguments::new_v1(&["Assertion failed: elem == 1\nWith captures:\n elem = ", - "\n"], &[::core::fmt::ArgumentV1::new_debug(&__capture0)])) - } - } - }; -} diff --git a/src/test/ui/macros/rfc-2011-nicer-assert-messages/non-consuming-methods-have-optimized-codegen.rs b/src/test/ui/macros/rfc-2011-nicer-assert-messages/non-consuming-methods-have-optimized-codegen.rs new file mode 100644 index 0000000000000..5ec84b08ff808 --- /dev/null +++ b/src/test/ui/macros/rfc-2011-nicer-assert-messages/non-consuming-methods-have-optimized-codegen.rs @@ -0,0 +1,32 @@ +// check-pass +// compile-flags: -Z unpretty=expanded + +#![feature(core_intrinsics, generic_assert, generic_assert_internals)] + +fn arbitrary_consuming_method_for_demonstration_purposes() { + let elem = 1i32; + assert!(elem as usize); +} + +fn addr_of() { + let elem = 1i32; + assert!(&elem); +} + +fn binary() { + let elem = 1i32; + assert!(elem == 1); + assert!(elem >= 1); + assert!(elem > 0); + assert!(elem < 3); + assert!(elem <= 3); + assert!(elem != 3); +} + +fn unary() { + let elem = &1i32; + assert!(*elem); +} + +fn main() { +} diff --git a/src/test/ui/macros/rfc-2011-nicer-assert-messages/non-consuming-methods-have-optimized-codegen.stdout b/src/test/ui/macros/rfc-2011-nicer-assert-messages/non-consuming-methods-have-optimized-codegen.stdout new file mode 100644 index 0000000000000..90f858f80e6b5 --- /dev/null +++ b/src/test/ui/macros/rfc-2011-nicer-assert-messages/non-consuming-methods-have-optimized-codegen.stdout @@ -0,0 +1,147 @@ +#![feature(prelude_import)] +#![no_std] +// check-pass +// compile-flags: -Z unpretty=expanded + +#![feature(core_intrinsics, generic_assert, generic_assert_internals)] +#[prelude_import] +use ::std::prelude::rust_2015::*; +#[macro_use] +extern crate std; + +fn arbitrary_consuming_method_for_demonstration_purposes() { + let elem = 1i32; + { + #[allow(unused_imports)] + use ::core::asserting::{TryCaptureGeneric, TryCapturePrintable}; + let mut __capture0 = ::core::asserting::Capture::new(); + let __local_bind0 = &elem; + if ::core::intrinsics::unlikely(!(*{ + (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0); + __local_bind0 + } as usize)) { + + + + + { + ::std::rt::panic_fmt(::core::fmt::Arguments::new_v1(&["Assertion failed: elem as usize\nWith captures:\n elem = ", + "\n"], &[::core::fmt::ArgumentV1::new_debug(&__capture0)])) + } + } + }; +} +fn addr_of() { + let elem = 1i32; + { + #[allow(unused_imports)] + use ::core::asserting::{TryCaptureGeneric, TryCapturePrintable}; + let mut __capture0 = ::core::asserting::Capture::new(); + let __local_bind0 = &elem; + if ::core::intrinsics::unlikely(!&*__local_bind0) { + (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0); + { + ::std::rt::panic_fmt(::core::fmt::Arguments::new_v1(&["Assertion failed: &elem\nWith captures:\n elem = ", + "\n"], &[::core::fmt::ArgumentV1::new_debug(&__capture0)])) + } + } + }; +} +fn binary() { + let elem = 1i32; + { + #[allow(unused_imports)] + use ::core::asserting::{TryCaptureGeneric, TryCapturePrintable}; + let mut __capture0 = ::core::asserting::Capture::new(); + let __local_bind0 = &elem; + if ::core::intrinsics::unlikely(!(*__local_bind0 == 1)) { + (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0); + { + ::std::rt::panic_fmt(::core::fmt::Arguments::new_v1(&["Assertion failed: elem == 1\nWith captures:\n elem = ", + "\n"], &[::core::fmt::ArgumentV1::new_debug(&__capture0)])) + } + } + }; + { + #[allow(unused_imports)] + use ::core::asserting::{TryCaptureGeneric, TryCapturePrintable}; + let mut __capture0 = ::core::asserting::Capture::new(); + let __local_bind0 = &elem; + if ::core::intrinsics::unlikely(!(*__local_bind0 >= 1)) { + (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0); + { + ::std::rt::panic_fmt(::core::fmt::Arguments::new_v1(&["Assertion failed: elem >= 1\nWith captures:\n elem = ", + "\n"], &[::core::fmt::ArgumentV1::new_debug(&__capture0)])) + } + } + }; + { + #[allow(unused_imports)] + use ::core::asserting::{TryCaptureGeneric, TryCapturePrintable}; + let mut __capture0 = ::core::asserting::Capture::new(); + let __local_bind0 = &elem; + if ::core::intrinsics::unlikely(!(*__local_bind0 > 0)) { + (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0); + { + ::std::rt::panic_fmt(::core::fmt::Arguments::new_v1(&["Assertion failed: elem > 0\nWith captures:\n elem = ", + "\n"], &[::core::fmt::ArgumentV1::new_debug(&__capture0)])) + } + } + }; + { + #[allow(unused_imports)] + use ::core::asserting::{TryCaptureGeneric, TryCapturePrintable}; + let mut __capture0 = ::core::asserting::Capture::new(); + let __local_bind0 = &elem; + if ::core::intrinsics::unlikely(!(*__local_bind0 < 3)) { + (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0); + { + ::std::rt::panic_fmt(::core::fmt::Arguments::new_v1(&["Assertion failed: elem < 3\nWith captures:\n elem = ", + "\n"], &[::core::fmt::ArgumentV1::new_debug(&__capture0)])) + } + } + }; + { + #[allow(unused_imports)] + use ::core::asserting::{TryCaptureGeneric, TryCapturePrintable}; + let mut __capture0 = ::core::asserting::Capture::new(); + let __local_bind0 = &elem; + if ::core::intrinsics::unlikely(!(*__local_bind0 <= 3)) { + (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0); + { + ::std::rt::panic_fmt(::core::fmt::Arguments::new_v1(&["Assertion failed: elem <= 3\nWith captures:\n elem = ", + "\n"], &[::core::fmt::ArgumentV1::new_debug(&__capture0)])) + } + } + }; + { + #[allow(unused_imports)] + use ::core::asserting::{TryCaptureGeneric, TryCapturePrintable}; + let mut __capture0 = ::core::asserting::Capture::new(); + let __local_bind0 = &elem; + if ::core::intrinsics::unlikely(!(*__local_bind0 != 3)) { + (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0); + { + ::std::rt::panic_fmt(::core::fmt::Arguments::new_v1(&["Assertion failed: elem != 3\nWith captures:\n elem = ", + "\n"], &[::core::fmt::ArgumentV1::new_debug(&__capture0)])) + } + } + }; +} +fn unary() { + let elem = &1i32; + { + #[allow(unused_imports)] + use ::core::asserting::{TryCaptureGeneric, TryCapturePrintable}; + let mut __capture0 = ::core::asserting::Capture::new(); + let __local_bind0 = &elem; + if ::core::intrinsics::unlikely(!**__local_bind0) { + (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0); + { + ::std::rt::panic_fmt(::core::fmt::Arguments::new_v1(&["Assertion failed: *elem\nWith captures:\n elem = ", + "\n"], &[::core::fmt::ArgumentV1::new_debug(&__capture0)])) + } + } + }; +} +fn main() {} From c41630735c00187d54ae30ff0a7a98c1d689b831 Mon Sep 17 00:00:00 2001 From: Rida Dzhaafar <38199959+rdzhaafar@users.noreply.github.com> Date: Wed, 22 Jun 2022 15:24:54 +0300 Subject: [PATCH 12/24] Fixed RSS reporting on macOS --- .../rustc_data_structures/src/profiling.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/compiler/rustc_data_structures/src/profiling.rs b/compiler/rustc_data_structures/src/profiling.rs index 88ff33b4d09a1..d8b26f9840b63 100644 --- a/compiler/rustc_data_structures/src/profiling.rs +++ b/compiler/rustc_data_structures/src/profiling.rs @@ -826,6 +826,24 @@ cfg_if! { } } } + } else if #[cfg(target_os = "macos")] { + pub fn get_resident_set_size() -> Option { + use libc::{c_int, c_void, getpid, proc_pidinfo, proc_taskinfo, PROC_PIDTASKINFO}; + use std::mem; + const PROC_TASKINFO_SIZE: c_int = mem::size_of::() as c_int; + + unsafe { + let mut info: proc_taskinfo = mem::zeroed(); + let info_ptr = &mut info as *mut proc_taskinfo as *mut c_void; + let pid = getpid() as c_int; + let ret = proc_pidinfo(pid, PROC_PIDTASKINFO, 0, info_ptr, PROC_TASKINFO_SIZE); + if ret == PROC_TASKINFO_SIZE { + Some(info.pti_resident_size as usize) + } else { + None + } + } + } } else if #[cfg(unix)] { pub fn get_resident_set_size() -> Option { let field = 1; From f954f7b23b8564ecd0bbeb015d0d4da7e4c044eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Sun, 26 Jun 2022 22:52:19 +0200 Subject: [PATCH 13/24] Hermit: Fix initializing lazy locks --- library/std/src/sys/hermit/condvar.rs | 19 ++++++++++++++++--- library/std/src/sys/hermit/rwlock.rs | 11 ++++++++--- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/library/std/src/sys/hermit/condvar.rs b/library/std/src/sys/hermit/condvar.rs index 3f00160e55a25..22059ca0dbe10 100644 --- a/library/std/src/sys/hermit/condvar.rs +++ b/library/std/src/sys/hermit/condvar.rs @@ -3,6 +3,7 @@ use crate::ptr; use crate::sync::atomic::{AtomicUsize, Ordering::SeqCst}; use crate::sys::hermit::abi; use crate::sys::locks::Mutex; +use crate::sys_common::lazy_box::{LazyBox, LazyInit}; use crate::time::Duration; // The implementation is inspired by Andrew D. Birrell's paper @@ -14,14 +15,26 @@ pub struct Condvar { sem2: *const c_void, } -pub type MovableCondvar = Condvar; +pub(crate) type MovableCondvar = LazyBox; + +impl LazyInit for Condvar { + fn init() -> Box { + Box::new(Self::new()) + } +} unsafe impl Send for Condvar {} unsafe impl Sync for Condvar {} impl Condvar { - pub const fn new() -> Condvar { - Condvar { counter: AtomicUsize::new(0), sem1: ptr::null(), sem2: ptr::null() } + pub fn new() -> Self { + let mut condvar = + Self { counter: AtomicUsize::new(0), sem1: ptr::null(), sem2: ptr::null() }; + unsafe { + let _ = abi::sem_init(&mut condvar.sem1, 0); + let _ = abi::sem_init(&mut condvar.sem2, 0); + } + condvar } pub unsafe fn notify_one(&self) { diff --git a/library/std/src/sys/hermit/rwlock.rs b/library/std/src/sys/hermit/rwlock.rs index d43fa08a17150..9701bab1f660b 100644 --- a/library/std/src/sys/hermit/rwlock.rs +++ b/library/std/src/sys/hermit/rwlock.rs @@ -1,9 +1,10 @@ use crate::cell::UnsafeCell; -use crate::sys::locks::{Condvar, Mutex}; +use crate::sys::locks::{MovableCondvar, Mutex}; +use crate::sys_common::lazy_box::{LazyBox, LazyInit}; pub struct RwLock { lock: Mutex, - cond: Condvar, + cond: MovableCondvar, state: UnsafeCell, } @@ -28,7 +29,11 @@ unsafe impl Sync for RwLock {} impl RwLock { pub const fn new() -> RwLock { - RwLock { lock: Mutex::new(), cond: Condvar::new(), state: UnsafeCell::new(State::Unlocked) } + RwLock { + lock: Mutex::new(), + cond: MovableCondvar::new(), + state: UnsafeCell::new(State::Unlocked), + } } #[inline] From 0c8860273c32640db181f7c73b8c54202073fad6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Sun, 26 Jun 2022 23:20:41 +0200 Subject: [PATCH 14/24] Hermit: Make Mutex::init a no-op --- library/std/src/sys/hermit/mutex.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/library/std/src/sys/hermit/mutex.rs b/library/std/src/sys/hermit/mutex.rs index ef44bf411fba5..eb15a04ffcffb 100644 --- a/library/std/src/sys/hermit/mutex.rs +++ b/library/std/src/sys/hermit/mutex.rs @@ -175,9 +175,7 @@ impl Mutex { } #[inline] - pub unsafe fn init(&mut self) { - self.inner = Spinlock::new(MutexInner::new()); - } + pub unsafe fn init(&mut self) {} #[inline] pub unsafe fn lock(&self) { From 871c879bff903d9539c323b719d18d3b61759e62 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 22 Jun 2022 11:25:10 +0100 Subject: [PATCH 15/24] lint: fix condition in diagnostic lints Unfortunately, the diagnostic lints are very broken and trigger much more often than they should. Correct the conditional which checks if the function call being made is to a diagnostic function so that it returns in every intended case. Signed-off-by: David Wood --- compiler/rustc_lint/src/internal.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_lint/src/internal.rs b/compiler/rustc_lint/src/internal.rs index fadb1c8793398..56c8635a189e1 100644 --- a/compiler/rustc_lint/src/internal.rs +++ b/compiler/rustc_lint/src/internal.rs @@ -406,9 +406,12 @@ impl LateLintPass<'_> for Diagnostics { fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { let Some((span, def_id, substs)) = typeck_results_of_method_fn(cx, expr) else { return }; debug!(?span, ?def_id, ?substs); - if let Ok(Some(instance)) = ty::Instance::resolve(cx.tcx, cx.param_env, def_id, substs) && - !cx.tcx.has_attr(instance.def_id(), sym::rustc_lint_diagnostics) - { + let has_attr = ty::Instance::resolve(cx.tcx, cx.param_env, def_id, substs) + .ok() + .and_then(|inst| inst) + .map(|inst| cx.tcx.has_attr(inst.def_id(), sym::rustc_lint_diagnostics)) + .unwrap_or(false); + if !has_attr { return; } From ae612241dc1f474cfa0b3a3895599c984a43caeb Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 22 Jun 2022 13:24:35 +0100 Subject: [PATCH 16/24] various: add `rustc_lint_diagnostics` to diag fns The `rustc_lint_diagnostics` attribute is used by the diagnostic translation/struct migration lints to identify calls where non-translatable diagnostics or diagnostics outwith impls are being created. Any function used in creating a diagnostic should be annotated with this attribute so this commit adds the attribute to many more functions. Signed-off-by: David Wood --- .../rustc_borrowck/src/borrowck_errors.rs | 7 ++++-- compiler/rustc_borrowck/src/lib.rs | 1 + compiler/rustc_expand/src/base.rs | 3 +++ compiler/rustc_expand/src/lib.rs | 1 + compiler/rustc_parse/src/lib.rs | 1 + .../rustc_parse/src/parser/diagnostics.rs | 2 ++ compiler/rustc_session/src/session.rs | 23 +++++++++++++++++++ 7 files changed, 36 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_borrowck/src/borrowck_errors.rs b/compiler/rustc_borrowck/src/borrowck_errors.rs index a1233d62cb02e..708fe8719a1eb 100644 --- a/compiler/rustc_borrowck/src/borrowck_errors.rs +++ b/compiler/rustc_borrowck/src/borrowck_errors.rs @@ -1,4 +1,6 @@ -use rustc_errors::{struct_span_err, DiagnosticBuilder, DiagnosticId, ErrorGuaranteed, MultiSpan}; +use rustc_errors::{ + struct_span_err, DiagnosticBuilder, DiagnosticId, DiagnosticMessage, ErrorGuaranteed, MultiSpan, +}; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_span::Span; @@ -476,10 +478,11 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> { struct_span_err!(self, span, E0716, "temporary value dropped while borrowed",) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] fn struct_span_err_with_code>( &self, sp: S, - msg: &str, + msg: impl Into, code: DiagnosticId, ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { self.infcx.tcx.sess.struct_span_err_with_code(sp, msg, code) diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 8ef2974c37232..a2df072aa3119 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -6,6 +6,7 @@ #![feature(let_else)] #![feature(min_specialization)] #![feature(never_type)] +#![feature(rustc_attrs)] #![feature(stmt_expr_attributes)] #![feature(trusted_step)] #![feature(try_blocks)] diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index 245719bff1202..1e57d66dd9f56 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -1077,6 +1077,7 @@ impl<'a> ExtCtxt<'a> { self.current_expansion.id.expansion_cause() } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_span_err>( &self, sp: S, @@ -1101,9 +1102,11 @@ impl<'a> ExtCtxt<'a> { /// /// Compilation will be stopped in the near future (at the end of /// the macro expansion phase). + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn span_err>(&self, sp: S, msg: &str) { self.sess.parse_sess.span_diagnostic.span_err(sp, msg); } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn span_warn>(&self, sp: S, msg: &str) { self.sess.parse_sess.span_diagnostic.span_warn(sp, msg); } diff --git a/compiler/rustc_expand/src/lib.rs b/compiler/rustc_expand/src/lib.rs index 86ff110eec183..c18147592dc70 100644 --- a/compiler/rustc_expand/src/lib.rs +++ b/compiler/rustc_expand/src/lib.rs @@ -9,6 +9,7 @@ #![feature(proc_macro_diagnostic)] #![feature(proc_macro_internals)] #![feature(proc_macro_span)] +#![feature(rustc_attrs)] #![feature(try_blocks)] #![recursion_limit = "256"] diff --git a/compiler/rustc_parse/src/lib.rs b/compiler/rustc_parse/src/lib.rs index f3bdd63ee6dd4..113af328a91fa 100644 --- a/compiler/rustc_parse/src/lib.rs +++ b/compiler/rustc_parse/src/lib.rs @@ -6,6 +6,7 @@ #![feature(let_chains)] #![feature(let_else)] #![feature(never_type)] +#![feature(rustc_attrs)] #![recursion_limit = "256"] #[macro_use] diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 58d5d43cfbfa8..0869ed65ad2f3 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -357,6 +357,7 @@ impl<'a> DerefMut for SnapshotParser<'a> { } impl<'a> Parser<'a> { + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub(super) fn span_err>( &self, sp: S, @@ -365,6 +366,7 @@ impl<'a> Parser<'a> { err.span_err(sp, self.diagnostic()) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_span_err>( &self, sp: S, diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index f1814eebfa6fd..b5058fd699aca 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -280,6 +280,7 @@ impl Session { self.crate_types.set(crate_types).expect("`crate_types` was initialized twice") } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_span_warn>( &self, sp: S, @@ -287,6 +288,7 @@ impl Session { ) -> DiagnosticBuilder<'_, ()> { self.diagnostic().struct_span_warn(sp, msg) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_span_warn_with_expectation>( &self, sp: S, @@ -295,6 +297,7 @@ impl Session { ) -> DiagnosticBuilder<'_, ()> { self.diagnostic().struct_span_warn_with_expectation(sp, msg, id) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_span_warn_with_code>( &self, sp: S, @@ -303,9 +306,11 @@ impl Session { ) -> DiagnosticBuilder<'_, ()> { self.diagnostic().struct_span_warn_with_code(sp, msg, code) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_warn(&self, msg: impl Into) -> DiagnosticBuilder<'_, ()> { self.diagnostic().struct_warn(msg) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_warn_with_expectation( &self, msg: impl Into, @@ -313,6 +318,7 @@ impl Session { ) -> DiagnosticBuilder<'_, ()> { self.diagnostic().struct_warn_with_expectation(msg, id) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_span_allow>( &self, sp: S, @@ -320,9 +326,11 @@ impl Session { ) -> DiagnosticBuilder<'_, ()> { self.diagnostic().struct_span_allow(sp, msg) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_allow(&self, msg: impl Into) -> DiagnosticBuilder<'_, ()> { self.diagnostic().struct_allow(msg) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_expect( &self, msg: impl Into, @@ -330,6 +338,7 @@ impl Session { ) -> DiagnosticBuilder<'_, ()> { self.diagnostic().struct_expect(msg, id) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_span_err>( &self, sp: S, @@ -337,6 +346,7 @@ impl Session { ) -> DiagnosticBuilder<'_, ErrorGuaranteed> { self.diagnostic().struct_span_err(sp, msg) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_span_err_with_code>( &self, sp: S, @@ -346,12 +356,14 @@ impl Session { self.diagnostic().struct_span_err_with_code(sp, msg, code) } // FIXME: This method should be removed (every error should have an associated error code). + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_err( &self, msg: impl Into, ) -> DiagnosticBuilder<'_, ErrorGuaranteed> { self.parse_sess.struct_err(msg) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_err_with_code( &self, msg: impl Into, @@ -359,6 +371,7 @@ impl Session { ) -> DiagnosticBuilder<'_, ErrorGuaranteed> { self.diagnostic().struct_err_with_code(msg, code) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_warn_with_code( &self, msg: impl Into, @@ -366,6 +379,7 @@ impl Session { ) -> DiagnosticBuilder<'_, ()> { self.diagnostic().struct_warn_with_code(msg, code) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_span_fatal>( &self, sp: S, @@ -373,6 +387,7 @@ impl Session { ) -> DiagnosticBuilder<'_, !> { self.diagnostic().struct_span_fatal(sp, msg) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_span_fatal_with_code>( &self, sp: S, @@ -381,13 +396,16 @@ impl Session { ) -> DiagnosticBuilder<'_, !> { self.diagnostic().struct_span_fatal_with_code(sp, msg, code) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn struct_fatal(&self, msg: impl Into) -> DiagnosticBuilder<'_, !> { self.diagnostic().struct_fatal(msg) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn span_fatal>(&self, sp: S, msg: impl Into) -> ! { self.diagnostic().span_fatal(sp, msg) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn span_fatal_with_code>( &self, sp: S, @@ -396,9 +414,11 @@ impl Session { ) -> ! { self.diagnostic().span_fatal_with_code(sp, msg, code) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn fatal(&self, msg: impl Into) -> ! { self.diagnostic().fatal(msg).raise() } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn span_err_or_warn>( &self, is_warning: bool, @@ -411,6 +431,7 @@ impl Session { self.span_err(sp, msg); } } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn span_err>( &self, sp: S, @@ -418,6 +439,7 @@ impl Session { ) -> ErrorGuaranteed { self.diagnostic().span_err(sp, msg) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn span_err_with_code>( &self, sp: S, @@ -426,6 +448,7 @@ impl Session { ) { self.diagnostic().span_err_with_code(sp, msg, code) } + #[cfg_attr(not(bootstrap), rustc_lint_diagnostics)] pub fn err(&self, msg: impl Into) -> ErrorGuaranteed { self.diagnostic().err(msg) } From be9ebfdbceff0727d37d97a8f4bc812a05a0eff9 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 22 Jun 2022 14:17:34 +0100 Subject: [PATCH 17/24] privacy: port "field is private" diag Signed-off-by: David Wood --- Cargo.lock | 1 + .../locales/en-US/privacy.ftl | 3 ++ compiler/rustc_error_messages/src/lib.rs | 1 + compiler/rustc_privacy/Cargo.toml | 7 +++-- compiler/rustc_privacy/src/errors.rs | 29 ++++++++++++++++++ compiler/rustc_privacy/src/lib.rs | 30 +++++++++---------- 6 files changed, 52 insertions(+), 19 deletions(-) create mode 100644 compiler/rustc_error_messages/locales/en-US/privacy.ftl create mode 100644 compiler/rustc_privacy/src/errors.rs diff --git a/Cargo.lock b/Cargo.lock index 347341b5ff7e2..96d9449db57c0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4280,6 +4280,7 @@ dependencies = [ "rustc_data_structures", "rustc_errors", "rustc_hir", + "rustc_macros", "rustc_middle", "rustc_session", "rustc_span", diff --git a/compiler/rustc_error_messages/locales/en-US/privacy.ftl b/compiler/rustc_error_messages/locales/en-US/privacy.ftl new file mode 100644 index 0000000000000..737aee5e364e1 --- /dev/null +++ b/compiler/rustc_error_messages/locales/en-US/privacy.ftl @@ -0,0 +1,3 @@ +privacy-field-is-private = field `{$field_name}` of {$variant_descr} `{$def_path_str}` is private +privacy-field-is-private-is-update-syntax-label = field `{$field_name}` is private +privacy-field-is-private-label = private field diff --git a/compiler/rustc_error_messages/src/lib.rs b/compiler/rustc_error_messages/src/lib.rs index 673e160cc1e74..90eb5ef54462d 100644 --- a/compiler/rustc_error_messages/src/lib.rs +++ b/compiler/rustc_error_messages/src/lib.rs @@ -32,6 +32,7 @@ pub use unic_langid::{langid, LanguageIdentifier}; // Generates `DEFAULT_LOCALE_RESOURCES` static and `fluent_generated` module. fluent_messages! { parser => "../locales/en-US/parser.ftl", + privacy => "../locales/en-US/privacy.ftl", typeck => "../locales/en-US/typeck.ftl", builtin_macros => "../locales/en-US/builtin_macros.ftl", } diff --git a/compiler/rustc_privacy/Cargo.toml b/compiler/rustc_privacy/Cargo.toml index d952e288a6477..5785921fb1eda 100644 --- a/compiler/rustc_privacy/Cargo.toml +++ b/compiler/rustc_privacy/Cargo.toml @@ -4,14 +4,15 @@ version = "0.0.0" edition = "2021" [dependencies] -rustc_middle = { path = "../rustc_middle" } rustc_ast = { path = "../rustc_ast" } rustc_attr = { path = "../rustc_attr" } +rustc_data_structures = { path = "../rustc_data_structures" } rustc_errors = { path = "../rustc_errors" } rustc_hir = { path = "../rustc_hir" } -rustc_typeck = { path = "../rustc_typeck" } +rustc_macros = { path = "../rustc_macros" } +rustc_middle = { path = "../rustc_middle" } rustc_session = { path = "../rustc_session" } rustc_span = { path = "../rustc_span" } -rustc_data_structures = { path = "../rustc_data_structures" } rustc_trait_selection = { path = "../rustc_trait_selection" } +rustc_typeck = { path = "../rustc_typeck" } tracing = "0.1" diff --git a/compiler/rustc_privacy/src/errors.rs b/compiler/rustc_privacy/src/errors.rs new file mode 100644 index 0000000000000..b101fae0f9414 --- /dev/null +++ b/compiler/rustc_privacy/src/errors.rs @@ -0,0 +1,29 @@ +use rustc_macros::{SessionDiagnostic, SessionSubdiagnostic}; +use rustc_span::{Span, Symbol}; + +#[derive(SessionDiagnostic)] +#[error(privacy::field_is_private, code = "E0451")] +pub struct FieldIsPrivate { + #[primary_span] + pub span: Span, + pub field_name: Symbol, + pub variant_descr: &'static str, + pub def_path_str: String, + #[subdiagnostic] + pub label: FieldIsPrivateLabel, +} + +#[derive(SessionSubdiagnostic)] +pub enum FieldIsPrivateLabel { + #[label(privacy::field_is_private_is_update_syntax_label)] + IsUpdateSyntax { + #[primary_span] + span: Span, + field_name: Symbol, + }, + #[label(privacy::field_is_private_label)] + Other { + #[primary_span] + span: Span, + }, +} diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index b27c986d0f9da..efe1e4bad367d 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -5,6 +5,8 @@ #![recursion_limit = "256"] #![allow(rustc::potential_query_instability)] +mod errors; + use rustc_ast::MacroDef; use rustc_attr as attr; use rustc_data_structures::fx::FxHashSet; @@ -34,6 +36,8 @@ use std::marker::PhantomData; use std::ops::ControlFlow; use std::{cmp, fmt, mem}; +use errors::{FieldIsPrivate, FieldIsPrivateLabel}; + //////////////////////////////////////////////////////////////////////////////// /// Generic infrastructure used to implement specific visitors below. //////////////////////////////////////////////////////////////////////////////// @@ -935,23 +939,17 @@ impl<'tcx> NamePrivacyVisitor<'tcx> { let hir_id = self.tcx.hir().local_def_id_to_hir_id(self.current_item); let def_id = self.tcx.adjust_ident_and_get_scope(ident, def.did(), hir_id).1; if !field.vis.is_accessible_from(def_id, self.tcx) { - let label = if in_update_syntax { - format!("field `{}` is private", field.name) - } else { - "private field".to_string() - }; - - struct_span_err!( - self.tcx.sess, + self.tcx.sess.emit_err(FieldIsPrivate { span, - E0451, - "field `{}` of {} `{}` is private", - field.name, - def.variant_descr(), - self.tcx.def_path_str(def.did()) - ) - .span_label(span, label) - .emit(); + field_name: field.name, + variant_descr: def.variant_descr(), + def_path_str: self.tcx.def_path_str(def.did()), + label: if in_update_syntax { + FieldIsPrivateLabel::IsUpdateSyntax { span, field_name: field.name } + } else { + FieldIsPrivateLabel::Other { span } + }, + }); } } } From cb90a4f30c9f34b0f2b9c55daedb4fa832bbcf70 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 22 Jun 2022 13:31:24 +0100 Subject: [PATCH 18/24] privacy: port "item is private" diag Signed-off-by: David Wood --- .../rustc_error_messages/locales/en-US/privacy.ftl | 3 +++ compiler/rustc_privacy/src/errors.rs | 10 ++++++++++ compiler/rustc_privacy/src/lib.rs | 12 ++++++------ 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_error_messages/locales/en-US/privacy.ftl b/compiler/rustc_error_messages/locales/en-US/privacy.ftl index 737aee5e364e1..6f558d1c6cca9 100644 --- a/compiler/rustc_error_messages/locales/en-US/privacy.ftl +++ b/compiler/rustc_error_messages/locales/en-US/privacy.ftl @@ -1,3 +1,6 @@ privacy-field-is-private = field `{$field_name}` of {$variant_descr} `{$def_path_str}` is private privacy-field-is-private-is-update-syntax-label = field `{$field_name}` is private privacy-field-is-private-label = private field + +privacy-item-is-private = {$kind} `{$descr}` is private + .label = private {$kind} diff --git a/compiler/rustc_privacy/src/errors.rs b/compiler/rustc_privacy/src/errors.rs index b101fae0f9414..a3072ded5a32a 100644 --- a/compiler/rustc_privacy/src/errors.rs +++ b/compiler/rustc_privacy/src/errors.rs @@ -27,3 +27,13 @@ pub enum FieldIsPrivateLabel { span: Span, }, } + +#[derive(SessionDiagnostic)] +#[error(privacy::item_is_private)] +pub struct ItemIsPrivate<'a> { + #[primary_span] + #[label] + pub span: Span, + pub kind: &'a str, + pub descr: String, +} diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index efe1e4bad367d..2ff1d03178385 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -36,7 +36,7 @@ use std::marker::PhantomData; use std::ops::ControlFlow; use std::{cmp, fmt, mem}; -use errors::{FieldIsPrivate, FieldIsPrivateLabel}; +use errors::{FieldIsPrivate, FieldIsPrivateLabel, ItemIsPrivate}; //////////////////////////////////////////////////////////////////////////////// /// Generic infrastructure used to implement specific visitors below. @@ -1073,11 +1073,11 @@ impl<'tcx> TypePrivacyVisitor<'tcx> { fn check_def_id(&mut self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) -> bool { let is_error = !self.item_is_accessible(def_id); if is_error { - self.tcx - .sess - .struct_span_err(self.span, &format!("{} `{}` is private", kind, descr)) - .span_label(self.span, &format!("private {}", kind)) - .emit(); + self.tcx.sess.emit_err(ItemIsPrivate { + span: self.span, + kind, + descr: descr.to_string(), + }); } is_error } From 0557d02a9dcca2d0f2e3c6a5a0a69935da4cf1f5 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 22 Jun 2022 13:42:30 +0100 Subject: [PATCH 19/24] privacy: port unnamed "item is private" diag Signed-off-by: David Wood --- .../rustc_error_messages/locales/en-US/privacy.ftl | 2 ++ compiler/rustc_privacy/src/errors.rs | 8 ++++++++ compiler/rustc_privacy/src/lib.rs | 11 ++++------- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_error_messages/locales/en-US/privacy.ftl b/compiler/rustc_error_messages/locales/en-US/privacy.ftl index 6f558d1c6cca9..de5b94e391650 100644 --- a/compiler/rustc_error_messages/locales/en-US/privacy.ftl +++ b/compiler/rustc_error_messages/locales/en-US/privacy.ftl @@ -4,3 +4,5 @@ privacy-field-is-private-label = private field privacy-item-is-private = {$kind} `{$descr}` is private .label = private {$kind} +privacy-unnamed-item-is-private = {$kind} is private + .label = private {$kind} diff --git a/compiler/rustc_privacy/src/errors.rs b/compiler/rustc_privacy/src/errors.rs index a3072ded5a32a..4db712ac8adc1 100644 --- a/compiler/rustc_privacy/src/errors.rs +++ b/compiler/rustc_privacy/src/errors.rs @@ -37,3 +37,11 @@ pub struct ItemIsPrivate<'a> { pub kind: &'a str, pub descr: String, } + +#[derive(SessionDiagnostic)] +#[error(privacy::unnamed_item_is_private)] +pub struct UnnamedItemIsPrivate { + #[primary_span] + pub span: Span, + pub kind: &'static str, +} diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 2ff1d03178385..59064563de5ce 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -36,7 +36,7 @@ use std::marker::PhantomData; use std::ops::ControlFlow; use std::{cmp, fmt, mem}; -use errors::{FieldIsPrivate, FieldIsPrivateLabel, ItemIsPrivate}; +use errors::{FieldIsPrivate, FieldIsPrivateLabel, ItemIsPrivate, UnnamedItemIsPrivate}; //////////////////////////////////////////////////////////////////////////////// /// Generic infrastructure used to implement specific visitors below. @@ -1248,13 +1248,10 @@ impl<'tcx> Visitor<'tcx> for TypePrivacyVisitor<'tcx> { hir::QPath::TypeRelative(_, segment) => Some(segment.ident.to_string()), }; let kind = kind.descr(def_id); - let msg = match name { - Some(name) => format!("{} `{}` is private", kind, name), - None => format!("{} is private", kind), + let _ = match name { + Some(name) => sess.emit_err(ItemIsPrivate { span, kind, descr: name }), + None => sess.emit_err(UnnamedItemIsPrivate { span, kind }), }; - sess.struct_span_err(span, &msg) - .span_label(span, &format!("private {}", kind)) - .emit(); return; } } From 74f3a965f44c69d2975f9d1206160d8001ac4e70 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 22 Jun 2022 14:11:39 +0100 Subject: [PATCH 20/24] privacy: port "in public interface" diag Signed-off-by: David Wood --- .../locales/en-US/privacy.ftl | 4 ++ compiler/rustc_privacy/src/errors.rs | 28 +++++++++++++ compiler/rustc_privacy/src/lib.rs | 40 +++++++++++++------ 3 files changed, 60 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_error_messages/locales/en-US/privacy.ftl b/compiler/rustc_error_messages/locales/en-US/privacy.ftl index de5b94e391650..2b0778f48caee 100644 --- a/compiler/rustc_error_messages/locales/en-US/privacy.ftl +++ b/compiler/rustc_error_messages/locales/en-US/privacy.ftl @@ -6,3 +6,7 @@ privacy-item-is-private = {$kind} `{$descr}` is private .label = private {$kind} privacy-unnamed-item-is-private = {$kind} is private .label = private {$kind} + +privacy-in-public-interface = {$vis_descr} {$kind} `{$descr}` in public interface + .label = can't leak {$vis_descr} {$kind} + .visibility-label = `{$descr}` declared as {$vis_descr} diff --git a/compiler/rustc_privacy/src/errors.rs b/compiler/rustc_privacy/src/errors.rs index 4db712ac8adc1..482721d373ab7 100644 --- a/compiler/rustc_privacy/src/errors.rs +++ b/compiler/rustc_privacy/src/errors.rs @@ -45,3 +45,31 @@ pub struct UnnamedItemIsPrivate { pub span: Span, pub kind: &'static str, } + +// Duplicate of `InPublicInterface` but with a different error code, shares the same slug. +#[derive(SessionDiagnostic)] +#[error(privacy::in_public_interface, code = "E0445")] +pub struct InPublicInterfaceTraits<'a> { + #[primary_span] + #[label] + pub span: Span, + pub vis_descr: &'static str, + pub kind: &'a str, + pub descr: String, + #[label(privacy::visibility_label)] + pub vis_span: Span, +} + +// Duplicate of `InPublicInterfaceTraits` but with a different error code, shares the same slug. +#[derive(SessionDiagnostic)] +#[error(privacy::in_public_interface, code = "E0446")] +pub struct InPublicInterface<'a> { + #[primary_span] + #[label] + pub span: Span, + pub vis_descr: &'static str, + pub kind: &'a str, + pub descr: String, + #[label(privacy::visibility_label)] + pub vis_span: Span, +} diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 59064563de5ce..c8170fa08e60c 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -11,7 +11,6 @@ use rustc_ast::MacroDef; use rustc_attr as attr; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::intern::Interned; -use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId, LocalDefIdSet, CRATE_DEF_ID}; @@ -36,7 +35,10 @@ use std::marker::PhantomData; use std::ops::ControlFlow; use std::{cmp, fmt, mem}; -use errors::{FieldIsPrivate, FieldIsPrivateLabel, ItemIsPrivate, UnnamedItemIsPrivate}; +use errors::{ + FieldIsPrivate, FieldIsPrivateLabel, InPublicInterface, InPublicInterfaceTraits, ItemIsPrivate, + UnnamedItemIsPrivate, +}; //////////////////////////////////////////////////////////////////////////////// /// Generic infrastructure used to implement specific visitors below. @@ -1748,22 +1750,31 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { } } }; - let make_msg = || format!("{} {} `{}` in public interface", vis_descr, kind, descr); let span = self.tcx.def_span(self.item_def_id.to_def_id()); if self.has_old_errors || self.in_assoc_ty || self.tcx.resolutions(()).has_pub_restricted { - let mut err = if kind == "trait" { - struct_span_err!(self.tcx.sess, span, E0445, "{}", make_msg()) - } else { - struct_span_err!(self.tcx.sess, span, E0446, "{}", make_msg()) - }; + let descr = descr.to_string(); let vis_span = self.tcx.sess.source_map().guess_head_span(self.tcx.def_span(def_id)); - err.span_label(span, format!("can't leak {} {}", vis_descr, kind)); - err.span_label(vis_span, format!("`{}` declared as {}", descr, vis_descr)); - err.emit(); + if kind == "trait" { + self.tcx.sess.emit_err(InPublicInterfaceTraits { + span, + vis_descr, + kind, + descr, + vis_span, + }); + } else { + self.tcx.sess.emit_err(InPublicInterface { + span, + vis_descr, + kind, + descr, + vis_span, + }); + } } else { let err_code = if kind == "trait" { "E0445" } else { "E0446" }; self.tcx.struct_span_lint_hir( @@ -1771,7 +1782,12 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { hir_id, span, |lint| { - lint.build(&format!("{} (error {})", make_msg(), err_code)).emit(); + lint.build(&format!( + "{} (error {})", + format!("{} {} `{}` in public interface", vis_descr, kind, descr), + err_code + )) + .emit(); }, ); } From 15d61d711d1c06ac07c774a2e6988ebefae5b79c Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 22 Jun 2022 14:17:14 +0100 Subject: [PATCH 21/24] privacy: deny diagnostic migration lints Signed-off-by: David Wood --- compiler/rustc_privacy/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index c8170fa08e60c..238c917bbc33f 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -1,9 +1,12 @@ #![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")] +#![feature(associated_type_defaults)] #![feature(control_flow_enum)] +#![feature(rustc_private)] #![feature(try_blocks)] -#![feature(associated_type_defaults)] #![recursion_limit = "256"] #![allow(rustc::potential_query_instability)] +#![cfg_attr(not(bootstrap), deny(rustc::untranslatable_diagnostic))] +#![cfg_attr(not(bootstrap), deny(rustc::diagnostic_outside_of_impl))] mod errors; From c24f06354aa4434e872c72cbb88fb205140738dd Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Tue, 24 May 2022 17:55:13 +0900 Subject: [PATCH 22/24] Remove a back-compat hack on lazy TAIT --- .../rustc_infer/src/infer/opaque_types.rs | 14 ++++++------- .../src/traits/project.rs | 21 ++++--------------- src/test/ui/impl-trait/nested-return-type2.rs | 8 ++----- .../ui/impl-trait/nested-return-type2.stderr | 16 ++++++++++++++ 4 files changed, 28 insertions(+), 31 deletions(-) create mode 100644 src/test/ui/impl-trait/nested-return-type2.stderr diff --git a/compiler/rustc_infer/src/infer/opaque_types.rs b/compiler/rustc_infer/src/infer/opaque_types.rs index 7684d861f3c93..ebb8d4434215f 100644 --- a/compiler/rustc_infer/src/infer/opaque_types.rs +++ b/compiler/rustc_infer/src/infer/opaque_types.rs @@ -39,21 +39,19 @@ pub struct OpaqueTypeDecl<'tcx> { } impl<'a, 'tcx> InferCtxt<'a, 'tcx> { - /// This is a backwards compatibility hack to prevent breaking changes from - /// lazy TAIT around RPIT handling. - pub fn replace_opaque_types_with_inference_vars>( + pub fn replace_opaque_types_with_inference_vars( &self, - value: T, + ty: Ty<'tcx>, body_id: HirId, span: Span, code: ObligationCauseCode<'tcx>, param_env: ty::ParamEnv<'tcx>, - ) -> InferOk<'tcx, T> { - if !value.has_opaque_types() { - return InferOk { value, obligations: vec![] }; + ) -> InferOk<'tcx, Ty<'tcx>> { + if !ty.has_opaque_types() { + return InferOk { value: ty, obligations: vec![] }; } let mut obligations = vec![]; - let value = value.fold_with(&mut ty::fold::BottomUpFolder { + let value = ty.fold_with(&mut ty::fold::BottomUpFolder { tcx: self.tcx, lt_op: |lt| lt, ct_op: |ct| ct, diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs index 82c54291a5d5e..aba4f144d4bcc 100644 --- a/compiler/rustc_trait_selection/src/traits/project.rs +++ b/compiler/rustc_trait_selection/src/traits/project.rs @@ -28,7 +28,6 @@ use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; use rustc_hir::lang_items::LangItem; use rustc_infer::infer::resolve::OpportunisticRegionResolver; -use rustc_infer::traits::ObligationCauseCode; use rustc_middle::traits::select::OverflowError; use rustc_middle::ty::fold::{MaxUniverse, TypeFoldable, TypeFolder, TypeSuperFoldable}; use rustc_middle::ty::subst::Subst; @@ -252,22 +251,10 @@ fn project_and_unify_type<'cx, 'tcx>( Err(InProgress) => return ProjectAndUnifyResult::Recursive, }; debug!(?normalized, ?obligations, "project_and_unify_type result"); - let actual = obligation.predicate.term; - // HACK: lazy TAIT would regress src/test/ui/impl-trait/nested-return-type2.rs, so we add - // a back-compat hack hat converts the RPITs into inference vars, just like they were before - // lazy TAIT. - // This does not affect TAITs in general, as tested in the nested-return-type-tait* tests. - let InferOk { value: actual, obligations: new } = - selcx.infcx().replace_opaque_types_with_inference_vars( - actual, - obligation.cause.body_id, - obligation.cause.span, - ObligationCauseCode::MiscObligation, - obligation.param_env, - ); - obligations.extend(new); - - match infcx.at(&obligation.cause, obligation.param_env).eq(normalized, actual) { + match infcx + .at(&obligation.cause, obligation.param_env) + .eq(normalized, obligation.predicate.term) + { Ok(InferOk { obligations: inferred_obligations, value: () }) => { obligations.extend(inferred_obligations); ProjectAndUnifyResult::Holds(obligations) diff --git a/src/test/ui/impl-trait/nested-return-type2.rs b/src/test/ui/impl-trait/nested-return-type2.rs index 39928d543e15d..279641a46c3d0 100644 --- a/src/test/ui/impl-trait/nested-return-type2.rs +++ b/src/test/ui/impl-trait/nested-return-type2.rs @@ -1,5 +1,3 @@ -// check-pass - trait Duh {} impl Duh for i32 {} @@ -20,11 +18,9 @@ impl R> Trait for F { // the hidden type. We already have obligations registered on the inference // var to make it uphold the `: Duh` bound on `Trait::Assoc`. The opaque // type does not implement `Duh`, even if its hidden type does. -// Lazy TAIT would error out, but we inserted a hack to make it work again, -// keeping backwards compatibility. fn foo() -> impl Trait { + //~^ ERROR `impl Send: Duh` is not satisfied || 42 } -fn main() { -} +fn main() {} diff --git a/src/test/ui/impl-trait/nested-return-type2.stderr b/src/test/ui/impl-trait/nested-return-type2.stderr new file mode 100644 index 0000000000000..f996e99de0742 --- /dev/null +++ b/src/test/ui/impl-trait/nested-return-type2.stderr @@ -0,0 +1,16 @@ +error[E0277]: the trait bound `impl Send: Duh` is not satisfied + --> $DIR/nested-return-type2.rs:21:13 + | +LL | fn foo() -> impl Trait { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Duh` is not implemented for `impl Send` + | + = help: the trait `Duh` is implemented for `i32` +note: required because of the requirements on the impl of `Trait` for `[closure@$DIR/nested-return-type2.rs:23:5: 23:10]` + --> $DIR/nested-return-type2.rs:12:31 + | +LL | impl R> Trait for F { + | ^^^^^ ^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. From 6400736142172ea0cd64e742bf43cd119e24f02a Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Mon, 27 Jun 2022 15:49:59 -0700 Subject: [PATCH 23/24] Implement `Send` and `Sync` for `ThinBox` Just like `Box`, `ThinBox` owns its data on the heap, so it should implement `Send` and `Sync` when `T` does. --- library/alloc/src/boxed/thin.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/library/alloc/src/boxed/thin.rs b/library/alloc/src/boxed/thin.rs index 807c035fdbd0d..203e5dff0c77f 100644 --- a/library/alloc/src/boxed/thin.rs +++ b/library/alloc/src/boxed/thin.rs @@ -33,6 +33,14 @@ pub struct ThinBox { _marker: PhantomData, } +/// `ThinBox` is `Send` if `T` is `Send` because the data is owned. +#[unstable(feature = "thin_box", issue = "92791")] +unsafe impl Send for ThinBox {} + +/// `ThinBox` is `Sync` if `T` is `Sync` because the data is owned. +#[unstable(feature = "thin_box", issue = "92791")] +unsafe impl Sync for ThinBox {} + #[unstable(feature = "thin_box", issue = "92791")] impl ThinBox { /// Moves a type to the heap with its `Metadata` stored in the heap allocation instead of on From f5a38d1b307971b5d7b03e7ace623b0a9936ddc5 Mon Sep 17 00:00:00 2001 From: Weiyi Wang Date: Mon, 27 Jun 2022 19:35:33 -0400 Subject: [PATCH 24/24] Remove unstable CStr/CString change from 1.62 release note (Discovered in https://github.com/rust-lang/rust/pull/98571#discussion_r907469604) The change to move CStr/CString to core/alloc is currently behind feature flags as stated in https://github.com/rust-lang/rust/issues/98314 --- RELEASES.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 3d88891ad215c..7479735012cc9 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -29,7 +29,6 @@ Compiler Libraries --------- -- [Move `CStr` to libcore, and `CString` to liballoc][94079] - [Windows: Use a pipe relay for chaining pipes][95841] - [Replace Linux Mutex and Condvar with futex based ones.][95035] - [Replace RwLock by a futex based one on Linux][95801] @@ -90,7 +89,6 @@ and related tools. [93313]: https://github.com/rust-lang/rust/pull/93313/ [93969]: https://github.com/rust-lang/rust/pull/93969/ -[94079]: https://github.com/rust-lang/rust/pull/94079/ [94206]: https://github.com/rust-lang/rust/pull/94206/ [94457]: https://github.com/rust-lang/rust/pull/94457/ [94775]: https://github.com/rust-lang/rust/pull/94775/