Merge branch 'master' into handle_potential_query_instability_lint_for_rustc_metadata

Author: ismailarilik

.github/pull_request_template.md

@@ -7,6 +7,6 @@ tracking issue or there are none, feel free to ignore this.
 This PR will get automatically assigned to a reviewer. In case you would like
 a specific user to review your work, you can assign it to them by using
 
-    r​? <reviewer name>
+    r\? <reviewer name> (with the `\` removed)
 -->
 <!-- homu-ignore:end -->

.github/workflows/ci.yml

@@ -212,6 +212,16 @@ jobs:
         # erroring about invalid credentials instead.
         if: github.event_name == 'push' || env.DEPLOY == '1' || env.DEPLOY_ALT == '1'
 
+      - name: upload job metrics to DataDog
+        if: needs.calculate_matrix.outputs.run_type != 'pr'
+        env:
+          DATADOG_SITE: datadoghq.com
+          DATADOG_API_KEY: ${{ secrets.DATADOG_API_KEY }}
+          DD_GITHUB_JOB_NAME: ${{ matrix.name }}
+        run: |
+          npm install -g @datadog/datadog-ci@^2.x.x
+          python3 src/ci/scripts/upload-build-metrics.py build/cpu-usage.csv
+
   # This job isused to tell bors the final status of the build, as there is no practical way to detect
   # when a workflow is successful listening to webhooks only in our current bors implementation (homu).
   outcome:

Cargo.lock

@@ -3451,6 +3451,7 @@ dependencies = [
  "rustc_span",
  "rustc_symbol_mangling",
  "rustc_target",
+ "rustc_trait_selection",
  "rustc_type_ir",
  "serde_json",
  "smallvec",

compiler/rustc_borrowck/src/dataflow.rs

@@ -1,5 +1,3 @@
-use std::fmt;
-
 use rustc_data_structures::fx::FxIndexMap;
 use rustc_data_structures::graph;
 use rustc_index::bit_set::BitSet;
@@ -425,10 +423,6 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> {
         Borrows { tcx, body, borrow_set, borrows_out_of_scope_at_location }
     }
 
-    pub fn location(&self, idx: BorrowIndex) -> &Location {
-        &self.borrow_set[idx].reserve_location
-    }
-
     /// Add all borrows to the kill set, if those borrows are out of scope at `location`.
     /// That means they went out of a nonlexical scope
     fn kill_loans_out_of_scope_at_location(
@@ -615,8 +609,4 @@ impl<'tcx> rustc_mir_dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
     }
 }
 
-impl DebugWithContext<Borrows<'_, '_>> for BorrowIndex {
-    fn fmt_with(&self, ctxt: &Borrows<'_, '_>, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{:?}", ctxt.location(*self))
-    }
-}
+impl<C> DebugWithContext<C> for BorrowIndex {}

compiler/rustc_codegen_cranelift/src/unsize.rs

@@ -2,6 +2,7 @@
 //!
 //! [`PointerCoercion::Unsize`]: `rustc_middle::ty::adjustment::PointerCoercion::Unsize`
 
+use rustc_codegen_ssa::base::validate_trivial_unsize;
 use rustc_middle::ty::print::{with_no_trimmed_paths, with_no_visible_paths};
 
 use crate::base::codegen_panic_nounwind;
@@ -34,20 +35,8 @@ pub(crate) fn unsized_info<'tcx>(
             let old_info =
                 old_info.expect("unsized_info: missing old info for trait upcasting coercion");
             if data_a.principal_def_id() == data_b.principal_def_id() {
-                // Codegen takes advantage of the additional assumption, where if the
-                // principal trait def id of what's being casted doesn't change,
-                // then we don't need to adjust the vtable at all. This
-                // corresponds to the fact that `dyn Tr<A>: Unsize<dyn Tr<B>>`
-                // requires that `A = B`; we don't allow *upcasting* objects
-                // between the same trait with different args. If we, for
-                // some reason, were to relax the `Unsize` trait, it could become
-                // unsound, so let's assert here that the trait refs are *equal*.
-                //
-                // We can use `assert_eq` because the binders should have been anonymized,
-                // and because higher-ranked equality now requires the binders are equal.
-                debug_assert_eq!(
-                    data_a.principal(),
-                    data_b.principal(),
+                debug_assert!(
+                    validate_trivial_unsize(fx.tcx, data_a, data_b),
                     "NOP unsize vtable changed principal trait ref: {data_a} -> {data_b}"
                 );
                 return old_info;

compiler/rustc_codegen_ssa/Cargo.toml

@@ -34,6 +34,7 @@ rustc_session = { path = "../rustc_session" }
 rustc_span = { path = "../rustc_span" }
 rustc_symbol_mangling = { path = "../rustc_symbol_mangling" }
 rustc_target = { path = "../rustc_target" }
+rustc_trait_selection = { path = "../rustc_trait_selection" }
 rustc_type_ir = { path = "../rustc_type_ir" }
 serde_json = "1.0.59"
 smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -2959,11 +2959,12 @@ pub(crate) fn are_upstream_rust_objects_already_included(sess: &Session) -> bool
     }
 }
 
-/// We need to communicate four things to the linker on Apple/Darwin targets:
+/// We need to communicate five things to the linker on Apple/Darwin targets:
 /// - The architecture.
 /// - The operating system (and that it's an Apple platform).
-/// - The deployment target.
 /// - The environment / ABI.
+/// - The deployment target.
+/// - The SDK version.
 fn add_apple_link_args(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavor) {
     if !sess.target.is_like_osx {
         return;
@@ -3039,7 +3040,38 @@ fn add_apple_link_args(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavo
         let (major, minor, patch) = current_apple_deployment_target(&sess.target);
         let min_version = format!("{major}.{minor}.{patch}");
 
-        // Lie about the SDK version, we don't know it here
+        // The SDK version is used at runtime when compiling with a newer SDK / version of Xcode:
+        // - By dyld to give extra warnings and errors, see e.g.:
+        //   <https://github.com/apple-oss-distributions/dyld/blob/dyld-1165.3/common/MachOFile.cpp#L3029>
+        //   <https://github.com/apple-oss-distributions/dyld/blob/dyld-1165.3/common/MachOFile.cpp#L3738-L3857>
+        // - By system frameworks to change certain behaviour. For example, the default value of
+        //   `-[NSView wantsBestResolutionOpenGLSurface]` is `YES` when the SDK version is >= 10.15.
+        //   <https://developer.apple.com/documentation/appkit/nsview/1414938-wantsbestresolutionopenglsurface?language=objc>
+        //
+        // We do not currently know the actual SDK version though, so we have a few options:
+        // 1. Use the minimum version supported by rustc.
+        // 2. Use the same as the deployment target.
+        // 3. Use an arbitary recent version.
+        // 4. Omit the version.
+        //
+        // The first option is too low / too conservative, and means that users will not get the
+        // same behaviour from a binary compiled with rustc as with one compiled by clang.
+        //
+        // The second option is similarly conservative, and also wrong since if the user specified a
+        // higher deployment target than the SDK they're compiling/linking with, the runtime might
+        // make invalid assumptions about the capabilities of the binary.
+        //
+        // The third option requires that `rustc` is periodically kept up to date with Apple's SDK
+        // version, and is also wrong for similar reasons as above.
+        //
+        // The fourth option is bad because while `ld`, `otool`, `vtool` and such understand it to
+        // mean "absent" or `n/a`, dyld doesn't actually understand it, and will end up interpreting
+        // it as 0.0, which is again too low/conservative.
+        //
+        // Currently, we lie about the SDK version, and choose the second option.
+        //
+        // FIXME(madsmtm): Parse the SDK version from the SDK root instead.
+        // <https://github.com/rust-lang/rust/issues/129432>
         let sdk_version = &*min_version;
 
         // From the man page for ld64 (`man ld`):
@@ -3053,11 +3085,13 @@ fn add_apple_link_args(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavo
         cmd.link_args(&["-platform_version", platform_name, &*min_version, sdk_version]);
     } else {
         // cc == Cc::Yes
+        //
         // We'd _like_ to use `-target` everywhere, since that can uniquely
-        // communicate all the required details, but that doesn't work on GCC,
-        // and since we don't know whether the `cc` compiler is Clang, GCC, or
-        // something else, we fall back to other options that also work on GCC
-        // when compiling for macOS.
+        // communicate all the required details except for the SDK version
+        // (which is read by Clang itself from the SDKROOT), but that doesn't
+        // work on GCC, and since we don't know whether the `cc` compiler is
+        // Clang, GCC, or something else, we fall back to other options that
+        // also work on GCC when compiling for macOS.
         //
         // Targets other than macOS are ill-supported by GCC (it doesn't even
         // support e.g. `-miphoneos-version-min`), so in those cases we can

compiler/rustc_codegen_ssa/src/back/metadata.rs

@@ -402,13 +402,17 @@ fn macho_object_build_version_for_target(target: &Target) -> object::write::Mach
     let platform =
         rustc_target::spec::current_apple_platform(target).expect("unknown Apple target OS");
     let min_os = rustc_target::spec::current_apple_deployment_target(target);
-    let (sdk_major, sdk_minor) =
-        rustc_target::spec::current_apple_sdk_version(platform).expect("unknown Apple target OS");
 
     let mut build_version = object::write::MachOBuildVersion::default();
     build_version.platform = platform;
     build_version.minos = pack_version(min_os);
-    build_version.sdk = pack_version((sdk_major, sdk_minor, 0));
+    // The version here does not _really_ matter, since it is only used at runtime, and we specify
+    // it when linking the final binary, so we will omit the version. This is also what LLVM does,
+    // and the tooling also allows this (and shows the SDK version as `n/a`). Finally, it is the
+    // semantically correct choice, as the SDK has not influenced the binary generated by rustc at
+    // this point in time.
+    build_version.sdk = 0;
+
     build_version
 }
 

compiler/rustc_codegen_ssa/src/base.rs

@@ -27,6 +27,9 @@ use rustc_session::config::{self, CrateType, EntryFnType, OptLevel, OutputType};
 use rustc_span::symbol::sym;
 use rustc_span::{DUMMY_SP, Symbol};
 use rustc_target::abi::FIRST_VARIANT;
+use rustc_trait_selection::infer::at::ToTrace;
+use rustc_trait_selection::infer::{BoundRegionConversionTime, TyCtxtInferExt};
+use rustc_trait_selection::traits::{ObligationCause, ObligationCtxt};
 use tracing::{debug, info};
 
 use crate::assert_module_sources::CguReuse;
@@ -101,6 +104,54 @@ pub fn compare_simd_types<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
     bx.sext(cmp, ret_ty)
 }
 
+/// Codegen takes advantage of the additional assumption, where if the
+/// principal trait def id of what's being casted doesn't change,
+/// then we don't need to adjust the vtable at all. This
+/// corresponds to the fact that `dyn Tr<A>: Unsize<dyn Tr<B>>`
+/// requires that `A = B`; we don't allow *upcasting* objects
+/// between the same trait with different args. If we, for
+/// some reason, were to relax the `Unsize` trait, it could become
+/// unsound, so let's validate here that the trait refs are subtypes.
+pub fn validate_trivial_unsize<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    source_data: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
+    target_data: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
+) -> bool {
+    match (source_data.principal(), target_data.principal()) {
+        (Some(hr_source_principal), Some(hr_target_principal)) => {
+            let infcx = tcx.infer_ctxt().build();
+            let universe = infcx.universe();
+            let ocx = ObligationCtxt::new(&infcx);
+            infcx.enter_forall(hr_target_principal, |target_principal| {
+                let source_principal = infcx.instantiate_binder_with_fresh_vars(
+                    DUMMY_SP,
+                    BoundRegionConversionTime::HigherRankedType,
+                    hr_source_principal,
+                );
+                let Ok(()) = ocx.eq_trace(
+                    &ObligationCause::dummy(),
+                    ty::ParamEnv::reveal_all(),
+                    ToTrace::to_trace(
+                        &ObligationCause::dummy(),
+                        hr_target_principal,
+                        hr_source_principal,
+                    ),
+                    target_principal,
+                    source_principal,
+                ) else {
+                    return false;
+                };
+                if !ocx.select_all_or_error().is_empty() {
+                    return false;
+                }
+                infcx.leak_check(universe, None).is_ok()
+            })
+        }
+        (None, None) => true,
+        _ => false,
+    }
+}
+
 /// Retrieves the information we are losing (making dynamic) in an unsizing
 /// adjustment.
 ///
@@ -133,12 +184,8 @@ fn unsized_info<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
                 // between the same trait with different args. If we, for
                 // some reason, were to relax the `Unsize` trait, it could become
                 // unsound, so let's assert here that the trait refs are *equal*.
-                //
-                // We can use `assert_eq` because the binders should have been anonymized,
-                // and because higher-ranked equality now requires the binders are equal.
-                debug_assert_eq!(
-                    data_a.principal(),
-                    data_b.principal(),
+                debug_assert!(
+                    validate_trivial_unsize(cx.tcx(), data_a, data_b),
                     "NOP unsize vtable changed principal trait ref: {data_a} -> {data_b}"
                 );
 

compiler/rustc_const_eval/src/const_eval/dummy_machine.rs

@@ -6,7 +6,8 @@ use rustc_middle::{bug, span_bug, ty};
 use rustc_span::def_id::DefId;
 
 use crate::interpret::{
-    self, HasStaticRootDefId, ImmTy, Immediate, InterpCx, PointerArithmetic, throw_machine_stop,
+    self, HasStaticRootDefId, ImmTy, Immediate, InterpCx, PointerArithmetic, interp_ok,
+    throw_machine_stop,
 };
 
 /// Macro for machine-specific `InterpError` without allocation.
@@ -79,7 +80,7 @@ impl<'tcx> interpret::Machine<'tcx> for DummyMachine {
             throw_machine_stop_str!("can't access mutable globals in ConstProp");
         }
 
-        Ok(())
+        interp_ok(())
     }
 
     fn find_mir_or_eval_fn(
@@ -127,7 +128,7 @@ impl<'tcx> interpret::Machine<'tcx> for DummyMachine {
         right: &interpret::ImmTy<'tcx, Self::Provenance>,
     ) -> interpret::InterpResult<'tcx, ImmTy<'tcx, Self::Provenance>> {
         use rustc_middle::mir::BinOp::*;
-        Ok(match bin_op {
+        interp_ok(match bin_op {
             Eq | Ne | Lt | Le | Gt | Ge => {
                 // Types can differ, e.g. fn ptrs with different `for`.
                 assert_eq!(left.layout.abi, right.layout.abi);