From 06a41687b160cbb4cbf8fce0b3ad3a2e352e8338 Mon Sep 17 00:00:00 2001 From: WANG Rui Date: Thu, 14 Dec 2023 22:07:53 +0800 Subject: [PATCH 01/16] Add unstable `-Z direct-access-external-data` cmdline flag for `rustc` The new flag has been described in the Major Change Proposal at https://github.com/rust-lang/compiler-team/issues/707 --- compiler/rustc_codegen_llvm/src/mono_item.rs | 25 +++++++++++-------- compiler/rustc_interface/src/tests.rs | 1 + compiler/rustc_session/src/options.rs | 2 ++ compiler/rustc_session/src/session.rs | 7 ++++++ compiler/rustc_target/src/spec/mod.rs | 11 ++++++++ .../targets/loongarch64_unknown_linux_gnu.rs | 1 + .../direct-access-external-data.md | 16 ++++++++++++ tests/codegen/direct-access-external-data.rs | 21 ++++++++++++++++ 8 files changed, 74 insertions(+), 10 deletions(-) create mode 100644 src/doc/unstable-book/src/compiler-flags/direct-access-external-data.md create mode 100644 tests/codegen/direct-access-external-data.rs diff --git a/compiler/rustc_codegen_llvm/src/mono_item.rs b/compiler/rustc_codegen_llvm/src/mono_item.rs index f796ce0990f12..f763071936837 100644 --- a/compiler/rustc_codegen_llvm/src/mono_item.rs +++ b/compiler/rustc_codegen_llvm/src/mono_item.rs @@ -123,6 +123,17 @@ impl CodegenCx<'_, '_> { return false; } + // Match clang by only supporting COFF and ELF for now. + if self.tcx.sess.target.is_like_osx { + return false; + } + + // With pie relocation model calls of functions defined in the translation + // unit can use copy relocations. + if self.tcx.sess.relocation_model() == RelocModel::Pie && !is_declaration { + return true; + } + // Thread-local variables generally don't support copy relocations. let is_thread_local_var = llvm::LLVMIsAGlobalVariable(llval) .is_some_and(|v| llvm::LLVMIsThreadLocal(v) == llvm::True); @@ -130,18 +141,12 @@ impl CodegenCx<'_, '_> { return false; } - // Match clang by only supporting COFF and ELF for now. - if self.tcx.sess.target.is_like_osx { - return false; + // Respect the direct-access-external-data to override default behavior if present. + if let Some(direct) = self.tcx.sess.direct_access_external_data() { + return direct; } // Static relocation model should force copy relocations everywhere. - if self.tcx.sess.relocation_model() == RelocModel::Static { - return true; - } - - // With pie relocation model calls of functions defined in the translation - // unit can use copy relocations. - self.tcx.sess.relocation_model() == RelocModel::Pie && !is_declaration + self.tcx.sess.relocation_model() == RelocModel::Static } } diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 588139a303c52..948db6a6ea101 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -748,6 +748,7 @@ fn test_unstable_options_tracking_hash() { tracked!(debug_macros, true); tracked!(default_hidden_visibility, Some(true)); tracked!(dep_info_omit_d_target, true); + tracked!(direct_access_external_data, Some(true)); tracked!(dual_proc_macros, true); tracked!(dwarf_version, Some(5)); tracked!(emit_thin_lto, false); diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index c97b18ebd66ad..ff8c5f4b3568e 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1553,6 +1553,8 @@ options! { dep_info_omit_d_target: bool = (false, parse_bool, [TRACKED], "in dep-info output, omit targets for tracking dependencies of the dep-info files \ themselves (default: no)"), + direct_access_external_data: Option = (None, parse_opt_bool, [TRACKED], + "Direct or use GOT indirect to reference external data symbols"), dual_proc_macros: bool = (false, parse_bool, [TRACKED], "load proc macros for both target and host, but only link to the target (default: no)"), dump_dep_graph: bool = (false, parse_bool, [UNTRACKED], diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 720599f609544..392485e2238c5 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -793,6 +793,13 @@ impl Session { self.opts.unstable_opts.tls_model.unwrap_or(self.target.tls_model) } + pub fn direct_access_external_data(&self) -> Option { + self.opts + .unstable_opts + .direct_access_external_data + .or(self.target.direct_access_external_data) + } + pub fn split_debuginfo(&self) -> SplitDebuginfo { self.opts.cg.split_debuginfo.unwrap_or(self.target.split_debuginfo) } diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 8e26327196a1a..7979be7b5d6e8 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -1885,6 +1885,8 @@ pub struct TargetOptions { /// passed, and cannot be disabled even via `-C`. Corresponds to `llc /// -mattr=$features`. pub features: StaticCow, + /// Direct or use GOT indirect to reference external data symbols + pub direct_access_external_data: Option, /// Whether dynamic linking is available on this target. Defaults to false. pub dynamic_linking: bool, /// Whether dynamic linking can export TLS globals. Defaults to true. @@ -2279,6 +2281,7 @@ impl Default for TargetOptions { asm_args: cvs![], cpu: "generic".into(), features: "".into(), + direct_access_external_data: None, dynamic_linking: false, dll_tls_export: true, only_cdylib: false, @@ -2575,6 +2578,12 @@ impl Target { base.$key_name = s as u32; } } ); + ($key_name:ident, Option) => ( { + let name = (stringify!($key_name)).replace("_", "-"); + if let Some(s) = obj.remove(&name).and_then(|b| b.as_bool()) { + base.$key_name = Some(s); + } + } ); ($key_name:ident, Option) => ( { let name = (stringify!($key_name)).replace("_", "-"); if let Some(s) = obj.remove(&name).and_then(|b| b.as_u64()) { @@ -3003,6 +3012,7 @@ impl Target { key!(cpu); key!(features); key!(dynamic_linking, bool); + key!(direct_access_external_data, Option); key!(dll_tls_export, bool); key!(only_cdylib, bool); key!(executables, bool); @@ -3257,6 +3267,7 @@ impl ToJson for Target { target_option_val!(cpu); target_option_val!(features); target_option_val!(dynamic_linking); + target_option_val!(direct_access_external_data); target_option_val!(dll_tls_export); target_option_val!(only_cdylib); target_option_val!(executables); diff --git a/compiler/rustc_target/src/spec/targets/loongarch64_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/targets/loongarch64_unknown_linux_gnu.rs index 0f05e7c475a83..234270c999b2a 100644 --- a/compiler/rustc_target/src/spec/targets/loongarch64_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/targets/loongarch64_unknown_linux_gnu.rs @@ -11,6 +11,7 @@ pub fn target() -> Target { features: "+f,+d".into(), llvm_abiname: "lp64d".into(), max_atomic_width: Some(64), + direct_access_external_data: Some(false), ..base::linux_gnu::opts() }, } diff --git a/src/doc/unstable-book/src/compiler-flags/direct-access-external-data.md b/src/doc/unstable-book/src/compiler-flags/direct-access-external-data.md new file mode 100644 index 0000000000000..c72df43796038 --- /dev/null +++ b/src/doc/unstable-book/src/compiler-flags/direct-access-external-data.md @@ -0,0 +1,16 @@ +# `direct_access_external_data` + +The tracking issue for this feature is: https://github.com/rust-lang/compiler-team/issues/707 + +------------------------ + +Option `-Z direct-access-external-data` controls how to access symbols of +external data. + +Supported values for this option are: + +- `yes` - Don't use GOT indirection to reference external data symbols. +- `no` - Use GOT indirection to reference external data symbols. + +If the option is not explicitly specified, different targets have different +default values. diff --git a/tests/codegen/direct-access-external-data.rs b/tests/codegen/direct-access-external-data.rs new file mode 100644 index 0000000000000..ec4bfc33518db --- /dev/null +++ b/tests/codegen/direct-access-external-data.rs @@ -0,0 +1,21 @@ +// only-loongarch64-unknown-linux-gnu + +// revisions: DEFAULT DIRECT INDIRECT +// [DEFAULT] compile-flags: -C relocation-model=static +// [DIRECT] compile-flags: -C relocation-model=static -Z direct-access-external-data=yes +// [INDIRECT] compile-flags: -C relocation-model=static -Z direct-access-external-data=no + +#![crate_type = "rlib"] + +// DEFAULT: @VAR = external {{.*}} global i32 +// DIRECT: @VAR = external dso_local {{.*}} global i32 +// INDIRECT: @VAR = external {{.*}} global i32 + +extern "C" { + static VAR: i32; +} + +#[no_mangle] +pub fn get() -> i32 { + unsafe { VAR } +} From f38489e957a1f169be4071947a4426885793f03b Mon Sep 17 00:00:00 2001 From: Jarl Evanson Date: Sun, 28 Jan 2024 13:47:52 -0600 Subject: [PATCH 02/16] Enable `remove_storage_markers` MIR-opt test --- tests/mir-opt/remove_storage_markers.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/mir-opt/remove_storage_markers.rs b/tests/mir-opt/remove_storage_markers.rs index 6666ff3b7263b..74e399b6c0eed 100644 --- a/tests/mir-opt/remove_storage_markers.rs +++ b/tests/mir-opt/remove_storage_markers.rs @@ -1,4 +1,3 @@ -// skip-filecheck // EMIT_MIR_FOR_EACH_PANIC_STRATEGY // unit-test: RemoveStorageMarkers @@ -8,6 +7,10 @@ // EMIT_MIR remove_storage_markers.main.RemoveStorageMarkers.diff fn main() { + // CHECK-LABLE: fn main( + + // CHECK-NOT: StorageDead + // CHECK-NOT: StorageLive let mut sum = 0; for i in 0..10 { sum += i; From d1edc9d0dbd4ce9f1d71eba2ae4116efea8a6f2f Mon Sep 17 00:00:00 2001 From: Jarl Evanson Date: Sun, 28 Jan 2024 13:50:20 -0600 Subject: [PATCH 03/16] Enable `simplify` MIR-opt test --- tests/mir-opt/simplify_if.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/mir-opt/simplify_if.rs b/tests/mir-opt/simplify_if.rs index 19b5806f72094..f600c05958198 100644 --- a/tests/mir-opt/simplify_if.rs +++ b/tests/mir-opt/simplify_if.rs @@ -1,10 +1,13 @@ -// skip-filecheck // EMIT_MIR_FOR_EACH_PANIC_STRATEGY #[inline(never)] fn noop() {} // EMIT_MIR simplify_if.main.SimplifyConstCondition-after-const-prop.diff fn main() { + // CHECK-LABEL: fn main( + + // CHECK: bb0: { + // CHECK-NEXT: return; if false { noop(); } From 103159809aa83e00111f3b35be7281b8165681bc Mon Sep 17 00:00:00 2001 From: Jarl Evanson Date: Sun, 28 Jan 2024 16:04:07 -0600 Subject: [PATCH 04/16] Enable `lifetimes` SROA MIR-opt test --- tests/mir-opt/sroa/lifetimes.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/mir-opt/sroa/lifetimes.rs b/tests/mir-opt/sroa/lifetimes.rs index cc5c0c9bbcdb5..ea04fac15710e 100644 --- a/tests/mir-opt/sroa/lifetimes.rs +++ b/tests/mir-opt/sroa/lifetimes.rs @@ -1,4 +1,3 @@ -// skip-filecheck // unit-test: ScalarReplacementOfAggregates // compile-flags: -Cpanic=abort // no-prefer-dynamic @@ -16,6 +15,10 @@ struct Foo { // EMIT_MIR lifetimes.foo.ScalarReplacementOfAggregates.diff fn foo() { + // CHECK-LABEL: fn foo( + + // CHECK-NOT: [foo:_.*]: Foo + // CHECK-NOT: Box let foo: Foo = Foo { x: Ok(Box::new(5_u32)), y: 7_u32, From bae4f177b8767f6acab5474c6fec2734df759a32 Mon Sep 17 00:00:00 2001 From: Jarl Evanson Date: Sun, 4 Feb 2024 12:04:39 -0600 Subject: [PATCH 05/16] Enable `structs` SROA MIR-opt test --- tests/mir-opt/remove_storage_markers.rs | 2 +- tests/mir-opt/sroa/structs.rs | 125 ++++++++++++++++++++++-- 2 files changed, 120 insertions(+), 7 deletions(-) diff --git a/tests/mir-opt/remove_storage_markers.rs b/tests/mir-opt/remove_storage_markers.rs index 74e399b6c0eed..27661ab325411 100644 --- a/tests/mir-opt/remove_storage_markers.rs +++ b/tests/mir-opt/remove_storage_markers.rs @@ -7,7 +7,7 @@ // EMIT_MIR remove_storage_markers.main.RemoveStorageMarkers.diff fn main() { - // CHECK-LABLE: fn main( + // CHECK-LABEL: fn main( // CHECK-NOT: StorageDead // CHECK-NOT: StorageLive diff --git a/tests/mir-opt/sroa/structs.rs b/tests/mir-opt/sroa/structs.rs index 73563e12c94fc..5ea3795b86e2d 100644 --- a/tests/mir-opt/sroa/structs.rs +++ b/tests/mir-opt/sroa/structs.rs @@ -1,4 +1,3 @@ -// skip-filecheck // unit-test: ScalarReplacementOfAggregates // compile-flags: -Cpanic=abort // no-prefer-dynamic @@ -13,28 +12,68 @@ impl Drop for Tag { fn drop(&mut self) {} } +/// Check that SROA excludes structs with a `Drop` implementation. pub fn dropping() { + // CHECK-LABEL: fn dropping( + + // CHECK: [[aggregate:_[0-9]+]]: S; + + // CHECK: bb0: { + // CHECK: [[aggregate]] = S S(Tag(0), Tag(1), Tag(2)).1; } +/// Check that SROA excludes enums. pub fn enums(a: usize) -> usize { + // CHECK-LABEL: fn enums( + + // CHECK: [[enum:_[0-9]+]]: std::option::Option; + + // CHECK: bb0: { + // CHECK: [[enum]] = Option::::Some + // CHECK: _5 = (([[enum]] as Some).0: usize) + // CHECK: _0 = _5 if let Some(a) = Some(a) { a } else { 0 } } +/// Check that SROA destructures `U`. pub fn structs(a: f32) -> f32 { + // CHECK-LABEL: fn structs( struct U { _foo: usize, a: f32, } - + // CHECK: [[ret:_0]]: f32; + // CHECK: [[struct:_[0-9]+]]: structs::U; + // CHECK: [[a_tmp:_[0-9]+]]: f32; + // CHECK: [[foo:_[0-9]+]]: usize; + // CHECK: [[a_ret:_[0-9]+]]: f32; + + // CHECK: bb0: { + // CHECK-NOT: [[struct]] + // CHECK: [[a_tmp]] = _1; + // CHECK-NOT: [[struct]] + // CHECK: [[foo]] = const 0_usize; + // CHECK-NOT: [[struct]] + // CHECK: [[a_ret]] = move [[a_tmp]]; + // CHECK-NOT: [[struct]] + // CHECK: _0 = [[a_ret]]; + // CHECK-NOT: [[struct]] U { _foo: 0, a }.a } +/// Check that SROA excludes unions. pub fn unions(a: f32) -> u32 { + // CHECK-LABEL: fn unions( union Repr { f: f32, u: u32, } + // CHECK: [[union:_[0-9]+]]: unions::Repr; + + // CHECK: bb0: { + // CHECK: [[union]] = Repr { + // CHECK: _0 = ([[union]].1: u32) unsafe { Repr { f: a }.u } } @@ -46,11 +85,21 @@ struct Foo { d: Option, } -fn g() -> u32 { - 3 -} - +/// Check that non-escaping uses of a struct are destructured. pub fn flat() { + // CHECK-LABEL: fn flat( + + // CHECK: [[struct:_[0-9]+]]: Foo; + + // CHECK: bb0: { + // CHECK: [[init_unit:_[0-9]+]] = (); + // CHECK: [[init_opt_isize:_[0-9]+]] = Option::::Some + + // CHECK: [[destr_five:_[0-9]+]] = const 5_u8; + // CHECK: [[destr_unit:_[0-9]+]] = move [[init_unit]]; + // CHECK: [[destr_a:_[0-9]+]] = const "a"; + // CHECK: [[destr_opt_isize:_[0-9]+]] = move [[init_opt_isize]]; + let Foo { a, b, c, d } = Foo { a: 5, b: (), c: "a", d: Some(-4) }; let _ = a; let _ = b; @@ -65,6 +114,10 @@ struct Escaping { c: u32, } +fn g() -> u32 { + 3 +} + fn f(a: *const u32) { println!("{}", unsafe { *a.add(2) }); } @@ -76,10 +129,38 @@ fn f(a: *const u32) { // of them to `f`. However, this would lead to a miscompilation because `b` and `c` // might no longer appear right after `a` in memory. pub fn escaping() { + // CHECK-LABEL: fn escaping( + + // CHECK: [[ptr:_[0-9]+]]: *const u32; + // CHECK: [[ref:_[0-9]+]]: &u32; + // CHECK: [[struct:_[0-9]+]]: Escaping; + // CHECK: [[a:_[0-9]+]]: u32; + + // CHECK: bb0: { + // CHECK: [[struct]] = Escaping { + // CHECK: [[ref]] = &([[struct]].0 + // CHECK: [[ptr]] = &raw const (*[[ref]]); f(&Escaping { a: 1, b: 2, c: g() }.a); } +/// Check that copies from an internal struct are destructured and reassigned to +/// the original struct. fn copies(x: Foo) { + // CHECK-LABEL: fn copies( + + // CHECK: [[external:_[0-9]+]]: Foo) -> + // CHECK: [[internal:_[0-9]+]]: Foo; + // CHECK: [[byte:_[0-9]+]]: u8; + // CHECK: [[unit:_[0-9]+]]: (); + // CHECK: [[str:_[0-9]+]]: &str; + // CHECK: [[opt_isize:_[0-9]+]]: std::option::Option; + + // CHECK: bb0: { + // CHECK: [[byte]] = ([[external]].0 + // CHECK: [[unit]] = ([[external]].1 + // CHECK: [[str]] = ([[external]].2 + // CHECK: [[opt_isize]] = ([[external]].3 + let y = x; let t = y.a; let u = y.c; @@ -87,13 +168,44 @@ fn copies(x: Foo) { let a = z.b; } +/// Check that copies from an internal struct are destructured and reassigned to +/// the original struct. fn ref_copies(x: &Foo) { + // CHECK-LABEL: fn ref_copies( + + // CHECK: [[external:_[0-9]+]]: &Foo) -> + // CHECK: [[internal:_[0-9]+]]: Foo; + // CHECK: [[byte:_[0-9]+]]: u8; + // CHECK: [[unit:_[0-9]+]]: (); + // CHECK: [[str:_[0-9]+]]: &str; + // CHECK: [[opt_isize:_[0-9]+]]: std::option::Option; + + // CHECK: bb0: { + // CHECK: [[byte]] = ((*[[external]]).0 + // CHECK: [[unit]] = ((*[[external]]).1 + // CHECK: [[str]] = ((*[[external]]).2 + // CHECK: [[opt_isize]] = ((*[[external]]).3 + let y = *x; let t = y.a; let u = y.c; } +/// Check that deaggregated assignments from constants are placed after the constant's +/// assignment. Also check that copying field accesses from the copy of the constant are +/// reassigned to copy from the constant. fn constant() { + // CHECK-LABEL: constant( + + // CHECK: [[constant:_[0-9]+]]: (usize, u8); + // CHECK: [[t:_[0-9]+]]: usize; + // CHECK: [[u:_[0-9]+]]: u8; + + // CHECK: bb0: { + // CHECK-NOT: [[constant]] + // CHECK: [[constant]] = const + // CHECK: [[t]] = move ([[constant]].0: usize) + // CHECK: [[u]] = move ([[constant]].1: u8) const U: (usize, u8) = (5, 9); let y = U; let t = y.0; @@ -101,6 +213,7 @@ fn constant() { } fn main() { + // CHECK-LABEL: fn main( dropping(); enums(5); structs(5.); From 8411eaebd4b7cc0c73c7e78513b1614194a0f6bd Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 3 Feb 2024 18:39:24 +0000 Subject: [PATCH 06/16] Assert that ParamTy and ParamConst have identical names for identical indices --- compiler/rustc_middle/src/ty/relate.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_middle/src/ty/relate.rs b/compiler/rustc_middle/src/ty/relate.rs index 8543bd0bbdd96..8bd5605d39aa7 100644 --- a/compiler/rustc_middle/src/ty/relate.rs +++ b/compiler/rustc_middle/src/ty/relate.rs @@ -435,7 +435,10 @@ pub fn structurally_relate_tys<'tcx, R: TypeRelation<'tcx>>( Ok(a) } - (ty::Param(a_p), ty::Param(b_p)) if a_p.index == b_p.index => Ok(a), + (ty::Param(a_p), ty::Param(b_p)) if a_p.index == b_p.index => { + debug_assert_eq!(a_p.name, b_p.name, "param types with same index differ in name"); + Ok(a) + }, (ty::Placeholder(p1), ty::Placeholder(p2)) if p1 == p2 => Ok(a), @@ -586,7 +589,10 @@ pub fn structurally_relate_consts<'tcx, R: TypeRelation<'tcx>>( (ty::ConstKind::Error(_), _) => return Ok(a), (_, ty::ConstKind::Error(_)) => return Ok(b), - (ty::ConstKind::Param(a_p), ty::ConstKind::Param(b_p)) => a_p.index == b_p.index, + (ty::ConstKind::Param(a_p), ty::ConstKind::Param(b_p)) if a_p.index == b_p.index => { + debug_assert_eq!(a_p.name, b_p.name, "param types with same index differ in name"); + true + } (ty::ConstKind::Placeholder(p1), ty::ConstKind::Placeholder(p2)) => p1 == p2, (ty::ConstKind::Value(a_val), ty::ConstKind::Value(b_val)) => a_val == b_val, From 6c9a64f19cef4f23bae2d92d630d91ceeeb8d93b Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 3 Feb 2024 18:52:30 +0000 Subject: [PATCH 07/16] Do not create param types that differ only by name when comparing intrinsic signatures --- .../rustc_hir_analysis/src/check/intrinsic.rs | 25 ++++++++++++++++--- compiler/rustc_middle/src/ty/relate.rs | 2 +- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/intrinsic.rs b/compiler/rustc_hir_analysis/src/check/intrinsic.rs index 2d0d6611444c5..7f12ce1acf5e9 100644 --- a/compiler/rustc_hir_analysis/src/check/intrinsic.rs +++ b/compiler/rustc_hir_analysis/src/check/intrinsic.rs @@ -12,7 +12,7 @@ use rustc_errors::{codes::*, struct_span_code_err, DiagnosticMessage}; use rustc_hir as hir; use rustc_middle::traits::{ObligationCause, ObligationCauseCode}; use rustc_middle::ty::{self, Ty, TyCtxt}; -use rustc_span::symbol::{kw, sym, Symbol}; +use rustc_span::symbol::{kw, sym}; use rustc_target::spec::abi::Abi; fn equate_intrinsic_type<'tcx>( @@ -132,7 +132,17 @@ pub fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: DefId) -> hir /// Remember to add all intrinsics here, in `compiler/rustc_codegen_llvm/src/intrinsic.rs`, /// and in `library/core/src/intrinsics.rs`. pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) { - let param = |n| Ty::new_param(tcx, n, Symbol::intern(&format!("P{n}"))); + let generics = tcx.generics_of(it.owner_id); + let param = |n| { + if let Some(&ty::GenericParamDef { + name, kind: ty::GenericParamDefKind::Type { .. }, .. + }) = generics.opt_param_at(n as usize, tcx) + { + Ty::new_param(tcx, n, name) + } else { + Ty::new_error_with_message(tcx, tcx.def_span(it.owner_id), "expected param") + } + }; let intrinsic_id = it.owner_id.to_def_id(); let intrinsic_name = tcx.item_name(intrinsic_id); let name_str = intrinsic_name.as_str(); @@ -475,9 +485,16 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) { /// Type-check `extern "platform-intrinsic" { ... }` functions. pub fn check_platform_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) { + let generics = tcx.generics_of(it.owner_id); let param = |n| { - let name = Symbol::intern(&format!("P{n}")); - Ty::new_param(tcx, n, name) + if let Some(&ty::GenericParamDef { + name, kind: ty::GenericParamDefKind::Type { .. }, .. + }) = generics.opt_param_at(n as usize, tcx) + { + Ty::new_param(tcx, n, name) + } else { + Ty::new_error_with_message(tcx, tcx.def_span(it.owner_id), "expected param") + } }; let name = it.ident.name; diff --git a/compiler/rustc_middle/src/ty/relate.rs b/compiler/rustc_middle/src/ty/relate.rs index 8bd5605d39aa7..ed9e2e12ee48e 100644 --- a/compiler/rustc_middle/src/ty/relate.rs +++ b/compiler/rustc_middle/src/ty/relate.rs @@ -438,7 +438,7 @@ pub fn structurally_relate_tys<'tcx, R: TypeRelation<'tcx>>( (ty::Param(a_p), ty::Param(b_p)) if a_p.index == b_p.index => { debug_assert_eq!(a_p.name, b_p.name, "param types with same index differ in name"); Ok(a) - }, + } (ty::Placeholder(p1), ty::Placeholder(p2)) if p1 == p2 => Ok(a), From c80c83629f91696eb68d06423ba9269bbe487677 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 5 Feb 2024 17:43:28 +0000 Subject: [PATCH 08/16] Use correct param env when building and cleaning items in librustdoc --- src/librustdoc/clean/inline.rs | 56 +++++++++++++------- src/librustdoc/clean/mod.rs | 4 +- src/librustdoc/clean/utils.rs | 8 ++- src/librustdoc/passes/collect_trait_impls.rs | 12 +++-- 4 files changed, 56 insertions(+), 24 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index aab974ad79edd..08f5fe1f55a71 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -65,37 +65,49 @@ pub(crate) fn try_inline( let kind = match res { Res::Def(DefKind::Trait, did) => { record_extern_fqn(cx, did, ItemType::Trait); - build_impls(cx, did, attrs_without_docs, &mut ret); - clean::TraitItem(Box::new(build_external_trait(cx, did))) + cx.with_param_env(did, |cx| { + build_impls(cx, did, attrs_without_docs, &mut ret); + clean::TraitItem(Box::new(build_external_trait(cx, did))) + }) } Res::Def(DefKind::Fn, did) => { record_extern_fqn(cx, did, ItemType::Function); - clean::FunctionItem(build_external_function(cx, did)) + cx.with_param_env(did, |cx| clean::FunctionItem(build_external_function(cx, did))) } Res::Def(DefKind::Struct, did) => { record_extern_fqn(cx, did, ItemType::Struct); - build_impls(cx, did, attrs_without_docs, &mut ret); - clean::StructItem(build_struct(cx, did)) + cx.with_param_env(did, |cx| { + build_impls(cx, did, attrs_without_docs, &mut ret); + clean::StructItem(build_struct(cx, did)) + }) } Res::Def(DefKind::Union, did) => { record_extern_fqn(cx, did, ItemType::Union); - build_impls(cx, did, attrs_without_docs, &mut ret); - clean::UnionItem(build_union(cx, did)) + cx.with_param_env(did, |cx| { + build_impls(cx, did, attrs_without_docs, &mut ret); + clean::UnionItem(build_union(cx, did)) + }) } Res::Def(DefKind::TyAlias, did) => { record_extern_fqn(cx, did, ItemType::TypeAlias); - build_impls(cx, did, attrs_without_docs, &mut ret); - clean::TypeAliasItem(build_type_alias(cx, did, &mut ret)) + cx.with_param_env(did, |cx| { + build_impls(cx, did, attrs_without_docs, &mut ret); + clean::TypeAliasItem(build_type_alias(cx, did, &mut ret)) + }) } Res::Def(DefKind::Enum, did) => { record_extern_fqn(cx, did, ItemType::Enum); - build_impls(cx, did, attrs_without_docs, &mut ret); - clean::EnumItem(build_enum(cx, did)) + cx.with_param_env(did, |cx| { + build_impls(cx, did, attrs_without_docs, &mut ret); + clean::EnumItem(build_enum(cx, did)) + }) } Res::Def(DefKind::ForeignTy, did) => { record_extern_fqn(cx, did, ItemType::ForeignType); - build_impls(cx, did, attrs_without_docs, &mut ret); - clean::ForeignTypeItem + cx.with_param_env(did, |cx| { + build_impls(cx, did, attrs_without_docs, &mut ret); + clean::ForeignTypeItem + }) } // Never inline enum variants but leave them shown as re-exports. Res::Def(DefKind::Variant, _) => return None, @@ -108,11 +120,13 @@ pub(crate) fn try_inline( } Res::Def(DefKind::Static(_), did) => { record_extern_fqn(cx, did, ItemType::Static); - clean::StaticItem(build_static(cx, did, cx.tcx.is_mutable_static(did))) + cx.with_param_env(did, |cx| { + clean::StaticItem(build_static(cx, did, cx.tcx.is_mutable_static(did))) + }) } Res::Def(DefKind::Const, did) => { record_extern_fqn(cx, did, ItemType::Constant); - clean::ConstantItem(build_const(cx, did)) + cx.with_param_env(did, |cx| clean::ConstantItem(build_const(cx, did))) } Res::Def(DefKind::Macro(kind), did) => { let mac = build_macro(cx, did, name, import_def_id, kind); @@ -313,7 +327,9 @@ pub(crate) fn build_impls( // for each implementation of an item represented by `did`, build the clean::Item for that impl for &did in tcx.inherent_impls(did).into_iter().flatten() { - build_impl(cx, did, attrs, ret); + cx.with_param_env(did, |cx| { + build_impl(cx, did, attrs, ret); + }); } // This pretty much exists expressly for `dyn Error` traits that exist in the `alloc` crate. @@ -326,7 +342,9 @@ pub(crate) fn build_impls( let type_ = if tcx.is_trait(did) { SimplifiedType::Trait(did) } else { SimplifiedType::Adt(did) }; for &did in tcx.incoherent_impls(type_).into_iter().flatten() { - build_impl(cx, did, attrs, ret); + cx.with_param_env(did, |cx| { + build_impl(cx, did, attrs, ret); + }); } } } @@ -528,7 +546,9 @@ pub(crate) fn build_impl( } if let Some(did) = trait_.as_ref().map(|t| t.def_id()) { - record_extern_trait(cx, did); + cx.with_param_env(did, |cx| { + record_extern_trait(cx, did); + }); } let (merged_attrs, cfg) = merge_attrs(cx, load_attrs(cx, did), attrs); diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index bd1d68e7074e1..65b590231466b 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -947,7 +947,9 @@ fn clean_ty_alias_inner_type<'tcx>( }; if !adt_def.did().is_local() { - inline::build_impls(cx, adt_def.did(), None, ret); + cx.with_param_env(adt_def.did(), |cx| { + inline::build_impls(cx, adt_def.did(), None, ret); + }); } Some(if adt_def.is_enum() { diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index b2b20c95a7e6a..0b7d35d7be4c8 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -295,12 +295,16 @@ pub(crate) fn build_deref_target_impls( if let Some(prim) = target.primitive_type() { let _prof_timer = tcx.sess.prof.generic_activity("build_primitive_inherent_impls"); for did in prim.impls(tcx).filter(|did| !did.is_local()) { - inline::build_impl(cx, did, None, ret); + cx.with_param_env(did, |cx| { + inline::build_impl(cx, did, None, ret); + }); } } else if let Type::Path { path } = target { let did = path.def_id(); if !did.is_local() { - inline::build_impls(cx, did, None, ret); + cx.with_param_env(did, |cx| { + inline::build_impls(cx, did, None, ret); + }); } } } diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index e2a7ef8556ea6..53c08ef5e5ca5 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -49,7 +49,9 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) -> let _prof_timer = tcx.sess.prof.generic_activity("build_extern_trait_impls"); for &cnum in tcx.crates(()) { for &impl_def_id in tcx.trait_impls_in_crate(cnum) { - inline::build_impl(cx, impl_def_id, None, &mut new_items_external); + cx.with_param_env(impl_def_id, |cx| { + inline::build_impl(cx, impl_def_id, None, &mut new_items_external); + }); } } } @@ -74,7 +76,9 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) -> ); parent = tcx.opt_parent(did); } - inline::build_impl(cx, impl_def_id, Some((&attr_buf, None)), &mut new_items_local); + cx.with_param_env(impl_def_id, |cx| { + inline::build_impl(cx, impl_def_id, Some((&attr_buf, None)), &mut new_items_local); + }); attr_buf.clear(); } } @@ -83,7 +87,9 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) -> for def_id in PrimitiveType::all_impls(tcx) { // Try to inline primitive impls from other crates. if !def_id.is_local() { - inline::build_impl(cx, def_id, None, &mut new_items_external); + cx.with_param_env(def_id, |cx| { + inline::build_impl(cx, def_id, None, &mut new_items_external); + }); } } for (prim, did) in PrimitiveType::primitive_locations(tcx) { From 16cbdd0321f342093b71026dc1c5126606e4abe9 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 18 Jan 2024 16:39:20 +0000 Subject: [PATCH 09/16] Allow desugaring async fn in trait to compatible, concrete future types --- compiler/rustc_hir_analysis/messages.ftl | 4 --- .../src/check/compare_impl_item.rs | 31 ------------------- compiler/rustc_hir_analysis/src/errors.rs | 11 ------- .../in-trait/async-example-desugared-boxed.rs | 2 +- .../async-example-desugared-boxed.stderr | 11 ------- .../async-example-desugared-manual.rs | 2 +- .../async-example-desugared-manual.stderr | 11 ------- .../async-await/in-trait/fn-not-async-err.rs | 2 +- .../in-trait/fn-not-async-err.stderr | 18 +++++++---- 9 files changed, 15 insertions(+), 77 deletions(-) delete mode 100644 tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr delete mode 100644 tests/ui/async-await/in-trait/async-example-desugared-manual.stderr diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index 54d0fb6ffab72..d0fb0e3fb7646 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -33,10 +33,6 @@ hir_analysis_associated_type_trait_uninferred_generic_params = cannot use the as hir_analysis_associated_type_trait_uninferred_generic_params_multipart_suggestion = use a fully qualified path with explicit lifetimes -hir_analysis_async_trait_impl_should_be_async = - method `{$method_name}` should be async because the method from the trait is async - .trait_item_label = required because the trait method is async - hir_analysis_auto_deref_reached_recursion_limit = reached the recursion limit while auto-dereferencing `{$ty}` .label = deref recursion limit reached .help = consider increasing the recursion limit by adding a `#![recursion_limit = "{$suggested_limit}"]` attribute to your crate (`{$crate_name}`) diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index 379c1154e5f26..f9c1ed0e0e1da 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -74,7 +74,6 @@ fn check_method_is_structurally_compatible<'tcx>( compare_generic_param_kinds(tcx, impl_m, trait_m, delay)?; compare_number_of_method_arguments(tcx, impl_m, trait_m, delay)?; compare_synthetic_generics(tcx, impl_m, trait_m, delay)?; - compare_asyncness(tcx, impl_m, trait_m, delay)?; check_region_bounds_on_impl_item(tcx, impl_m, trait_m, delay)?; Ok(()) } @@ -414,36 +413,6 @@ impl<'tcx> TypeFolder> for RemapLateBound<'_, 'tcx> { } } -fn compare_asyncness<'tcx>( - tcx: TyCtxt<'tcx>, - impl_m: ty::AssocItem, - trait_m: ty::AssocItem, - delay: bool, -) -> Result<(), ErrorGuaranteed> { - if tcx.asyncness(trait_m.def_id).is_async() { - match tcx.fn_sig(impl_m.def_id).skip_binder().skip_binder().output().kind() { - ty::Alias(ty::Opaque, ..) => { - // allow both `async fn foo()` and `fn foo() -> impl Future` - } - ty::Error(_) => { - // We don't know if it's ok, but at least it's already an error. - } - _ => { - return Err(tcx - .dcx() - .create_err(crate::errors::AsyncTraitImplShouldBeAsync { - span: tcx.def_span(impl_m.def_id), - method_name: trait_m.name, - trait_item_span: tcx.hir().span_if_local(trait_m.def_id), - }) - .emit_unless(delay)); - } - }; - } - - Ok(()) -} - /// Given a method def-id in an impl, compare the method signature of the impl /// against the trait that it's implementing. In doing so, infer the hidden types /// that this method's signature provides to satisfy each return-position `impl Trait` diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index 4eba31e327f68..8b8c0f7ff8dbb 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -166,17 +166,6 @@ pub struct LifetimesOrBoundsMismatchOnTrait { pub ident: Ident, } -#[derive(Diagnostic)] -#[diag(hir_analysis_async_trait_impl_should_be_async)] -pub struct AsyncTraitImplShouldBeAsync { - #[primary_span] - // #[label] - pub span: Span, - #[label(hir_analysis_trait_item_label)] - pub trait_item_span: Option, - pub method_name: Symbol, -} - #[derive(Diagnostic)] #[diag(hir_analysis_drop_impl_on_wrong_item, code = E0120)] pub struct DropImplOnWrongItem { diff --git a/tests/ui/async-await/in-trait/async-example-desugared-boxed.rs b/tests/ui/async-await/in-trait/async-example-desugared-boxed.rs index c5a9841029e38..4008e09998e47 100644 --- a/tests/ui/async-await/in-trait/async-example-desugared-boxed.rs +++ b/tests/ui/async-await/in-trait/async-example-desugared-boxed.rs @@ -1,4 +1,5 @@ // edition: 2021 +// check-pass use std::future::Future; use std::pin::Pin; @@ -9,7 +10,6 @@ trait MyTrait { impl MyTrait for i32 { fn foo(&self) -> Pin + '_>> { - //~^ ERROR method `foo` should be async Box::pin(async { *self }) } } diff --git a/tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr b/tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr deleted file mode 100644 index 1462c694e161e..0000000000000 --- a/tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error: method `foo` should be async because the method from the trait is async - --> $DIR/async-example-desugared-boxed.rs:11:5 - | -LL | async fn foo(&self) -> i32; - | --------------------------- required because the trait method is async -... -LL | fn foo(&self) -> Pin + '_>> { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 1 previous error - diff --git a/tests/ui/async-await/in-trait/async-example-desugared-manual.rs b/tests/ui/async-await/in-trait/async-example-desugared-manual.rs index c287b9a5b847f..75f4ba1d076c6 100644 --- a/tests/ui/async-await/in-trait/async-example-desugared-manual.rs +++ b/tests/ui/async-await/in-trait/async-example-desugared-manual.rs @@ -1,4 +1,5 @@ // edition: 2021 +// check-pass use std::future::Future; use std::task::Poll; @@ -17,7 +18,6 @@ impl Future for MyFuture { impl MyTrait for u32 { fn foo(&self) -> MyFuture { - //~^ ERROR method `foo` should be async MyFuture } } diff --git a/tests/ui/async-await/in-trait/async-example-desugared-manual.stderr b/tests/ui/async-await/in-trait/async-example-desugared-manual.stderr deleted file mode 100644 index a2f1060e36fc4..0000000000000 --- a/tests/ui/async-await/in-trait/async-example-desugared-manual.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error: method `foo` should be async because the method from the trait is async - --> $DIR/async-example-desugared-manual.rs:19:5 - | -LL | async fn foo(&self) -> i32; - | --------------------------- required because the trait method is async -... -LL | fn foo(&self) -> MyFuture { - | ^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 1 previous error - diff --git a/tests/ui/async-await/in-trait/fn-not-async-err.rs b/tests/ui/async-await/in-trait/fn-not-async-err.rs index 60077a7e00cc8..6261ed1f1b7b3 100644 --- a/tests/ui/async-await/in-trait/fn-not-async-err.rs +++ b/tests/ui/async-await/in-trait/fn-not-async-err.rs @@ -8,7 +8,7 @@ trait MyTrait { impl MyTrait for i32 { fn foo(&self) -> i32 { - //~^ ERROR: method `foo` should be async + //~^ ERROR: `i32` is not a future *self } } diff --git a/tests/ui/async-await/in-trait/fn-not-async-err.stderr b/tests/ui/async-await/in-trait/fn-not-async-err.stderr index f75ccb65d1509..dc46077d583af 100644 --- a/tests/ui/async-await/in-trait/fn-not-async-err.stderr +++ b/tests/ui/async-await/in-trait/fn-not-async-err.stderr @@ -1,11 +1,17 @@ -error: method `foo` should be async because the method from the trait is async - --> $DIR/fn-not-async-err.rs:10:5 +error[E0277]: `i32` is not a future + --> $DIR/fn-not-async-err.rs:10:22 | -LL | async fn foo(&self) -> i32; - | --------------------------- required because the trait method is async -... LL | fn foo(&self) -> i32 { - | ^^^^^^^^^^^^^^^^^^^^ + | ^^^ `i32` is not a future + | + = help: the trait `Future` is not implemented for `i32` + = note: i32 must be a future or must implement `IntoFuture` to be awaited +note: required by a bound in `MyTrait::{opaque#0}` + --> $DIR/fn-not-async-err.rs:6:5 + | +LL | async fn foo(&self) -> i32; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `MyTrait::{opaque#0}` error: aborting due to 1 previous error +For more information about this error, try `rustc --explain E0277`. From e65abc0ea58f2f7dd6812fafa92c4f5b2a28af7b Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 18 Jan 2024 17:20:39 +0000 Subject: [PATCH 10/16] Make the error message better --- compiler/rustc_hir_analysis/messages.ftl | 3 + .../src/check/compare_impl_item.rs | 57 +++++++++++++++++-- compiler/rustc_hir_analysis/src/errors.rs | 10 ++++ .../async-await/in-trait/fn-not-async-err.rs | 2 +- .../in-trait/fn-not-async-err.stderr | 13 ++--- 5 files changed, 72 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index d0fb0e3fb7646..d6f604c180bf6 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -206,6 +206,9 @@ hir_analysis_manual_implementation = .label = manual implementations of `{$trait_name}` are experimental .help = add `#![feature(unboxed_closures)]` to the crate attributes to enable +hir_analysis_method_should_return_future = method should be `async` or return a future, but it is synchronous + .note = this method is `async` so it expects a future to be returned + hir_analysis_missing_one_of_trait_item = not all trait items implemented, missing one of: `{$missing_items_msg}` .label = missing one of `{$missing_items_msg}` in implementation .note = required because of this annotation diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index f9c1ed0e0e1da..18037ea6991e8 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -1,5 +1,5 @@ use super::potentially_plural_count; -use crate::errors::LifetimesOrBoundsMismatchOnTrait; +use crate::errors::{LifetimesOrBoundsMismatchOnTrait, MethodShouldReturnFuture}; use hir::def_id::{DefId, DefIdMap, LocalDefId}; use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet}; use rustc_errors::{codes::*, pluralize, struct_span_code_err, Applicability, ErrorGuaranteed}; @@ -10,7 +10,7 @@ use rustc_hir::{GenericParamKind, ImplItemKind}; use rustc_infer::infer::outlives::env::OutlivesEnvironment; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use rustc_infer::infer::{self, InferCtxt, TyCtxtInferExt}; -use rustc_infer::traits::util; +use rustc_infer::traits::{util, FulfillmentError}; use rustc_middle::ty::error::{ExpectedFound, TypeError}; use rustc_middle::ty::fold::BottomUpFolder; use rustc_middle::ty::util::ExplicitSelf; @@ -664,8 +664,13 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>( // RPITs. let errors = ocx.select_all_or_error(); if !errors.is_empty() { - let reported = infcx.err_ctxt().report_fulfillment_errors(errors); - return Err(reported); + if let Err(guar) = try_report_async_mismatch(tcx, infcx, &errors, trait_m, impl_m, impl_sig) + { + return Err(guar); + } + + let guar = infcx.err_ctxt().report_fulfillment_errors(errors); + return Err(guar); } // Finally, resolve all regions. This catches wily misuses of @@ -2217,3 +2222,47 @@ fn assoc_item_kind_str(impl_item: &ty::AssocItem) -> &'static str { ty::AssocKind::Type => "type", } } + +/// Manually check here that `async fn foo()` wasn't matched against `fn foo()`, +/// and extract a better error if so. +fn try_report_async_mismatch<'tcx>( + tcx: TyCtxt<'tcx>, + infcx: &InferCtxt<'tcx>, + errors: &[FulfillmentError<'tcx>], + trait_m: ty::AssocItem, + impl_m: ty::AssocItem, + impl_sig: ty::FnSig<'tcx>, +) -> Result<(), ErrorGuaranteed> { + if !tcx.asyncness(trait_m.def_id).is_async() { + return Ok(()); + } + + let ty::Alias(ty::Projection, ty::AliasTy { def_id: async_future_def_id, .. }) = + *tcx.fn_sig(trait_m.def_id).skip_binder().skip_binder().output().kind() + else { + bug!("expected `async fn` to return an RPITIT"); + }; + + for error in errors { + if let traits::BindingObligation(def_id, _) = *error.root_obligation.cause.code() + && def_id == async_future_def_id + && let Some(proj) = error.root_obligation.predicate.to_opt_poly_projection_pred() + && let Some(proj) = proj.no_bound_vars() + && infcx.can_eq( + error.root_obligation.param_env, + proj.term.ty().unwrap(), + impl_sig.output(), + ) + { + // FIXME: We should suggest making the fn `async`, but extracting + // the right span is a bit difficult. + return Err(tcx.sess.dcx().emit_err(MethodShouldReturnFuture { + span: tcx.def_span(impl_m.def_id), + method_name: trait_m.name, + trait_item_span: tcx.hir().span_if_local(trait_m.def_id), + })); + } + } + + Ok(()) +} diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index 8b8c0f7ff8dbb..bec53693d6c94 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -1501,6 +1501,16 @@ pub struct NotSupportedDelegation<'a> { pub callee_span: Span, } +#[derive(Diagnostic)] +#[diag(hir_analysis_method_should_return_future)] +pub struct MethodShouldReturnFuture { + #[primary_span] + pub span: Span, + pub method_name: Symbol, + #[note] + pub trait_item_span: Option, +} + #[derive(Diagnostic)] #[diag(hir_analysis_unused_generic_parameter)] pub(crate) struct UnusedGenericParameter { diff --git a/tests/ui/async-await/in-trait/fn-not-async-err.rs b/tests/ui/async-await/in-trait/fn-not-async-err.rs index 6261ed1f1b7b3..ecd5737cf3c23 100644 --- a/tests/ui/async-await/in-trait/fn-not-async-err.rs +++ b/tests/ui/async-await/in-trait/fn-not-async-err.rs @@ -8,7 +8,7 @@ trait MyTrait { impl MyTrait for i32 { fn foo(&self) -> i32 { - //~^ ERROR: `i32` is not a future + //~^ ERROR: method should be `async` or return a future, but it is synchronous *self } } diff --git a/tests/ui/async-await/in-trait/fn-not-async-err.stderr b/tests/ui/async-await/in-trait/fn-not-async-err.stderr index dc46077d583af..8260cd5271ee9 100644 --- a/tests/ui/async-await/in-trait/fn-not-async-err.stderr +++ b/tests/ui/async-await/in-trait/fn-not-async-err.stderr @@ -1,17 +1,14 @@ -error[E0277]: `i32` is not a future - --> $DIR/fn-not-async-err.rs:10:22 +error: method should be `async` or return a future, but it is synchronous + --> $DIR/fn-not-async-err.rs:10:5 | LL | fn foo(&self) -> i32 { - | ^^^ `i32` is not a future + | ^^^^^^^^^^^^^^^^^^^^ | - = help: the trait `Future` is not implemented for `i32` - = note: i32 must be a future or must implement `IntoFuture` to be awaited -note: required by a bound in `MyTrait::{opaque#0}` +note: this method is `async` so it expects a future to be returned --> $DIR/fn-not-async-err.rs:6:5 | LL | async fn foo(&self) -> i32; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `MyTrait::{opaque#0}` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 1 previous error -For more information about this error, try `rustc --explain E0277`. From 1a3214b774f47b2fb8dadf305939f97e20993fe2 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 18 Jan 2024 17:28:37 +0000 Subject: [PATCH 11/16] Make sure refinement still works --- .../in-trait/async-example-desugared-boxed.rs | 5 ++++- .../async-example-desugared-boxed.stderr | 22 +++++++++++++++++++ .../async-example-desugared-manual.rs | 7 ++++-- .../async-example-desugared-manual.stderr | 22 +++++++++++++++++++ 4 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr create mode 100644 tests/ui/async-await/in-trait/async-example-desugared-manual.stderr diff --git a/tests/ui/async-await/in-trait/async-example-desugared-boxed.rs b/tests/ui/async-await/in-trait/async-example-desugared-boxed.rs index 4008e09998e47..69871d0dca01d 100644 --- a/tests/ui/async-await/in-trait/async-example-desugared-boxed.rs +++ b/tests/ui/async-await/in-trait/async-example-desugared-boxed.rs @@ -4,12 +4,15 @@ use std::future::Future; use std::pin::Pin; -trait MyTrait { +#[allow(async_fn_in_trait)] +pub trait MyTrait { async fn foo(&self) -> i32; } impl MyTrait for i32 { + #[warn(refining_impl_trait)] fn foo(&self) -> Pin + '_>> { + //~^ WARN impl trait in impl method signature does not match trait method signature Box::pin(async { *self }) } } diff --git a/tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr b/tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr new file mode 100644 index 0000000000000..54aba77cc05d0 --- /dev/null +++ b/tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr @@ -0,0 +1,22 @@ +warning: impl trait in impl method signature does not match trait method signature + --> $DIR/async-example-desugared-boxed.rs:14:22 + | +LL | async fn foo(&self) -> i32; + | --------------------------- return type from trait method defined here +... +LL | fn foo(&self) -> Pin + '_>> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate +note: the lint level is defined here + --> $DIR/async-example-desugared-boxed.rs:13:12 + | +LL | #[warn(refining_impl_trait)] + | ^^^^^^^^^^^^^^^^^^^ +help: replace the return type so that it matches the trait + | +LL | fn foo(&self) -> impl Future { + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +warning: 1 warning emitted + diff --git a/tests/ui/async-await/in-trait/async-example-desugared-manual.rs b/tests/ui/async-await/in-trait/async-example-desugared-manual.rs index 75f4ba1d076c6..c6e8f1ae90607 100644 --- a/tests/ui/async-await/in-trait/async-example-desugared-manual.rs +++ b/tests/ui/async-await/in-trait/async-example-desugared-manual.rs @@ -4,11 +4,12 @@ use std::future::Future; use std::task::Poll; -trait MyTrait { +#[allow(async_fn_in_trait)] +pub trait MyTrait { async fn foo(&self) -> i32; } -struct MyFuture; +pub struct MyFuture; impl Future for MyFuture { type Output = i32; fn poll(self: std::pin::Pin<&mut Self>, _: &mut std::task::Context<'_>) -> Poll { @@ -17,7 +18,9 @@ impl Future for MyFuture { } impl MyTrait for u32 { + #[warn(refining_impl_trait)] fn foo(&self) -> MyFuture { + //~^ WARN impl trait in impl method signature does not match trait method signature MyFuture } } diff --git a/tests/ui/async-await/in-trait/async-example-desugared-manual.stderr b/tests/ui/async-await/in-trait/async-example-desugared-manual.stderr new file mode 100644 index 0000000000000..d94afd92c5691 --- /dev/null +++ b/tests/ui/async-await/in-trait/async-example-desugared-manual.stderr @@ -0,0 +1,22 @@ +warning: impl trait in impl method signature does not match trait method signature + --> $DIR/async-example-desugared-manual.rs:22:22 + | +LL | async fn foo(&self) -> i32; + | --------------------------- return type from trait method defined here +... +LL | fn foo(&self) -> MyFuture { + | ^^^^^^^^ + | + = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate +note: the lint level is defined here + --> $DIR/async-example-desugared-manual.rs:21:12 + | +LL | #[warn(refining_impl_trait)] + | ^^^^^^^^^^^^^^^^^^^ +help: replace the return type so that it matches the trait + | +LL | fn foo(&self) -> impl Future { + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +warning: 1 warning emitted + From 411967c0786495d750a60d1cb67087471bc3684f Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 31 Jan 2024 02:37:44 +0100 Subject: [PATCH 12/16] Zip together `place_ty` and `place_validity` --- .../rustc_pattern_analysis/src/usefulness.rs | 79 +++++++++++-------- 1 file changed, 46 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index bbb68b353e436..246b15ad7d013 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -767,12 +767,6 @@ impl<'a, Cx: TypeCx> PlaceCtxt<'a, Cx> { fn ctor_arity(&self, ctor: &Constructor) -> usize { self.cx.ctor_arity(ctor, self.ty) } - fn ctor_sub_tys( - &'a self, - ctor: &'a Constructor, - ) -> impl Iterator + ExactSizeIterator + Captures<'a> { - self.cx.ctor_sub_tys(ctor, self.ty) - } fn ctors_for_ty(&self) -> Result, Cx::Error> { self.cx.ctors_for_ty(self.ty) } @@ -828,6 +822,32 @@ impl fmt::Display for ValidityConstraint { } } +/// Data about a place under investigation. +struct PlaceInfo { + /// The type of the place. + ty: Cx::Ty, + /// Whether the place is known to contain valid data. + validity: ValidityConstraint, +} + +impl PlaceInfo { + fn specialize<'a>( + &'a self, + cx: &'a Cx, + ctor: &'a Constructor, + ) -> impl Iterator + ExactSizeIterator + Captures<'a> { + let ctor_sub_tys = cx.ctor_sub_tys(ctor, &self.ty); + let ctor_sub_validity = self.validity.specialize(ctor); + ctor_sub_tys.map(move |ty| PlaceInfo { ty, validity: ctor_sub_validity }) + } +} + +impl Clone for PlaceInfo { + fn clone(&self) -> Self { + Self { ty: self.ty.clone(), validity: self.validity } + } +} + /// Represents a pattern-tuple under investigation. // The three lifetimes are: // - 'p coming from the input @@ -1001,10 +1021,9 @@ struct Matrix<'p, Cx: TypeCx> { /// each column must have the same type. Each column corresponds to a place within the /// scrutinee. rows: Vec>, - /// Track the type of each column/place. - place_ty: SmallVec<[Cx::Ty; 2]>, - /// Track for each column/place whether it contains a known valid value. - place_validity: SmallVec<[ValidityConstraint; 2]>, + /// Track info about each place. Each place corresponds to a column in `rows`, and their types + /// must match. + place_info: SmallVec<[PlaceInfo; 2]>, /// Track whether the virtual wildcard row used to compute exhaustiveness is relevant. See top /// of the file for details on relevancy. wildcard_row_is_relevant: bool, @@ -1032,10 +1051,10 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> { scrut_ty: Cx::Ty, scrut_validity: ValidityConstraint, ) -> Self { + let place_info = PlaceInfo { ty: scrut_ty, validity: scrut_validity }; let mut matrix = Matrix { rows: Vec::with_capacity(arms.len()), - place_ty: smallvec![scrut_ty], - place_validity: smallvec![scrut_validity], + place_info: smallvec![place_info], wildcard_row_is_relevant: true, }; for (row_id, arm) in arms.iter().enumerate() { @@ -1051,11 +1070,11 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> { matrix } - fn head_ty(&self) -> Option<&Cx::Ty> { - self.place_ty.first() + fn head_place(&self) -> Option<&PlaceInfo> { + self.place_info.first() } fn column_count(&self) -> usize { - self.place_ty.len() + self.place_info.len() } fn rows( @@ -1083,18 +1102,13 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> { ctor: &Constructor, ctor_is_relevant: bool, ) -> Result, Cx::Error> { - let ctor_sub_tys = pcx.ctor_sub_tys(ctor); - let arity = ctor_sub_tys.len(); - let specialized_place_ty = ctor_sub_tys.chain(self.place_ty[1..].iter().cloned()).collect(); - let ctor_sub_validity = self.place_validity[0].specialize(ctor); - let specialized_place_validity = std::iter::repeat(ctor_sub_validity) - .take(arity) - .chain(self.place_validity[1..].iter().copied()) - .collect(); + let subfield_place_info = self.place_info[0].specialize(pcx.cx, ctor); + let arity = subfield_place_info.len(); + let specialized_place_info = + subfield_place_info.chain(self.place_info[1..].iter().cloned()).collect(); let mut matrix = Matrix { rows: Vec::new(), - place_ty: specialized_place_ty, - place_validity: specialized_place_validity, + place_info: specialized_place_info, wildcard_row_is_relevant: self.wildcard_row_is_relevant && ctor_is_relevant, }; for (i, row) in self.rows().enumerate() { @@ -1127,11 +1141,11 @@ impl<'p, Cx: TypeCx> fmt::Debug for Matrix<'p, Cx> { .map(|row| row.iter().map(|pat| format!("{pat:?}")).collect()) .collect(); pretty_printed_matrix - .push(self.place_validity.iter().map(|validity| format!("{validity}")).collect()); + .push(self.place_info.iter().map(|place| format!("{}", place.validity)).collect()); let column_count = self.column_count(); assert!(self.rows.iter().all(|row| row.len() == column_count)); - assert!(self.place_validity.len() == column_count); + assert!(self.place_info.len() == column_count); let column_widths: Vec = (0..column_count) .map(|col| pretty_printed_matrix.iter().map(|row| row[col].len()).max().unwrap_or(0)) .collect(); @@ -1447,7 +1461,7 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( return Ok(WitnessMatrix::empty()); } - let Some(ty) = matrix.head_ty().cloned() else { + let Some(place) = matrix.head_place() else { // The base case: there are no columns in the matrix. We are morally pattern-matching on (). // A row is useful iff it has no (unguarded) rows above it. let mut useful = true; // Whether the next row is useful. @@ -1467,18 +1481,17 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( }; }; - debug!("ty: {ty:?}"); - let pcx = &PlaceCtxt { cx: mcx.tycx, ty: &ty }; + let ty = &place.ty.clone(); // Clone it out so we can mutate `matrix` later. + let pcx = &PlaceCtxt { cx: mcx.tycx, ty }; + debug!("ty: {:?}", pcx.ty); let ctors_for_ty = pcx.ctors_for_ty()?; - // Whether the place/column we are inspecting is known to contain valid data. - let place_validity = matrix.place_validity[0]; // We treat match scrutinees of type `!` or `EmptyEnum` differently. let is_toplevel_exception = is_top_level && matches!(ctors_for_ty, ConstructorSet::NoConstructors); // Whether empty patterns are counted as useful or not. We only warn an empty arm unreachable if // it is guaranteed unreachable by the opsem (i.e. if the place is `known_valid`). - let empty_arms_are_unreachable = place_validity.is_known_valid() + let empty_arms_are_unreachable = place.validity.is_known_valid() && (is_toplevel_exception || mcx.tycx.is_exhaustive_patterns_feature_on() || mcx.tycx.is_min_exhaustive_patterns_feature_on()); From 6cac1c459ec1f2aa7dd31e9b1b90040c906c64f5 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 31 Jan 2024 02:46:10 +0100 Subject: [PATCH 13/16] Track `is_top_level` via `PlaceInfo` --- .../rustc_pattern_analysis/src/usefulness.rs | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 246b15ad7d013..1ac984ce67eec 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -828,6 +828,8 @@ struct PlaceInfo { ty: Cx::Ty, /// Whether the place is known to contain valid data. validity: ValidityConstraint, + /// Whether the place is the scrutinee itself or a subplace of it. + is_scrutinee: bool, } impl PlaceInfo { @@ -838,13 +840,17 @@ impl PlaceInfo { ) -> impl Iterator + ExactSizeIterator + Captures<'a> { let ctor_sub_tys = cx.ctor_sub_tys(ctor, &self.ty); let ctor_sub_validity = self.validity.specialize(ctor); - ctor_sub_tys.map(move |ty| PlaceInfo { ty, validity: ctor_sub_validity }) + ctor_sub_tys.map(move |ty| PlaceInfo { + ty, + validity: ctor_sub_validity, + is_scrutinee: false, + }) } } impl Clone for PlaceInfo { fn clone(&self) -> Self { - Self { ty: self.ty.clone(), validity: self.validity } + Self { ty: self.ty.clone(), validity: self.validity, is_scrutinee: self.is_scrutinee } } } @@ -1051,7 +1057,7 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> { scrut_ty: Cx::Ty, scrut_validity: ValidityConstraint, ) -> Self { - let place_info = PlaceInfo { ty: scrut_ty, validity: scrut_validity }; + let place_info = PlaceInfo { ty: scrut_ty, validity: scrut_validity, is_scrutinee: true }; let mut matrix = Matrix { rows: Vec::with_capacity(arms.len()), place_info: smallvec![place_info], @@ -1446,11 +1452,10 @@ fn collect_overlapping_range_endpoints<'p, Cx: TypeCx>( /// - unspecialization, where we lift the results from the previous step into results for this step /// (using `apply_constructor` and by updating `row.useful` for each parent row). /// This is all explained at the top of the file. -#[instrument(level = "debug", skip(mcx, is_top_level), ret)] +#[instrument(level = "debug", skip(mcx), ret)] fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( mcx: UsefulnessCtxt<'a, Cx>, matrix: &mut Matrix<'p, Cx>, - is_top_level: bool, ) -> Result, Cx::Error> { debug_assert!(matrix.rows().all(|r| r.len() == matrix.column_count())); @@ -1488,7 +1493,7 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( // We treat match scrutinees of type `!` or `EmptyEnum` differently. let is_toplevel_exception = - is_top_level && matches!(ctors_for_ty, ConstructorSet::NoConstructors); + place.is_scrutinee && matches!(ctors_for_ty, ConstructorSet::NoConstructors); // Whether empty patterns are counted as useful or not. We only warn an empty arm unreachable if // it is guaranteed unreachable by the opsem (i.e. if the place is `known_valid`). let empty_arms_are_unreachable = place.validity.is_known_valid() @@ -1517,7 +1522,7 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( // Decide what constructors to report. let is_integers = matches!(ctors_for_ty, ConstructorSet::Integers { .. }); - let always_report_all = is_top_level && !is_integers; + let always_report_all = place.is_scrutinee && !is_integers; // Whether we should report "Enum::A and Enum::C are missing" or "_ is missing". let report_individual_missing_ctors = always_report_all || !all_missing; // Which constructors are considered missing. We ensure that `!missing_ctors.is_empty() => @@ -1538,7 +1543,7 @@ fn compute_exhaustiveness_and_usefulness<'a, 'p, Cx: TypeCx>( let ctor_is_relevant = matches!(ctor, Constructor::Missing) || missing_ctors.is_empty(); let mut spec_matrix = matrix.specialize_constructor(pcx, &ctor, ctor_is_relevant)?; let mut witnesses = ensure_sufficient_stack(|| { - compute_exhaustiveness_and_usefulness(mcx, &mut spec_matrix, false) + compute_exhaustiveness_and_usefulness(mcx, &mut spec_matrix) })?; // Transform witnesses for `spec_matrix` into witnesses for `matrix`. @@ -1613,8 +1618,7 @@ pub fn compute_match_usefulness<'p, Cx: TypeCx>( ) -> Result, Cx::Error> { let cx = UsefulnessCtxt { tycx }; let mut matrix = Matrix::new(arms, scrut_ty, scrut_validity); - let non_exhaustiveness_witnesses = - compute_exhaustiveness_and_usefulness(cx, &mut matrix, true)?; + let non_exhaustiveness_witnesses = compute_exhaustiveness_and_usefulness(cx, &mut matrix)?; let non_exhaustiveness_witnesses: Vec<_> = non_exhaustiveness_witnesses.single_column(); let arm_usefulness: Vec<_> = arms From a2ab48c21b41bd3c842928d84c881b8b780648e4 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 4 Jan 2024 16:57:24 +0300 Subject: [PATCH 14/16] resolve: Unload speculatively resolved crates before freezing cstore --- compiler/rustc_metadata/src/creader.rs | 15 ++++++++++++- compiler/rustc_metadata/src/rmeta/decoder.rs | 7 +++++++ .../src/rmeta/decoder/cstore_impl.rs | 15 ++++++++++++- compiler/rustc_resolve/src/late.rs | 21 ++++++++++++------- compiler/rustc_resolve/src/lib.rs | 1 + tests/ui/extern-flag/empty-extern-arg.stderr | 9 ++------ 6 files changed, 52 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index bb02a8a1e4743..c18f0e7b7b938 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -534,7 +534,10 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { ) -> Option { self.used_extern_options.insert(name); match self.maybe_resolve_crate(name, dep_kind, None) { - Ok(cnum) => Some(cnum), + Ok(cnum) => { + self.cstore.set_used_recursively(cnum); + Some(cnum) + } Err(err) => { let missing_core = self.maybe_resolve_crate(sym::core, CrateDepKind::Explicit, None).is_err(); @@ -1067,6 +1070,16 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { pub fn maybe_process_path_extern(&mut self, name: Symbol) -> Option { self.maybe_resolve_crate(name, CrateDepKind::Explicit, None).ok() } + + pub fn unload_unused_crates(&mut self) { + for opt_cdata in &mut self.cstore.metas { + if let Some(cdata) = opt_cdata + && !cdata.used() + { + *opt_cdata = None; + } + } + } } fn global_allocator_spans(krate: &ast::Crate) -> Vec { diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 20e3ae3ba9492..11cb1bb6d9e6e 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -106,6 +106,8 @@ pub(crate) struct CrateMetadata { private_dep: bool, /// The hash for the host proc macro. Used to support `-Z dual-proc-macro`. host_hash: Option, + /// The crate was used non-speculatively. + used: bool, /// Additional data used for decoding `HygieneData` (e.g. `SyntaxContext` /// and `ExpnId`). @@ -1811,6 +1813,7 @@ impl CrateMetadata { source: Lrc::new(source), private_dep, host_hash, + used: false, extern_crate: None, hygiene_context: Default::default(), def_key_cache: Default::default(), @@ -1860,6 +1863,10 @@ impl CrateMetadata { self.private_dep &= private_dep; } + pub(crate) fn used(&self) -> bool { + self.used + } + pub(crate) fn required_panic_strategy(&self) -> Option { self.root.required_panic_strategy } diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 0b352a02b64c7..7cd2f58779f84 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -26,6 +26,7 @@ use rustc_span::symbol::{kw, Symbol}; use rustc_span::Span; use std::any::Any; +use std::mem; use super::{Decodable, DecodeContext, DecodeIterator}; @@ -576,12 +577,24 @@ impl CStore { self.get_crate_data(cnum).get_proc_macro_quoted_span(id, sess) } + pub fn set_used_recursively(&mut self, cnum: CrateNum) { + let cmeta = self.get_crate_data_mut(cnum); + if !cmeta.used { + cmeta.used = true; + let dependencies = mem::take(&mut cmeta.dependencies); + for &dep_cnum in &dependencies { + self.set_used_recursively(dep_cnum); + } + self.get_crate_data_mut(cnum).dependencies = dependencies; + } + } + pub(crate) fn update_extern_crate(&mut self, cnum: CrateNum, extern_crate: ExternCrate) { let cmeta = self.get_crate_data_mut(cnum); if cmeta.update_extern_crate(extern_crate) { // Propagate the extern crate info to dependencies if it was updated. let extern_crate = ExternCrate { dependency_of: cnum, ..extern_crate }; - let dependencies = std::mem::take(&mut cmeta.dependencies); + let dependencies = mem::take(&mut cmeta.dependencies); for &dep_cnum in &dependencies { self.update_extern_crate(dep_cnum, extern_crate); } diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 1f2803d4368ae..c020c134b770c 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -23,6 +23,7 @@ use rustc_hir::def::Namespace::{self, *}; use rustc_hir::def::{self, CtorKind, DefKind, LifetimeRes, NonMacroAttrKind, PartialRes, PerNS}; use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID, LOCAL_CRATE}; use rustc_hir::{BindingAnnotation, PrimTy, TraitCandidate}; +use rustc_metadata::creader::CStore; use rustc_middle::middle::resolve_bound_vars::Set1; use rustc_middle::{bug, span_bug}; use rustc_session::config::{CrateType, ResolveDocLinks}; @@ -4541,14 +4542,20 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { if let Some(res) = res && let Some(def_id) = res.opt_def_id() && !def_id.is_local() - && self.r.tcx.crate_types().contains(&CrateType::ProcMacro) - && matches!( - self.r.tcx.sess.opts.resolve_doc_links, - ResolveDocLinks::ExportedMetadata - ) { - // Encoding foreign def ids in proc macro crate metadata will ICE. - return None; + if self.r.tcx.crate_types().contains(&CrateType::ProcMacro) + && matches!( + self.r.tcx.sess.opts.resolve_doc_links, + ResolveDocLinks::ExportedMetadata + ) + { + // Encoding foreign def ids in proc macro crate metadata will ICE. + return None; + } + // Doc paths should be resolved speculatively and should not produce any + // diagnostics, but if they are indeed resolved, then we need to keep the + // corresponding crate alive. + CStore::from_tcx_mut(self.r.tcx).set_used_recursively(def_id.krate); } res }); diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 9d09d060b59b8..6ca9236075b48 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -1625,6 +1625,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { self.tcx .sess .time("resolve_postprocess", || self.crate_loader(|c| c.postprocess(krate))); + self.crate_loader(|c| c.unload_unused_crates()); }); // Make sure we don't mutate the cstore from here on. diff --git a/tests/ui/extern-flag/empty-extern-arg.stderr b/tests/ui/extern-flag/empty-extern-arg.stderr index 79efcc5d8b041..6ad3effe0e26e 100644 --- a/tests/ui/extern-flag/empty-extern-arg.stderr +++ b/tests/ui/extern-flag/empty-extern-arg.stderr @@ -1,11 +1,6 @@ error: extern location for std does not exist: -error: `#[panic_handler]` function required, but not found +error: requires `sized` lang_item -error: unwinding panics are not supported without std - | - = help: using nightly cargo, use -Zbuild-std with panic="abort" to avoid unwinding - = note: since the core library is usually precompiled with panic="unwind", rebuilding your crate with panic="abort" may not be enough to fix the problem - -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors From aef18c9bfb8c7844b95f6d950133f61b0fa2158b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 29 Jan 2024 18:54:29 +0000 Subject: [PATCH 15/16] Mark "unused binding" suggestion as maybe incorrect Ignoring unused bindings should be a determination made by a human, `rustfix` shouldn't auto-apply the suggested change. Fix #54196. --- compiler/rustc_passes/src/errors.rs | 4 ++-- tests/ui/lint/future-incompat-json-test.stderr | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 77bfe57e3706f..5db2369ad76c2 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -1739,7 +1739,7 @@ pub struct UnusedVariableTryPrefix { #[derive(Subdiagnostic)] pub enum UnusedVariableSugg { - #[multipart_suggestion(passes_suggestion, applicability = "machine-applicable")] + #[multipart_suggestion(passes_suggestion, applicability = "maybe-incorrect")] TryPrefixSugg { #[suggestion_part(code = "_{name}")] spans: Vec, @@ -1778,7 +1778,7 @@ pub struct UnusedVarTryIgnore { } #[derive(Subdiagnostic)] -#[multipart_suggestion(passes_suggestion, applicability = "machine-applicable")] +#[multipart_suggestion(passes_suggestion, applicability = "maybe-incorrect")] pub struct UnusedVarTryIgnoreSugg { #[suggestion_part(code = "{name}: _")] pub shorthands: Vec, diff --git a/tests/ui/lint/future-incompat-json-test.stderr b/tests/ui/lint/future-incompat-json-test.stderr index c4ab5a00d16cc..18fc3f17f0020 100644 --- a/tests/ui/lint/future-incompat-json-test.stderr +++ b/tests/ui/lint/future-incompat-json-test.stderr @@ -1,4 +1,4 @@ -{"$message_type":"future_incompat","future_incompat_report":[{"diagnostic":{"$message_type":"diagnostic","message":"unused variable: `x`","code":{"code":"unused_variables","explanation":null},"level":"warning","spans":[{"file_name":"$DIR/future-incompat-json-test.rs","byte_start":338,"byte_end":339,"line_start":9,"line_end":9,"column_start":9,"column_end":10,"is_primary":true,"text":[{"text":" let x = 1;","highlight_start":9,"highlight_end":10}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"`-A unused-variables` implied by `-A unused`","code":null,"level":"note","spans":[],"children":[],"rendered":null},{"message":"to override `-A unused` add `#[allow(unused_variables)]`","code":null,"level":"help","spans":[],"children":[],"rendered":null},{"message":"if this is intentional, prefix it with an underscore","code":null,"level":"help","spans":[{"file_name":"$DIR/future-incompat-json-test.rs","byte_start":338,"byte_end":339,"line_start":9,"line_end":9,"column_start":9,"column_end":10,"is_primary":true,"text":[{"text":" let x = 1;","highlight_start":9,"highlight_end":10}],"label":null,"suggested_replacement":"_x","suggestion_applicability":"MachineApplicable","expansion":null}],"children":[],"rendered":null}],"rendered":"warning: unused variable: `x` +{"$message_type":"future_incompat","future_incompat_report":[{"diagnostic":{"$message_type":"diagnostic","message":"unused variable: `x`","code":{"code":"unused_variables","explanation":null},"level":"warning","spans":[{"file_name":"$DIR/future-incompat-json-test.rs","byte_start":338,"byte_end":339,"line_start":9,"line_end":9,"column_start":9,"column_end":10,"is_primary":true,"text":[{"text":" let x = 1;","highlight_start":9,"highlight_end":10}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"`-A unused-variables` implied by `-A unused`","code":null,"level":"note","spans":[],"children":[],"rendered":null},{"message":"to override `-A unused` add `#[allow(unused_variables)]`","code":null,"level":"help","spans":[],"children":[],"rendered":null},{"message":"if this is intentional, prefix it with an underscore","code":null,"level":"help","spans":[{"file_name":"$DIR/future-incompat-json-test.rs","byte_start":338,"byte_end":339,"line_start":9,"line_end":9,"column_start":9,"column_end":10,"is_primary":true,"text":[{"text":" let x = 1;","highlight_start":9,"highlight_end":10}],"label":null,"suggested_replacement":"_x","suggestion_applicability":"MaybeIncorrect","expansion":null}],"children":[],"rendered":null}],"rendered":"warning: unused variable: `x` --> $DIR/future-incompat-json-test.rs:9:9 | LL | let x = 1; From 14dda5fdfb81e61b2acc85f27609f879aeaa166d Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Tue, 6 Feb 2024 21:03:07 -0500 Subject: [PATCH 16/16] Don't use bashism in checktools.sh --- src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh b/src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh index cc0c658aabd2b..f5c426a717f78 100755 --- a/src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh +++ b/src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh @@ -30,7 +30,7 @@ python3 "$X_PY" test --stage 2 src/tools/rustfmt # We set the GC interval to the shortest possible value (0 would be off) to increase the chance # that bugs which only surface when the GC runs at a specific time are more likely to cause CI to fail. # This significantly increases the runtime of our test suite, or we'd do this in PR CI too. -if [[ -z "${PR_CI_JOB:-}" ]]; then +if [ -z "${PR_CI_JOB:-}" ]; then MIRIFLAGS=-Zmiri-provenance-gc=1 python3 "$X_PY" test --stage 2 src/tools/miri else python3 "$X_PY" test --stage 2 src/tools/miri