Auto merge of rust-lang#121897 - matthiaskrgr:rollup-89tcmau, r=matthiaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#121194 (Refactor pre-getopts command line argument handling)
 - rust-lang#121666 (Use the OS thread name by default if `THREAD_INFO` has not been initialized)
 - rust-lang#121758 (Move thread local implementation to `sys`)
 - rust-lang#121759 (attempt to further clarify addr_of docs)
 - rust-lang#121888 (style library/core/src/error.rs)
 - rust-lang#121892 (The ordinary lowering of `thir::ExprKind::Let` is unreachable)

r? `@ghost`
`@rustbot` modify labels: rollup

Author: bors

compiler/rustc_driver_impl/src/args.rs

@@ -1,9 +1,7 @@
-use std::error;
-use std::fmt;
-use std::fs;
-use std::io;
+use std::{env, error, fmt, fs, io};
 
 use rustc_session::EarlyDiagCtxt;
+use rustc_span::ErrorGuaranteed;
 
 /// Expands argfiles in command line arguments.
 #[derive(Default)]
@@ -86,41 +84,70 @@ impl Expander {
     fn read_file(path: &str) -> Result<String, Error> {
         fs::read_to_string(path).map_err(|e| {
             if e.kind() == io::ErrorKind::InvalidData {
-                Error::Utf8Error(Some(path.to_string()))
+                Error::Utf8Error(path.to_string())
             } else {
                 Error::IOError(path.to_string(), e)
             }
         })
     }
 }
 
+/// Replaces any `@file` arguments with the contents of `file`, with each line of `file` as a
+/// separate argument.
+///
 /// **Note:** This function doesn't interpret argument 0 in any special way.
 /// If this function is intended to be used with command line arguments,
 /// `argv[0]` must be removed prior to calling it manually.
-pub fn arg_expand_all(early_dcx: &EarlyDiagCtxt, at_args: &[String]) -> Vec<String> {
+pub fn arg_expand_all(
+    early_dcx: &EarlyDiagCtxt,
+    at_args: &[String],
+) -> Result<Vec<String>, ErrorGuaranteed> {
     let mut expander = Expander::default();
+    let mut result = Ok(());
     for arg in at_args {
         if let Err(err) = expander.arg(arg) {
-            early_dcx.early_fatal(format!("Failed to load argument file: {err}"));
+            result = Err(early_dcx.early_err(format!("failed to load argument file: {err}")));
         }
     }
-    expander.finish()
+    result.map(|()| expander.finish())
+}
+
+/// Gets the raw unprocessed command-line arguments as Unicode strings, without doing any further
+/// processing (e.g., without `@file` expansion).
+///
+/// This function is identical to [`env::args()`] except that it emits an error when it encounters
+/// non-Unicode arguments instead of panicking.
+pub fn raw_args(early_dcx: &EarlyDiagCtxt) -> Result<Vec<String>, ErrorGuaranteed> {
+    let mut res = Ok(Vec::new());
+    for (i, arg) in env::args_os().enumerate() {
+        match arg.into_string() {
+            Ok(arg) => {
+                if let Ok(args) = &mut res {
+                    args.push(arg);
+                }
+            }
+            Err(arg) => {
+                res =
+                    Err(early_dcx.early_err(format!("argument {i} is not valid Unicode: {arg:?}")))
+            }
+        }
+    }
+    res
 }
 
 #[derive(Debug)]
-pub enum Error {
-    Utf8Error(Option<String>),
+enum Error {
+    Utf8Error(String),
     IOError(String, io::Error),
     ShellParseError(String),
 }
 
 impl fmt::Display for Error {
     fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            Error::Utf8Error(None) => write!(fmt, "Utf8 error"),
-            Error::Utf8Error(Some(path)) => write!(fmt, "Utf8 error in {path}"),
-            Error::IOError(path, err) => write!(fmt, "IO Error: {path}: {err}"),
-            Error::ShellParseError(path) => write!(fmt, "Invalid shell-style arguments in {path}"),
+            Error::Utf8Error(path) => write!(fmt, "UTF-8 error in {path}"),
+            Error::IOError(path, err) => write!(fmt, "IO error: {path}: {err}"),
+            Error::ShellParseError(path) => write!(fmt, "invalid shell-style arguments in {path}"),
         }
     }
 }

compiler/rustc_driver_impl/src/lib.rs

@@ -291,7 +291,7 @@ fn run_compiler(
     // the compiler with @empty_file as argv[0] and no more arguments.
     let at_args = at_args.get(1..).unwrap_or_default();
 
-    let args = args::arg_expand_all(&default_early_dcx, at_args);
+    let args = args::arg_expand_all(&default_early_dcx, at_args)?;
 
     let Some(matches) = handle_options(&default_early_dcx, &args) else { return Ok(()) };
 
@@ -1489,15 +1489,7 @@ pub fn main() -> ! {
     let mut callbacks = TimePassesCallbacks::default();
     let using_internal_features = install_ice_hook(DEFAULT_BUG_REPORT_URL, |_| ());
     let exit_code = catch_with_exit_code(|| {
-        let args = env::args_os()
-            .enumerate()
-            .map(|(i, arg)| {
-                arg.into_string().unwrap_or_else(|arg| {
-                    early_dcx.early_fatal(format!("argument {i} is not valid Unicode: {arg:?}"))
-                })
-            })
-            .collect::<Vec<_>>();
-        RunCompiler::new(&args, &mut callbacks)
+        RunCompiler::new(&args::raw_args(&early_dcx)?, &mut callbacks)
             .set_using_internal_features(using_internal_features)
             .run()
     });

compiler/rustc_mir_build/src/build/expr/into.rs

@@ -109,38 +109,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 this.cfg.goto(else_blk, source_info, join_block);
                 join_block.unit()
             }
-            ExprKind::Let { expr, ref pat } => {
-                let scope = this.local_scope();
-                let (true_block, false_block) = this.in_if_then_scope(scope, expr_span, |this| {
-                    this.lower_let_expr(block, expr, pat, scope, None, expr_span, true)
-                });
-
-                this.cfg.push_assign_constant(
-                    true_block,
-                    source_info,
-                    destination,
-                    ConstOperand {
-                        span: expr_span,
-                        user_ty: None,
-                        const_: Const::from_bool(this.tcx, true),
-                    },
-                );
-
-                this.cfg.push_assign_constant(
-                    false_block,
-                    source_info,
-                    destination,
-                    ConstOperand {
-                        span: expr_span,
-                        user_ty: None,
-                        const_: Const::from_bool(this.tcx, false),
-                    },
-                );
-
-                let join_block = this.cfg.start_new_block();
-                this.cfg.goto(true_block, source_info, join_block);
-                this.cfg.goto(false_block, source_info, join_block);
-                join_block.unit()
+            ExprKind::Let { .. } => {
+                // After desugaring, `let` expressions should only appear inside `if`
+                // expressions or `match` guards, possibly nested within a let-chain.
+                // In both cases they are specifically handled by the lowerings of
+                // those expressions, so this case is currently unreachable.
+                span_bug!(expr_span, "unexpected let expression outside of if or match-guard");
             }
             ExprKind::NeverToAny { source } => {
                 let source_expr = &this.thir[source];

library/core/src/error.rs

@@ -183,6 +183,7 @@ pub trait Error: Debug + Display {
     #[allow(unused_variables)]
     fn provide<'a>(&'a self, request: &mut Request<'a>) {}
 }
+
 mod private {
     // This is a hack to prevent `type_id` from being overridden by `Error`
     // implementations, since that can enable unsound downcasting.

library/core/src/ptr/mod.rs

@@ -2071,11 +2071,16 @@ impl<F: FnPtr> fmt::Debug for F {
 /// as all other references. This macro can create a raw pointer *without* creating
 /// a reference first.
 ///
-/// The `expr` in `addr_of!(expr)` is evaluated as a place expression, but never loads
-/// from the place or requires the place to be dereferenceable. This means that
-/// `addr_of!(*ptr)` is defined behavior even if `ptr` is null, dangling, or misaligned.
-/// Note however that `addr_of!((*ptr).field)` still requires the projection to
-/// `field` to be in-bounds, using the same rules as [`offset`].
+/// See [`addr_of_mut`] for how to create a pointer to uninitialized data.
+/// Doing that with `addr_of` would not make much sense since one could only
+/// read the data, and that would be Undefined Behavior.
+///
+/// # Safety
+///
+/// The `expr` in `addr_of!(expr)` is evaluated as a place expression, but never loads from the
+/// place or requires the place to be dereferenceable. This means that `addr_of!((*ptr).field)`
+/// still requires the projection to `field` to be in-bounds, using the same rules as [`offset`].
+/// However, `addr_of!(*ptr)` is defined behavior even if `ptr` is null, dangling, or misaligned.
 ///
 /// Note that `Deref`/`Index` coercions (and their mutable counterparts) are applied inside
 /// `addr_of!` like everywhere else, in which case a reference is created to call `Deref::deref` or
@@ -2086,6 +2091,8 @@ impl<F: FnPtr> fmt::Debug for F {
 ///
 /// # Example
 ///
+/// **Correct usage: Creating a pointer to unaligned data**
+///
 /// ```
 /// use std::ptr;
 ///
@@ -2101,9 +2108,27 @@ impl<F: FnPtr> fmt::Debug for F {
 /// assert_eq!(unsafe { raw_f2.read_unaligned() }, 2);
 /// ```
 ///
-/// See [`addr_of_mut`] for how to create a pointer to uninitialized data.
-/// Doing that with `addr_of` would not make much sense since one could only
-/// read the data, and that would be Undefined Behavior.
+/// **Incorrect usage: Out-of-bounds fields projection**
+///
+/// ```rust,no_run
+/// use std::ptr;
+///
+/// #[repr(C)]
+/// struct MyStruct {
+///     field1: i32,
+///     field2: i32,
+/// }
+///
+/// let ptr: *const MyStruct = ptr::null();
+/// let fieldptr = unsafe { ptr::addr_of!((*ptr).field2) }; // Undefined Behavior ⚠️
+/// ```
+///
+/// The field projection `.field2` would offset the pointer by 4 bytes,
+/// but the pointer is not in-bounds of an allocation for 4 bytes,
+/// so this offset is Undefined Behavior.
+/// See the [`offset`] docs for a full list of requirements for inbounds pointer arithmetic; the
+/// same requirements apply to field projections, even inside `addr_of!`. (In particular, it makes
+/// no difference whether the pointer is null or dangling.)
 #[stable(feature = "raw_ref_macros", since = "1.51.0")]
 #[rustc_macro_transparency = "semitransparent"]
 #[allow_internal_unstable(raw_ref_op)]
@@ -2120,11 +2145,12 @@ pub macro addr_of($place:expr) {
 /// as all other references. This macro can create a raw pointer *without* creating
 /// a reference first.
 ///
-/// The `expr` in `addr_of_mut!(expr)` is evaluated as a place expression, but never loads
-/// from the place or requires the place to be dereferenceable. This means that
-/// `addr_of_mut!(*ptr)` is defined behavior even if `ptr` is null, dangling, or misaligned.
-/// Note however that `addr_of_mut!((*ptr).field)` still requires the projection to
-/// `field` to be in-bounds, using the same rules as [`offset`].
+/// # Safety
+///
+/// The `expr` in `addr_of_mut!(expr)` is evaluated as a place expression, but never loads from the
+/// place or requires the place to be dereferenceable. This means that `addr_of_mut!((*ptr).field)`
+/// still requires the projection to `field` to be in-bounds, using the same rules as [`offset`].
+/// However, `addr_of_mut!(*ptr)` is defined behavior even if `ptr` is null, dangling, or misaligned.
 ///
 /// Note that `Deref`/`Index` coercions (and their mutable counterparts) are applied inside
 /// `addr_of_mut!` like everywhere else, in which case a reference is created to call `Deref::deref`
@@ -2135,7 +2161,7 @@ pub macro addr_of($place:expr) {
 ///
 /// # Examples
 ///
-/// **Creating a pointer to unaligned data:**
+/// **Correct usage: Creating a pointer to unaligned data**
 ///
 /// ```
 /// use std::ptr;
@@ -2153,7 +2179,7 @@ pub macro addr_of($place:expr) {
 /// assert_eq!({packed.f2}, 42); // `{...}` forces copying the field instead of creating a reference.
 /// ```
 ///
-/// **Creating a pointer to uninitialized data:**
+/// **Correct usage: Creating a pointer to uninitialized data**
 ///
 /// ```rust
 /// use std::{ptr, mem::MaybeUninit};
@@ -2169,6 +2195,28 @@ pub macro addr_of($place:expr) {
 /// unsafe { f1_ptr.write(true); }
 /// let init = unsafe { uninit.assume_init() };
 /// ```
+///
+/// **Incorrect usage: Out-of-bounds fields projection**
+///
+/// ```rust,no_run
+/// use std::ptr;
+///
+/// #[repr(C)]
+/// struct MyStruct {
+///     field1: i32,
+///     field2: i32,
+/// }
+///
+/// let ptr: *mut MyStruct = ptr::null_mut();
+/// let fieldptr = unsafe { ptr::addr_of_mut!((*ptr).field2) }; // Undefined Behavior ⚠️
+/// ```
+///
+/// The field projection `.field2` would offset the pointer by 4 bytes,
+/// but the pointer is not in-bounds of an allocation for 4 bytes,
+/// so this offset is Undefined Behavior.
+/// See the [`offset`] docs for a full list of requirements for inbounds pointer arithmetic; the
+/// same requirements apply to field projections, even inside `addr_of_mut!`. (In particular, it
+/// makes no difference whether the pointer is null or dangling.)
 #[stable(feature = "raw_ref_macros", since = "1.51.0")]
 #[rustc_macro_transparency = "semitransparent"]
 #[allow_internal_unstable(raw_ref_op)]

library/std/src/sys/mod.rs

@@ -9,6 +9,9 @@ pub mod cmath;
 pub mod locks;
 pub mod os_str;
 pub mod path;
+#[allow(dead_code)]
+#[allow(unused_imports)]
+pub mod thread_local;
 
 // FIXME(117276): remove this, move feature implementations into individual
 //                submodules.

library/std/src/sys/pal/common/mod.rs

@@ -12,8 +12,6 @@
 
 pub mod alloc;
 pub mod small_c_string;
-#[allow(unused_imports)]
-pub mod thread_local;
 
 #[cfg(test)]
 mod tests;

library/std/src/sys/pal/hermit/thread.rs

@@ -2,7 +2,7 @@
 
 use super::abi;
 use super::thread_local_dtor::run_dtors;
-use crate::ffi::CStr;
+use crate::ffi::{CStr, CString};
 use crate::io;
 use crate::mem;
 use crate::num::NonZero;
@@ -71,6 +71,10 @@ impl Thread {
         // nope
     }
 
+    pub fn get_name() -> Option<CString> {
+        None
+    }
+
     #[inline]
     pub fn sleep(dur: Duration) {
         unsafe {

library/std/src/sys/pal/itron/thread.rs

@@ -8,7 +8,7 @@ use super::{
 };
 use crate::{
     cell::UnsafeCell,
-    ffi::CStr,
+    ffi::{CStr, CString},
     hint, io,
     mem::ManuallyDrop,
     num::NonZero,
@@ -204,6 +204,10 @@ impl Thread {
         // nope
     }
 
+    pub fn get_name() -> Option<CString> {
+        None
+    }
+
     pub fn sleep(dur: Duration) {
         for timeout in dur2reltims(dur) {
             expect_success(unsafe { abi::dly_tsk(timeout) }, &"dly_tsk");

library/std/src/sys/pal/sgx/thread.rs

@@ -1,6 +1,6 @@
 #![cfg_attr(test, allow(dead_code))] // why is this necessary?
 use super::unsupported;
-use crate::ffi::CStr;
+use crate::ffi::{CStr, CString};
 use crate::io;
 use crate::num::NonZero;
 use crate::time::Duration;
@@ -133,6 +133,10 @@ impl Thread {
         // which succeeds as-is with the SGX target.
     }
 
+    pub fn get_name() -> Option<CString> {
+        None
+    }
+
     pub fn sleep(dur: Duration) {
         usercalls::wait_timeout(0, dur, || true);
     }

library/std/src/sys/pal/teeos/thread.rs

@@ -1,7 +1,7 @@
 use core::convert::TryInto;
 
 use crate::cmp;
-use crate::ffi::CStr;
+use crate::ffi::{CStr, CString};
 use crate::io;
 use crate::mem;
 use crate::num::NonZero;
@@ -101,6 +101,10 @@ impl Thread {
         // contact the teeos rustzone team.
     }
 
+    pub fn get_name() -> Option<CString> {
+        None
+    }
+
     /// only main thread could wait for sometime in teeos
     pub fn sleep(dur: Duration) {
         let sleep_millis = dur.as_millis();