Auto merge of #134844 - Zalathar:rollup-1225wh9, r=Zalathar

Rollup of 5 pull requests

Successful merges:

 - #134737 (Implement `default_overrides_default_fields` lint)
 - #134760 (Migrate `branch-protection-check-IBT` to rmake.rs)
 - #134829 (Migrate `libs-through-symlink` to rmake.rs)
 - #134832 (Update `compiler-builtins` to 0.1.140)
 - #134840 (compiletest: Only pass the post-colon value to `parse_normalize_rule`)

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

Author: bors

compiler/rustc_codegen_cranelift/patches/0029-stdlib-Disable-f16-and-f128-in-compiler-builtins.patch

@@ -16,8 +16,8 @@ index 7165c3e48af..968552ad435 100644
 
  [dependencies]
  core = { path = "../core" }
--compiler_builtins = { version = "=0.1.138", features = ['rustc-dep-of-std'] }
-+compiler_builtins = { version = "=0.1.138", features = ['rustc-dep-of-std', 'no-f16-f128'] }
+-compiler_builtins = { version = "=0.1.140", features = ['rustc-dep-of-std'] }
++compiler_builtins = { version = "=0.1.140", features = ['rustc-dep-of-std', 'no-f16-f128'] }
 
  [dev-dependencies]
  rand = { version = "0.8.5", default-features = false, features = ["alloc"] }

compiler/rustc_lint/src/default_could_be_derived.rs

@@ -0,0 +1,185 @@
+use rustc_data_structures::fx::FxHashMap;
+use rustc_errors::Diag;
+use rustc_hir as hir;
+use rustc_middle::ty;
+use rustc_session::{declare_lint, impl_lint_pass};
+use rustc_span::Symbol;
+use rustc_span::symbol::sym;
+
+use crate::{LateContext, LateLintPass};
+
+declare_lint! {
+    /// The `default_overrides_default_fields` lint checks for manual `impl` blocks of the
+    /// `Default` trait of types with default field values.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,compile_fail
+    /// #![feature(default_field_values)]
+    /// struct Foo {
+    ///     x: i32 = 101,
+    ///     y: NonDefault,
+    /// }
+    ///
+    /// struct NonDefault;
+    ///
+    /// #[deny(default_overrides_default_fields)]
+    /// impl Default for Foo {
+    ///     fn default() -> Foo {
+    ///         Foo { x: 100, y: NonDefault }
+    ///     }
+    /// }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Manually writing a `Default` implementation for a type that has
+    /// default field values runs the risk of diverging behavior between
+    /// `Type { .. }` and `<Type as Default>::default()`, which would be a
+    /// foot-gun for users of that type that would expect these to be
+    /// equivalent. If `Default` can't be derived due to some fields not
+    /// having a `Default` implementation, we encourage the use of `..` for
+    /// the fields that do have a default field value.
+    pub DEFAULT_OVERRIDES_DEFAULT_FIELDS,
+    Deny,
+    "detect `Default` impl that should use the type's default field values",
+    @feature_gate = default_field_values;
+}
+
+#[derive(Default)]
+pub(crate) struct DefaultCouldBeDerived;
+
+impl_lint_pass!(DefaultCouldBeDerived => [DEFAULT_OVERRIDES_DEFAULT_FIELDS]);
+
+impl<'tcx> LateLintPass<'tcx> for DefaultCouldBeDerived {
+    fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &hir::ImplItem<'_>) {
+        // Look for manual implementations of `Default`.
+        let Some(default_def_id) = cx.tcx.get_diagnostic_item(sym::Default) else { return };
+        let hir::ImplItemKind::Fn(_sig, body_id) = impl_item.kind else { return };
+        let assoc = cx.tcx.associated_item(impl_item.owner_id);
+        let parent = assoc.container_id(cx.tcx);
+        if cx.tcx.has_attr(parent, sym::automatically_derived) {
+            // We don't care about what `#[derive(Default)]` produces in this lint.
+            return;
+        }
+        let Some(trait_ref) = cx.tcx.impl_trait_ref(parent) else { return };
+        let trait_ref = trait_ref.instantiate_identity();
+        if trait_ref.def_id != default_def_id {
+            return;
+        }
+        let ty = trait_ref.self_ty();
+        let ty::Adt(def, _) = ty.kind() else { return };
+
+        // We now know we have a manually written definition of a `<Type as Default>::default()`.
+
+        let hir = cx.tcx.hir();
+
+        let type_def_id = def.did();
+        let body = hir.body(body_id);
+
+        // FIXME: evaluate bodies with statements and evaluate bindings to see if they would be
+        // derivable.
+        let hir::ExprKind::Block(hir::Block { stmts: _, expr: Some(expr), .. }, None) =
+            body.value.kind
+        else {
+            return;
+        };
+
+        // Keep a mapping of field name to `hir::FieldDef` for every field in the type. We'll use
+        // these to check for things like checking whether it has a default or using its span for
+        // suggestions.
+        let orig_fields = match hir.get_if_local(type_def_id) {
+            Some(hir::Node::Item(hir::Item {
+                kind:
+                    hir::ItemKind::Struct(hir::VariantData::Struct { fields, recovered: _ }, _generics),
+                ..
+            })) => fields.iter().map(|f| (f.ident.name, f)).collect::<FxHashMap<_, _>>(),
+            _ => return,
+        };
+
+        // We check `fn default()` body is a single ADT literal and get all the fields that are
+        // being set.
+        let hir::ExprKind::Struct(_qpath, fields, tail) = expr.kind else { return };
+
+        // We have a struct literal
+        //
+        // struct Foo {
+        //     field: Type,
+        // }
+        //
+        // impl Default for Foo {
+        //     fn default() -> Foo {
+        //         Foo {
+        //             field: val,
+        //         }
+        //     }
+        // }
+        //
+        // We would suggest `#[derive(Default)]` if `field` has a default value, regardless of what
+        // it is; we don't want to encourage divergent behavior between `Default::default()` and
+        // `..`.
+
+        if let hir::StructTailExpr::Base(_) = tail {
+            // This is *very* niche. We'd only get here if someone wrote
+            // impl Default for Ty {
+            //     fn default() -> Ty {
+            //         Ty { ..something() }
+            //     }
+            // }
+            // where `something()` would have to be a call or path.
+            // We have nothing meaninful to do with this.
+            return;
+        }
+
+        // At least one of the fields with a default value have been overriden in
+        // the `Default` implementation. We suggest removing it and relying on `..`
+        // instead.
+        let any_default_field_given =
+            fields.iter().any(|f| orig_fields.get(&f.ident.name).and_then(|f| f.default).is_some());
+
+        if !any_default_field_given {
+            // None of the default fields were actually provided explicitly, so the manual impl
+            // doesn't override them (the user used `..`), so there's no risk of divergent behavior.
+            return;
+        }
+
+        let Some(local) = parent.as_local() else { return };
+        let hir_id = cx.tcx.local_def_id_to_hir_id(local);
+        let hir::Node::Item(item) = cx.tcx.hir_node(hir_id) else { return };
+        cx.tcx.node_span_lint(DEFAULT_OVERRIDES_DEFAULT_FIELDS, hir_id, item.span, |diag| {
+            mk_lint(diag, orig_fields, fields);
+        });
+    }
+}
+
+fn mk_lint(
+    diag: &mut Diag<'_, ()>,
+    orig_fields: FxHashMap<Symbol, &hir::FieldDef<'_>>,
+    fields: &[hir::ExprField<'_>],
+) {
+    diag.primary_message("`Default` impl doesn't use the declared default field values");
+
+    // For each field in the struct expression
+    //   - if the field in the type has a default value, it should be removed
+    //   - elif the field is an expression that could be a default value, it should be used as the
+    //     field's default value (FIXME: not done).
+    //   - else, we wouldn't touch this field, it would remain in the manual impl
+    let mut removed_all_fields = true;
+    for field in fields {
+        if orig_fields.get(&field.ident.name).and_then(|f| f.default).is_some() {
+            diag.span_label(field.expr.span, "this field has a default value");
+        } else {
+            removed_all_fields = false;
+        }
+    }
+
+    diag.help(if removed_all_fields {
+        "to avoid divergence in behavior between `Struct { .. }` and \
+         `<Struct as Default>::default()`, derive the `Default`"
+    } else {
+        "use the default values in the `impl` with `Struct { mandatory_field, .. }` to avoid them \
+         diverging over time"
+    });
+}

compiler/rustc_lint/src/lib.rs

@@ -41,6 +41,7 @@ mod async_fn_in_trait;
 pub mod builtin;
 mod context;
 mod dangling;
+mod default_could_be_derived;
 mod deref_into_dyn_supertrait;
 mod drop_forget_useless;
 mod early;
@@ -85,6 +86,7 @@ use async_closures::AsyncClosureUsage;
 use async_fn_in_trait::AsyncFnInTrait;
 use builtin::*;
 use dangling::*;
+use default_could_be_derived::DefaultCouldBeDerived;
 use deref_into_dyn_supertrait::*;
 use drop_forget_useless::*;
 use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
@@ -189,6 +191,7 @@ late_lint_methods!(
         BuiltinCombinedModuleLateLintPass,
         [
             ForLoopsOverFallibles: ForLoopsOverFallibles,
+            DefaultCouldBeDerived: DefaultCouldBeDerived::default(),
             DerefIntoDynSupertrait: DerefIntoDynSupertrait,
             DropForgetUseless: DropForgetUseless,
             ImproperCTypesDeclarations: ImproperCTypesDeclarations,

library/Cargo.lock

@@ -61,9 +61,9 @@ dependencies = [
 
 [[package]]
 name = "compiler_builtins"
-version = "0.1.138"
+version = "0.1.140"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "53f0ea7fff95b51f84371588f06062557e96bbe363d2b36218ddb806f3ca8611"
+checksum = "df14d41c5d172a886df3753d54238eefb0f61c96cbd8b363c33ccc92c457bee3"
 dependencies = [
  "cc",
  "rustc-std-workspace-core",

library/alloc/Cargo.toml

@@ -10,7 +10,7 @@ edition = "2021"
 
 [dependencies]
 core = { path = "../core" }
-compiler_builtins = { version = "=0.1.138", features = ['rustc-dep-of-std'] }
+compiler_builtins = { version = "=0.1.140", features = ['rustc-dep-of-std'] }
 
 [dev-dependencies]
 rand = { version = "0.8.5", default-features = false, features = ["alloc"] }

library/std/Cargo.toml

@@ -17,7 +17,7 @@ cfg-if = { version = "1.0", features = ['rustc-dep-of-std'] }
 panic_unwind = { path = "../panic_unwind", optional = true }
 panic_abort = { path = "../panic_abort" }
 core = { path = "../core", public = true }
-compiler_builtins = { version = "=0.1.138" }
+compiler_builtins = { version = "=0.1.140" }
 unwind = { path = "../unwind" }
 hashbrown = { version = "0.15", default-features = false, features = [
     'rustc-dep-of-std',

src/tools/compiletest/src/header.rs

@@ -978,10 +978,10 @@ impl Config {
         }
     }
 
-    fn parse_custom_normalization(&self, line: &str) -> Option<NormalizeRule> {
+    fn parse_custom_normalization(&self, raw_directive: &str) -> Option<NormalizeRule> {
         // FIXME(Zalathar): Integrate name/value splitting into `DirectiveLine`
         // instead of doing it here.
-        let (directive_name, _value) = line.split_once(':')?;
+        let (directive_name, raw_value) = raw_directive.split_once(':')?;
 
         let kind = match directive_name {
             "normalize-stdout" => NormalizeKind::Stdout,
@@ -991,11 +991,9 @@ impl Config {
             _ => return None,
         };
 
-        // FIXME(Zalathar): The normalize rule parser should only care about
-        // the value part, not the "line" (which isn't even the whole line).
-        let Some((regex, replacement)) = parse_normalize_rule(line) else {
+        let Some((regex, replacement)) = parse_normalize_rule(raw_value) else {
             panic!(
-                "couldn't parse custom normalization rule: `{line}`\n\
+                "couldn't parse custom normalization rule: `{raw_directive}`\n\
                 help: expected syntax is: `{directive_name}: \"REGEX\" -> \"REPLACEMENT\"`"
             );
         };
@@ -1141,24 +1139,26 @@ enum NormalizeKind {
 /// Parses the regex and replacement values of a `//@ normalize-*` header,
 /// in the format:
 /// ```text
-/// normalize-*: "REGEX" -> "REPLACEMENT"
+/// "REGEX" -> "REPLACEMENT"
 /// ```
-fn parse_normalize_rule(header: &str) -> Option<(String, String)> {
+fn parse_normalize_rule(raw_value: &str) -> Option<(String, String)> {
     // FIXME: Support escaped double-quotes in strings.
     let captures = static_regex!(
         r#"(?x) # (verbose mode regex)
         ^
-        [^:\s]+:\s*             # (header name followed by colon)
+        \s*                     # (leading whitespace)
         "(?<regex>[^"]*)"       # "REGEX"
         \s+->\s+                # ->
         "(?<replacement>[^"]*)" # "REPLACEMENT"
         $
         "#
     )
-    .captures(header)?;
+    .captures(raw_value)?;
     let regex = captures["regex"].to_owned();
     let replacement = captures["replacement"].to_owned();
-    // FIXME: Support escaped new-line in strings.
+    // A `\n` sequence in the replacement becomes an actual newline.
+    // FIXME: Do unescaping in a less ad-hoc way, and perhaps support escaped
+    // backslashes and double-quotes.
     let replacement = replacement.replace("\\n", "\n");
     Some((regex, replacement))
 }

src/tools/compiletest/src/header/tests.rs

@@ -35,11 +35,14 @@ fn make_test_description<R: Read>(
 
 #[test]
 fn test_parse_normalize_rule() {
-    let good_data = &[(
-        r#"normalize-stderr-32bit: "something (32 bits)" -> "something ($WORD bits)""#,
-        "something (32 bits)",
-        "something ($WORD bits)",
-    )];
+    let good_data = &[
+        (
+            r#""something (32 bits)" -> "something ($WORD bits)""#,
+            "something (32 bits)",
+            "something ($WORD bits)",
+        ),
+        (r#"  " with whitespace"   ->   "    replacement""#, " with whitespace", "    replacement"),
+    ];
 
     for &(input, expected_regex, expected_replacement) in good_data {
         let parsed = parse_normalize_rule(input);
@@ -49,15 +52,15 @@ fn test_parse_normalize_rule() {
     }
 
     let bad_data = &[
-        r#"normalize-stderr-32bit "something (32 bits)" -> "something ($WORD bits)""#,
-        r#"normalize-stderr-16bit: something (16 bits) -> something ($WORD bits)"#,
-        r#"normalize-stderr-32bit: something (32 bits) -> something ($WORD bits)"#,
-        r#"normalize-stderr-32bit: "something (32 bits) -> something ($WORD bits)"#,
-        r#"normalize-stderr-32bit: "something (32 bits)" -> "something ($WORD bits)"#,
-        r#"normalize-stderr-32bit: "something (32 bits)" -> "something ($WORD bits)"."#,
+        r#"something (11 bits) -> something ($WORD bits)"#,
+        r#"something (12 bits) -> something ($WORD bits)"#,
+        r#""something (13 bits) -> something ($WORD bits)"#,
+        r#""something (14 bits)" -> "something ($WORD bits)"#,
+        r#""something (15 bits)" -> "something ($WORD bits)"."#,
     ];
 
     for &input in bad_data {
+        println!("- {input:?}");
         let parsed = parse_normalize_rule(input);
         assert_eq!(parsed, None);
     }

src/tools/tidy/src/allowed_run_make_makefiles.txt

@@ -1,8 +1,6 @@
-run-make/branch-protection-check-IBT/Makefile
 run-make/cat-and-grep-sanity-check/Makefile
 run-make/extern-fn-reachable/Makefile
 run-make/jobserver-error/Makefile
-run-make/libs-through-symlinks/Makefile
 run-make/split-debuginfo/Makefile
 run-make/symbol-mangling-hashed/Makefile
 run-make/translation/Makefile