From 8e91a51cd81de5cf686d21db891634fecdc250c0 Mon Sep 17 00:00:00 2001 From: Matthias Geier Date: Tue, 2 Apr 2024 18:54:24 +0200 Subject: [PATCH 01/13] DOC: Add FFI example for slice::from_raw_parts() --- library/core/src/slice/raw.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/library/core/src/slice/raw.rs b/library/core/src/slice/raw.rs index 29a12f106c5ed..d001688d79d80 100644 --- a/library/core/src/slice/raw.rs +++ b/library/core/src/slice/raw.rs @@ -83,6 +83,27 @@ use crate::ub_checks; /// } /// ``` /// +/// ### FFI: Handling null pointers +/// +/// In languages such as C++, pointers to empty collections are not guaranteed to be non-null. +/// When accepting such pointers, they have to be checked for null-ness to avoid undefined +/// behavior. +/// +/// ``` +/// use std::slice; +/// +/// unsafe extern "C" fn handle_slice(ptr: *const f32, len: usize) { +/// let data = if ptr.is_null() { +/// // `len` is assumed to be 0. +/// &[] +/// } else { +/// unsafe { slice::from_raw_parts(ptr, len) } +/// }; +/// dbg!(data); +/// // ... +/// } +/// ``` +/// /// [valid]: ptr#safety /// [`NonNull::dangling()`]: ptr::NonNull::dangling #[inline] From 22319bf6ba861eb0b5efb81aaf7616db30f4872f Mon Sep 17 00:00:00 2001 From: Matthias Geier Date: Fri, 26 Apr 2024 20:44:59 +0200 Subject: [PATCH 02/13] Add "safety" comment --- library/core/src/slice/raw.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/library/core/src/slice/raw.rs b/library/core/src/slice/raw.rs index d001688d79d80..8ff429218a4df 100644 --- a/library/core/src/slice/raw.rs +++ b/library/core/src/slice/raw.rs @@ -92,11 +92,16 @@ use crate::ub_checks; /// ``` /// use std::slice; /// +/// /// # Safety +/// /// +/// /// If ptr is not NULL, it must be correctly aligned and +/// /// point to `len` initialized items of type `f32`. /// unsafe extern "C" fn handle_slice(ptr: *const f32, len: usize) { /// let data = if ptr.is_null() { /// // `len` is assumed to be 0. /// &[] /// } else { +/// // SAFETY: see function docstring. /// unsafe { slice::from_raw_parts(ptr, len) } /// }; /// dbg!(data); From 4df1303cff1eeb3b0c7f00d70fecf160279e7263 Mon Sep 17 00:00:00 2001 From: Matthias Geier Date: Fri, 26 Apr 2024 20:53:23 +0200 Subject: [PATCH 03/13] Extend the example code and assert the result --- library/core/src/slice/raw.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/library/core/src/slice/raw.rs b/library/core/src/slice/raw.rs index 8ff429218a4df..efdd210c1fac8 100644 --- a/library/core/src/slice/raw.rs +++ b/library/core/src/slice/raw.rs @@ -92,11 +92,13 @@ use crate::ub_checks; /// ``` /// use std::slice; /// +/// /// Sum the elements of an FFI slice. +/// /// /// /// # Safety /// /// /// /// If ptr is not NULL, it must be correctly aligned and /// /// point to `len` initialized items of type `f32`. -/// unsafe extern "C" fn handle_slice(ptr: *const f32, len: usize) { +/// unsafe extern "C" fn sum_slice(ptr: *const f32, len: usize) -> f32 { /// let data = if ptr.is_null() { /// // `len` is assumed to be 0. /// &[] @@ -104,9 +106,14 @@ use crate::ub_checks; /// // SAFETY: see function docstring. /// unsafe { slice::from_raw_parts(ptr, len) } /// }; -/// dbg!(data); -/// // ... +/// data.sum() /// } +/// +/// // This could be the result of C++'s std::vector::data(): +/// let ptr = std::ptr::null(); +/// // And this could be std::vector::size(): +/// let len = 0; +/// assert_eq!(unsafe { sum_slice(ptr, len) }, 0.0); /// ``` /// /// [valid]: ptr#safety From 30b676cc00324c16963ef529ba37b49e0200ac5f Mon Sep 17 00:00:00 2001 From: Matthias Geier Date: Fri, 26 Apr 2024 21:18:00 +0200 Subject: [PATCH 04/13] Add missing .into_iter() --- library/core/src/slice/raw.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/slice/raw.rs b/library/core/src/slice/raw.rs index efdd210c1fac8..a42c2cc4e63a9 100644 --- a/library/core/src/slice/raw.rs +++ b/library/core/src/slice/raw.rs @@ -106,7 +106,7 @@ use crate::ub_checks; /// // SAFETY: see function docstring. /// unsafe { slice::from_raw_parts(ptr, len) } /// }; -/// data.sum() +/// data.into_iter().sum() /// } /// /// // This could be the result of C++'s std::vector::data(): From fe52b5439edd024b1402f4790bc8e12e0fd91d34 Mon Sep 17 00:00:00 2001 From: binarycat Date: Sun, 9 Jun 2024 20:22:46 -0400 Subject: [PATCH 05/13] docs(core): make more const_ptr doctests assert instead of printing fixes #124669 --- library/core/src/ptr/const_ptr.rs | 38 ++++++++++++++++++------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index 8c02aee8bfb40..b1f94caed3513 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -330,7 +330,7 @@ impl *const T { /// /// unsafe { /// if let Some(val_back) = ptr.as_ref() { - /// println!("We got back the value: {val_back}!"); + /// assert_eq!(val_back, &10); /// } /// } /// ``` @@ -346,7 +346,7 @@ impl *const T { /// /// unsafe { /// let val_back = &*ptr; - /// println!("We got back the value: {val_back}!"); + /// assert_eq!(val_back, &10); /// } /// ``` #[stable(feature = "ptr_as_ref", since = "1.9.0")] @@ -393,7 +393,7 @@ impl *const T { /// let ptr: *const u8 = &10u8 as *const u8; /// /// unsafe { - /// println!("We got back the value: {}!", ptr.as_ref_unchecked()); + /// assert_eq!(ptr.as_ref_unchecked(), &10); /// } /// ``` // FIXME: mention it in the docs for `as_ref` and `as_uninit_ref` once stabilized. @@ -439,7 +439,7 @@ impl *const T { /// /// unsafe { /// if let Some(val_back) = ptr.as_uninit_ref() { - /// println!("We got back the value: {}!", val_back.assume_init()); + /// assert_eq!(val_back.assume_init(), 10); /// } /// } /// ``` @@ -501,8 +501,8 @@ impl *const T { /// let ptr: *const u8 = s.as_ptr(); /// /// unsafe { - /// println!("{}", *ptr.offset(1) as char); - /// println!("{}", *ptr.offset(2) as char); + /// assert_eq!(*ptr.offset(1) as char, '2'); + /// assert_eq!(*ptr.offset(2) as char, '3'); /// } /// ``` #[stable(feature = "rust1", since = "1.0.0")] @@ -573,19 +573,21 @@ impl *const T { /// # Examples /// /// ``` + /// # use std::fmt::Write; /// // Iterate using a raw pointer in increments of two elements /// let data = [1u8, 2, 3, 4, 5]; /// let mut ptr: *const u8 = data.as_ptr(); /// let step = 2; /// let end_rounded_up = ptr.wrapping_offset(6); /// - /// // This loop prints "1, 3, 5, " + /// let mut out = String::new(); /// while ptr != end_rounded_up { /// unsafe { - /// print!("{}, ", *ptr); + /// write!(&mut out, "{}, ", *ptr).unwrap(); /// } /// ptr = ptr.wrapping_offset(step); /// } + /// assert_eq!(out.as_str(), "1, 3, 5, "); /// ``` #[stable(feature = "ptr_wrapping_offset", since = "1.16.0")] #[must_use = "returns a new pointer rather than modifying its argument"] @@ -988,8 +990,8 @@ impl *const T { /// let ptr: *const u8 = s.as_ptr(); /// /// unsafe { - /// println!("{}", *ptr.add(1) as char); - /// println!("{}", *ptr.add(2) as char); + /// assert_eq!(*ptr.add(1), b'2'); + /// assert_eq!(*ptr.add(2), b'3'); /// } /// ``` #[stable(feature = "pointer_methods", since = "1.26.0")] @@ -1073,8 +1075,8 @@ impl *const T { /// /// unsafe { /// let end: *const u8 = s.as_ptr().add(3); - /// println!("{}", *end.sub(1) as char); - /// println!("{}", *end.sub(2) as char); + /// assert_eq!(*end.sub(1), b'3'); + /// assert_eq!(*end.sub(2), b'2'); /// } /// ``` #[stable(feature = "pointer_methods", since = "1.26.0")] @@ -1155,19 +1157,21 @@ impl *const T { /// # Examples /// /// ``` + /// # use std::fmt::Write; /// // Iterate using a raw pointer in increments of two elements /// let data = [1u8, 2, 3, 4, 5]; /// let mut ptr: *const u8 = data.as_ptr(); /// let step = 2; /// let end_rounded_up = ptr.wrapping_add(6); /// - /// // This loop prints "1, 3, 5, " + /// let mut out = String::new(); /// while ptr != end_rounded_up { /// unsafe { - /// print!("{}, ", *ptr); + /// write!(&mut out, "{}, ", *ptr).unwrap(); /// } /// ptr = ptr.wrapping_add(step); /// } + /// assert_eq!(out, "1, 3, 5, "); /// ``` #[stable(feature = "pointer_methods", since = "1.26.0")] #[must_use = "returns a new pointer rather than modifying its argument"] @@ -1234,19 +1238,21 @@ impl *const T { /// # Examples /// /// ``` + /// # use std::fmt::Write; /// // Iterate using a raw pointer in increments of two elements (backwards) /// let data = [1u8, 2, 3, 4, 5]; /// let mut ptr: *const u8 = data.as_ptr(); /// let start_rounded_down = ptr.wrapping_sub(2); /// ptr = ptr.wrapping_add(4); /// let step = 2; - /// // This loop prints "5, 3, 1, " + /// let mut out = String::new(); /// while ptr != start_rounded_down { /// unsafe { - /// print!("{}, ", *ptr); + /// write!(&mut out, "{}, ", *ptr).unwrap(); /// } /// ptr = ptr.wrapping_sub(step); /// } + /// assert_eq!(out, "5, 3, 1, "); /// ``` #[stable(feature = "pointer_methods", since = "1.26.0")] #[must_use = "returns a new pointer rather than modifying its argument"] From bfb7757c3c059d3bbcfbc604ab326fa777be13aa Mon Sep 17 00:00:00 2001 From: Boxy Date: Mon, 10 Jun 2024 14:32:50 +0100 Subject: [PATCH 06/13] Correct parent for nested anon consts --- .../src/collect/generics_of.rs | 26 +++++++++++++++---- .../repeat_expr_hack_gives_right_generics.rs | 20 ++++++++++++++ ...peat_expr_hack_gives_right_generics.stderr | 11 ++++++++ 3 files changed, 52 insertions(+), 5 deletions(-) create mode 100644 tests/ui/const-generics/repeat_expr_hack_gives_right_generics.rs create mode 100644 tests/ui/const-generics/repeat_expr_hack_gives_right_generics.stderr diff --git a/compiler/rustc_hir_analysis/src/collect/generics_of.rs b/compiler/rustc_hir_analysis/src/collect/generics_of.rs index abdf85ad707bc..303fa23dbc1e8 100644 --- a/compiler/rustc_hir_analysis/src/collect/generics_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/generics_of.rs @@ -11,6 +11,7 @@ use rustc_session::lint; use rustc_span::symbol::{kw, Symbol}; use rustc_span::Span; +#[instrument(level = "debug", skip(tcx))] pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics { use rustc_hir::*; @@ -66,7 +67,22 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics { // FIXME(#43408) always enable this once `lazy_normalization` is // stable enough and does not need a feature gate anymore. Node::AnonConst(_) => { - let parent_def_id = tcx.hir().get_parent_item(hir_id); + let parent_did = tcx.parent(def_id.to_def_id()); + + // We don't do this unconditionally because the `DefId` parent of an anon const + // might be an implicitly created closure during `async fn` desugaring. This would + // have the wrong generics. + // + // i.e. `async fn foo<'a>() { let a = [(); { 1 + 2 }]; bar().await() }` + // would implicitly have a closure in its body that would be the parent of + // the `{ 1 + 2 }` anon const. This closure's generics is simply a witness + // instead of `['a]`. + let parent_did = if let DefKind::AnonConst = tcx.def_kind(parent_did) { + parent_did + } else { + tcx.hir().get_parent_item(hir_id).to_def_id() + }; + debug!(?parent_did); let mut in_param_ty = false; for (_parent, node) in tcx.hir().parent_iter(hir_id) { @@ -121,7 +137,7 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics { // // This has some implications for how we get the predicates available to the anon const // see `explicit_predicates_of` for more information on this - let generics = tcx.generics_of(parent_def_id.to_def_id()); + let generics = tcx.generics_of(parent_did); let param_def_idx = generics.param_def_id_to_index[¶m_id.to_def_id()]; // In the above example this would be .params[..N#0] let own_params = generics.params_to(param_def_idx as usize, tcx).to_owned(); @@ -147,7 +163,7 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics { // // Note that we do not supply the parent generics when using // `min_const_generics`. - Some(parent_def_id.to_def_id()) + Some(parent_did) } } else { let parent_node = tcx.parent_hir_node(hir_id); @@ -159,7 +175,7 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics { Node::Expr(Expr { kind: ExprKind::Repeat(_, constant), .. }) if constant.hir_id() == hir_id => { - Some(parent_def_id.to_def_id()) + Some(parent_did) } // Exclude `GlobalAsm` here which cannot have generics. Node::Expr(&Expr { kind: ExprKind::InlineAsm(asm), .. }) @@ -171,7 +187,7 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics { _ => false, }) => { - Some(parent_def_id.to_def_id()) + Some(parent_did) } _ => None, } diff --git a/tests/ui/const-generics/repeat_expr_hack_gives_right_generics.rs b/tests/ui/const-generics/repeat_expr_hack_gives_right_generics.rs new file mode 100644 index 0000000000000..899db191ae7d7 --- /dev/null +++ b/tests/ui/const-generics/repeat_expr_hack_gives_right_generics.rs @@ -0,0 +1,20 @@ +// Given an anon const `a`: `{ N }` and some anon const `b` which references the +// first anon const: `{ [1; a] }`. `b` should not have any generics as it is not +// a simple `N` argument nor is it a repeat expr count. +// +// On the other hand `b` *is* a repeat expr count and so it should inherit its +// parents generics as part of the `const_evaluatable_unchecked` fcw (#76200). +// +// In this specific case however `b`'s parent should be `a` and so it should wind +// up not having any generics after all. If `a` were to inherit its generics from +// the enclosing item then the reference to `a` from `b` would contain generic +// parameters not usable by `b` which would cause us to ICE. + +fn bar() {} + +fn foo() { + bar::<{ [1; N] }>(); + //~^ ERROR: generic parameters may not be used in const operations +} + +fn main() {} diff --git a/tests/ui/const-generics/repeat_expr_hack_gives_right_generics.stderr b/tests/ui/const-generics/repeat_expr_hack_gives_right_generics.stderr new file mode 100644 index 0000000000000..64548cc5a301e --- /dev/null +++ b/tests/ui/const-generics/repeat_expr_hack_gives_right_generics.stderr @@ -0,0 +1,11 @@ +error: generic parameters may not be used in const operations + --> $DIR/repeat_expr_hack_gives_right_generics.rs:16:17 + | +LL | bar::<{ [1; N] }>(); + | ^ cannot perform const operation using `N` + | + = help: const parameters may only be used as standalone arguments, i.e. `N` + = help: add `#![feature(generic_const_exprs)]` to allow generic const expressions + +error: aborting due to 1 previous error + From 1462f3d4eb6d5ce18c9c430c10315bc5543090bc Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 11 Jun 2024 08:42:20 +0200 Subject: [PATCH 07/13] Add {{target}} substitution to compiletest --- src/tools/compiletest/src/header.rs | 5 +++++ src/tools/tidy/src/target_specific_tests.rs | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index fd4ab72bbf463..fc6c721722211 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -1281,6 +1281,7 @@ fn expand_variables(mut value: String, config: &Config) -> String { const BUILD_BASE: &str = "{{build-base}}"; const SYSROOT_BASE: &str = "{{sysroot-base}}"; const TARGET_LINKER: &str = "{{target-linker}}"; + const TARGET: &str = "{{target}}"; if value.contains(CWD) { let cwd = env::current_dir().unwrap(); @@ -1303,6 +1304,10 @@ fn expand_variables(mut value: String, config: &Config) -> String { value = value.replace(TARGET_LINKER, config.target_linker.as_deref().unwrap_or("")); } + if value.contains(TARGET) { + value = value.replace(TARGET, &config.target); + } + value } diff --git a/src/tools/tidy/src/target_specific_tests.rs b/src/tools/tidy/src/target_specific_tests.rs index f3a64b38e8c66..8be27d1e117ce 100644 --- a/src/tools/tidy/src/target_specific_tests.rs +++ b/src/tools/tidy/src/target_specific_tests.rs @@ -53,9 +53,9 @@ pub fn check(path: &Path, bad: &mut bool) { } else if directive.starts_with(COMPILE_FLAGS_HEADER) { let compile_flags = &directive[COMPILE_FLAGS_HEADER.len()..]; if let Some((_, v)) = compile_flags.split_once("--target") { - if let Some((arch, _)) = - v.trim_start_matches(|c| c == ' ' || c == '=').split_once("-") - { + let v = v.trim_start_matches(|c| c == ' ' || c == '='); + let v = if v == "{{target}}" { Some((v, v)) } else { v.split_once("-") }; + if let Some((arch, _)) = v { let info = header_map.entry(revision).or_insert(RevisionInfo::default()); info.target_arch.replace(arch); } else { From d6955445f53062a8af3e5a4e84dbf756a7913cbe Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Mon, 10 Jun 2024 17:55:28 -0700 Subject: [PATCH 08/13] Simplify `[T; N]::try_map` signature People keep making fun of this signature for being so gnarly. Associated type bounds lend it a much simpler scribbling. ChangeOutputType can also come along for the ride. --- library/core/src/array/mod.rs | 6 ++---- library/core/src/ops/try_trait.rs | 4 +++- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index 05874ab6c4cbb..2569ce237077e 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -533,11 +533,9 @@ impl [T; N] { /// assert_eq!(c, Some(a)); /// ``` #[unstable(feature = "array_try_map", issue = "79711")] - pub fn try_map(self, f: F) -> ChangeOutputType + pub fn try_map(self, f: impl FnMut(T) -> R) -> ChangeOutputType where - F: FnMut(T) -> R, - R: Try, - R::Residual: Residual<[R::Output; N]>, + R: Try>, { drain_array_with(self, |iter| try_from_trusted_iterator(iter.map(f))) } diff --git a/library/core/src/ops/try_trait.rs b/library/core/src/ops/try_trait.rs index 483f55b207093..cd444c86ed06e 100644 --- a/library/core/src/ops/try_trait.rs +++ b/library/core/src/ops/try_trait.rs @@ -363,7 +363,9 @@ pub trait Residual { } #[unstable(feature = "pub_crate_should_not_need_unstable_attr", issue = "none")] -pub(crate) type ChangeOutputType = <::Residual as Residual>::TryType; +#[allow(type_alias_bounds)] +pub(crate) type ChangeOutputType>, V> = + >::TryType; /// An adapter for implementing non-try methods via the `Try` implementation. /// From 26fa5c2c300f3c3a3ee3109c009bd4a6803a2a4c Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 11 Jun 2024 10:13:07 +0200 Subject: [PATCH 09/13] Make issue-122805.rs big endian compatible Instead of not generating the function at all on big endian (which makes the CHECK lines fail), instead use to_le() on big endian, so that we essentially perform a bswap for both endiannesses. --- tests/codegen/issues/issue-122805.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/tests/codegen/issues/issue-122805.rs b/tests/codegen/issues/issue-122805.rs index 6d108ada6dd2b..8e03c6c8884a5 100644 --- a/tests/codegen/issues/issue-122805.rs +++ b/tests/codegen/issues/issue-122805.rs @@ -39,17 +39,20 @@ // OPT3WINX64-NEXT: store <8 x i16> // CHECK-NEXT: ret void #[no_mangle] -#[cfg(target_endian = "little")] pub fn convert(value: [u16; 8]) -> [u8; 16] { + #[cfg(target_endian = "little")] + let bswap = u16::to_be; + #[cfg(target_endian = "big")] + let bswap = u16::to_le; let addr16 = [ - value[0].to_be(), - value[1].to_be(), - value[2].to_be(), - value[3].to_be(), - value[4].to_be(), - value[5].to_be(), - value[6].to_be(), - value[7].to_be(), + bswap(value[0]), + bswap(value[1]), + bswap(value[2]), + bswap(value[3]), + bswap(value[4]), + bswap(value[5]), + bswap(value[6]), + bswap(value[7]), ]; unsafe { core::mem::transmute::<_, [u8; 16]>(addr16) } } From 751143ef4031d06e718bcf859f1b3139716d043b Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Tue, 11 Jun 2024 17:12:20 +0000 Subject: [PATCH 10/13] set_env: State the conclusion upfront --- library/std/src/env.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/library/std/src/env.rs b/library/std/src/env.rs index 4d649f8a6f136..295e782639bd6 100644 --- a/library/std/src/env.rs +++ b/library/std/src/env.rs @@ -323,8 +323,10 @@ impl Error for VarError { /// This function is also always safe to call on Windows, in single-threaded /// and multi-threaded programs. /// -/// In multi-threaded programs on other operating systems, we strongly suggest -/// not using `set_var` or `remove_var` at all. The exact requirement is: you +/// In multi-threaded programs on other operating systems, the only safe option is +/// to not use `set_var` or `remove_var` at all. +/// +/// The exact requirement is: you /// must ensure that there are no other threads concurrently writing or /// *reading*(!) the environment through functions or global variables other /// than the ones in this module. The problem is that these operating systems @@ -390,8 +392,10 @@ unsafe fn _set_var(key: &OsStr, value: &OsStr) { /// This function is also always safe to call on Windows, in single-threaded /// and multi-threaded programs. /// -/// In multi-threaded programs on other operating systems, we strongly suggest -/// not using `set_var` or `remove_var` at all. The exact requirement is: you +/// In multi-threaded programs on other operating systems, the only safe option is +/// to not use `set_var` or `remove_var` at all. +/// +/// The exact requirement is: you /// must ensure that there are no other threads concurrently writing or /// *reading*(!) the environment through functions or global variables other /// than the ones in this module. The problem is that these operating systems From 1a6059726551a464290117adfdf652a9e001ec75 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 11 Jun 2024 11:36:48 +1000 Subject: [PATCH 11/13] Make `storage-live.rs` robust against rustc internal changes. Currently it can be made to fail by rearranging code within `compiler/rustc_mir_transform/src/lint.rs`. --- tests/ui/mir/lint/storage-live.rs | 1 + tests/ui/mir/lint/storage-live.stderr | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/ui/mir/lint/storage-live.rs b/tests/ui/mir/lint/storage-live.rs index 8273544b56a1f..d734b773642e9 100644 --- a/tests/ui/mir/lint/storage-live.rs +++ b/tests/ui/mir/lint/storage-live.rs @@ -5,6 +5,7 @@ //@ normalize-stderr-test "note: .*\n\n" -> "" //@ normalize-stderr-test "thread 'rustc' panicked.*\n" -> "" //@ normalize-stderr-test "storage_live\[....\]" -> "storage_live[HASH]" +//@ normalize-stderr-test "(delayed at [^:]+):\d+:\d+ - " -> "$1:LL:CC - " //@ rustc-env:RUST_BACKTRACE=0 #![feature(custom_mir, core_intrinsics)] diff --git a/tests/ui/mir/lint/storage-live.stderr b/tests/ui/mir/lint/storage-live.stderr index 2eb8d8e700010..7d4c3f0832a97 100644 --- a/tests/ui/mir/lint/storage-live.stderr +++ b/tests/ui/mir/lint/storage-live.stderr @@ -1,12 +1,12 @@ error: internal compiler error: broken MIR in Item(DefId(0:8 ~ storage_live[HASH]::multiple_storage)) (after pass CheckPackedRef) at bb0[1]: StorageLive(_1) which already has storage here - --> $DIR/storage-live.rs:22:13 + --> $DIR/storage-live.rs:23:13 | LL | StorageLive(a); | ^^^^^^^^^^^^^^ | -note: delayed at compiler/rustc_mir_transform/src/lint.rs:97:26 - disabled backtrace - --> $DIR/storage-live.rs:22:13 +note: delayed at compiler/rustc_mir_transform/src/lint.rs:LL:CC - disabled backtrace + --> $DIR/storage-live.rs:23:13 | LL | StorageLive(a); | ^^^^^^^^^^^^^^ From 7e7da49f2a780917909e7aa4b5b1bb2e48900c47 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 11 Jun 2024 15:01:57 +1000 Subject: [PATCH 12/13] Update a cranelift patch file for formatting changes. PR #125443 will reformat all the use declarations in the repo. This would break a patch kept in `rustc_codegen_cranelift` that gets applied to `library/std/src/sys/pal/windows/rand.rs`. So this commit formats the use declarations in `library/std/src/sys/pal/windows/rand.rs` in advance of #125443 and updates the patch file accordingly. The motivation is that #125443 is a huge change and we want to get fiddly little changes like this out of the way so it can be nothing more than an `x fmt --all`. --- .../patches/0029-stdlib-rawdylib-processprng.patch | 6 +++--- library/std/src/sys/pal/windows/rand.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/patches/0029-stdlib-rawdylib-processprng.patch b/compiler/rustc_codegen_cranelift/patches/0029-stdlib-rawdylib-processprng.patch index 6af11e54d88af..584dbdb647f60 100644 --- a/compiler/rustc_codegen_cranelift/patches/0029-stdlib-rawdylib-processprng.patch +++ b/compiler/rustc_codegen_cranelift/patches/0029-stdlib-rawdylib-processprng.patch @@ -12,7 +12,7 @@ diff --git a/library/std/src/sys/pal/windows/c.rs b/library/std/src/sys/pal/wind index ad8e01bfa9b..9ca8e4c16ce 100644 --- a/library/std/src/sys/pal/windows/c.rs +++ b/library/std/src/sys/pal/windows/c.rs -@@ -323,7 +323,7 @@ pub unsafe fn NtWriteFile( +@@ -312,7 +312,7 @@ pub unsafe fn NtWriteFile( // Use raw-dylib to import ProcessPrng as we can't rely on there being an import library. cfg_if::cfg_if! { @@ -26,8 +26,8 @@ index e427546222a..f2fe42a4d51 100644 --- a/library/std/src/sys/pal/windows/rand.rs +++ b/library/std/src/sys/pal/windows/rand.rs @@ -2,7 +2,7 @@ - use core::mem; - use core::ptr; + + use crate::sys::c; -#[cfg(not(target_vendor = "win7"))] +#[cfg(any())] diff --git a/library/std/src/sys/pal/windows/rand.rs b/library/std/src/sys/pal/windows/rand.rs index e427546222aea..09f527a09bf12 100644 --- a/library/std/src/sys/pal/windows/rand.rs +++ b/library/std/src/sys/pal/windows/rand.rs @@ -1,6 +1,6 @@ +use core::{mem, ptr}; + use crate::sys::c; -use core::mem; -use core::ptr; #[cfg(not(target_vendor = "win7"))] #[inline] From 0b3fec9388d358b0b48d1ea2f83ff43468402f23 Mon Sep 17 00:00:00 2001 From: surechen Date: Sun, 9 Jun 2024 17:20:25 +0800 Subject: [PATCH 13/13] For E0277 suggest adding `Result` return type for function which using QuesionMark `?` in the body. --- compiler/rustc_hir/src/hir.rs | 1 + .../src/traits/error_reporting/suggestions.rs | 41 +++++++++ .../error_reporting/type_err_ctxt_ext.rs | 4 + ...turn-from-residual-sugg-issue-125997.fixed | 42 +++++++++ .../return-from-residual-sugg-issue-125997.rs | 34 ++++++++ ...urn-from-residual-sugg-issue-125997.stderr | 86 +++++++++++++++++++ .../ui/try-trait/try-operator-on-main.stderr | 10 +++ 7 files changed, 218 insertions(+) create mode 100644 tests/ui/return/return-from-residual-sugg-issue-125997.fixed create mode 100644 tests/ui/return/return-from-residual-sugg-issue-125997.rs create mode 100644 tests/ui/return/return-from-residual-sugg-issue-125997.stderr diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index e64f7aeb11b3e..ff149e9750d50 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -1010,6 +1010,7 @@ pub struct Block<'hir> { pub hir_id: HirId, /// Distinguishes between `unsafe { ... }` and `{ ... }`. pub rules: BlockCheckMode, + /// The span includes the curly braces `{` and `}` around the block. pub span: Span, /// If true, then there may exist `break 'a` values that aim to /// break out of this block early. diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 9336148fd672c..fddb7c263beaf 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -4586,6 +4586,47 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { _ => "/* value */".to_string(), }) } + + fn suggest_add_result_as_return_type( + &self, + obligation: &PredicateObligation<'tcx>, + err: &mut Diag<'_>, + trait_ref: ty::PolyTraitRef<'tcx>, + ) { + if ObligationCauseCode::QuestionMark != *obligation.cause.code().peel_derives() { + return; + } + + let node = self.tcx.hir_node_by_def_id(obligation.cause.body_id); + if let hir::Node::Item(item) = node + && let hir::ItemKind::Fn(sig, _, body_id) = item.kind + && let hir::FnRetTy::DefaultReturn(ret_span) = sig.decl.output + && self.tcx.is_diagnostic_item(sym::FromResidual, trait_ref.def_id()) + && let ty::Tuple(l) = trait_ref.skip_binder().args.type_at(0).kind() + && l.len() == 0 + && let ty::Adt(def, _) = trait_ref.skip_binder().args.type_at(1).kind() + && self.tcx.is_diagnostic_item(sym::Result, def.did()) + { + let body = self.tcx.hir().body(body_id); + let mut sugg_spans = + vec![(ret_span, " -> Result<(), Box>".to_string())]; + + if let hir::ExprKind::Block(b, _) = body.value.kind + && b.expr.is_none() + { + sugg_spans.push(( + // The span will point to the closing curly brace `}` of the block. + b.span.shrink_to_hi().with_lo(b.span.hi() - BytePos(1)), + "\n Ok(())\n}".to_string(), + )); + } + err.multipart_suggestion_verbose( + format!("consider adding return type"), + sugg_spans, + Applicability::MaybeIncorrect, + ); + } + } } /// Add a hint to add a missing borrow or remove an unnecessary one. diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index da611b748da6b..9d1cf0e74f3eb 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -612,6 +612,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { &mut err, trait_predicate, ); + self.suggest_add_result_as_return_type(&obligation, + &mut err, + trait_ref); + if self.suggest_add_reference_to_arg( &obligation, &mut err, diff --git a/tests/ui/return/return-from-residual-sugg-issue-125997.fixed b/tests/ui/return/return-from-residual-sugg-issue-125997.fixed new file mode 100644 index 0000000000000..b2eca69aeb902 --- /dev/null +++ b/tests/ui/return/return-from-residual-sugg-issue-125997.fixed @@ -0,0 +1,42 @@ +//@ run-rustfix + +#![allow(unused_imports)] +#![allow(dead_code)] + +use std::fs::File; +use std::io::prelude::*; + +fn test1() -> Result<(), Box> { + let mut _file = File::create("foo.txt")?; + //~^ ERROR the `?` operator can only be used in a function + + Ok(()) +} + +fn test2() -> Result<(), Box> { + let mut _file = File::create("foo.txt")?; + //~^ ERROR the `?` operator can only be used in a function + println!(); + + Ok(()) +} + +macro_rules! mac { + () => { + fn test3() -> Result<(), Box> { + let mut _file = File::create("foo.txt")?; + //~^ ERROR the `?` operator can only be used in a function + println!(); + + Ok(()) +} + }; +} + +fn main() -> Result<(), Box> { + let mut _file = File::create("foo.txt")?; + //~^ ERROR the `?` operator can only be used in a function + mac!(); + + Ok(()) +} diff --git a/tests/ui/return/return-from-residual-sugg-issue-125997.rs b/tests/ui/return/return-from-residual-sugg-issue-125997.rs new file mode 100644 index 0000000000000..dd8550a388b86 --- /dev/null +++ b/tests/ui/return/return-from-residual-sugg-issue-125997.rs @@ -0,0 +1,34 @@ +//@ run-rustfix + +#![allow(unused_imports)] +#![allow(dead_code)] + +use std::fs::File; +use std::io::prelude::*; + +fn test1() { + let mut _file = File::create("foo.txt")?; + //~^ ERROR the `?` operator can only be used in a function +} + +fn test2() { + let mut _file = File::create("foo.txt")?; + //~^ ERROR the `?` operator can only be used in a function + println!(); +} + +macro_rules! mac { + () => { + fn test3() { + let mut _file = File::create("foo.txt")?; + //~^ ERROR the `?` operator can only be used in a function + println!(); + } + }; +} + +fn main() { + let mut _file = File::create("foo.txt")?; + //~^ ERROR the `?` operator can only be used in a function + mac!(); +} diff --git a/tests/ui/return/return-from-residual-sugg-issue-125997.stderr b/tests/ui/return/return-from-residual-sugg-issue-125997.stderr new file mode 100644 index 0000000000000..ef938f0213dfb --- /dev/null +++ b/tests/ui/return/return-from-residual-sugg-issue-125997.stderr @@ -0,0 +1,86 @@ +error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `FromResidual`) + --> $DIR/return-from-residual-sugg-issue-125997.rs:10:44 + | +LL | fn test1() { + | ---------- this function should return `Result` or `Option` to accept `?` +LL | let mut _file = File::create("foo.txt")?; + | ^ cannot use the `?` operator in a function that returns `()` + | + = help: the trait `FromResidual>` is not implemented for `()` +help: consider adding return type + | +LL ~ fn test1() -> Result<(), Box> { +LL | let mut _file = File::create("foo.txt")?; +LL | +LL + +LL + Ok(()) +LL + } + | + +error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `FromResidual`) + --> $DIR/return-from-residual-sugg-issue-125997.rs:15:44 + | +LL | fn test2() { + | ---------- this function should return `Result` or `Option` to accept `?` +LL | let mut _file = File::create("foo.txt")?; + | ^ cannot use the `?` operator in a function that returns `()` + | + = help: the trait `FromResidual>` is not implemented for `()` +help: consider adding return type + | +LL ~ fn test2() -> Result<(), Box> { +LL | let mut _file = File::create("foo.txt")?; +LL | +LL | println!(); +LL + +LL + Ok(()) +LL + } + | + +error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `FromResidual`) + --> $DIR/return-from-residual-sugg-issue-125997.rs:31:44 + | +LL | fn main() { + | --------- this function should return `Result` or `Option` to accept `?` +LL | let mut _file = File::create("foo.txt")?; + | ^ cannot use the `?` operator in a function that returns `()` + | + = help: the trait `FromResidual>` is not implemented for `()` +help: consider adding return type + | +LL ~ fn main() -> Result<(), Box> { +LL | let mut _file = File::create("foo.txt")?; +LL | +LL | mac!(); +LL + +LL + Ok(()) +LL + } + | + +error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `FromResidual`) + --> $DIR/return-from-residual-sugg-issue-125997.rs:23:52 + | +LL | fn test3() { + | ---------- this function should return `Result` or `Option` to accept `?` +LL | let mut _file = File::create("foo.txt")?; + | ^ cannot use the `?` operator in a function that returns `()` +... +LL | mac!(); + | ------ in this macro invocation + | + = help: the trait `FromResidual>` is not implemented for `()` + = note: this error originates in the macro `mac` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider adding return type + | +LL ~ fn test3() -> Result<(), Box> { +LL | let mut _file = File::create("foo.txt")?; +LL | +LL | println!(); +LL ~ +LL + Ok(()) +LL + } + | + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/try-trait/try-operator-on-main.stderr b/tests/ui/try-trait/try-operator-on-main.stderr index 7cd38e0cf95ea..ba6eacde68fd5 100644 --- a/tests/ui/try-trait/try-operator-on-main.stderr +++ b/tests/ui/try-trait/try-operator-on-main.stderr @@ -8,6 +8,16 @@ LL | std::fs::File::open("foo")?; | ^ cannot use the `?` operator in a function that returns `()` | = help: the trait `FromResidual>` is not implemented for `()` +help: consider adding return type + | +LL ~ fn main() -> Result<(), Box> { +LL | // error for a `Try` type on a non-`Try` fn + ... +LL | try_trait_generic::<()>(); +LL + +LL + Ok(()) +LL + } + | error[E0277]: the `?` operator can only be applied to values that implement `Try` --> $DIR/try-operator-on-main.rs:10:5