Skip to content

Commit

Permalink
Relax a debug assertion in codegen
Browse files Browse the repository at this point in the history
  • Loading branch information
compiler-errors committed Sep 30, 2024
1 parent c3ce4e6 commit eb75d20
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 20 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3448,6 +3448,7 @@ dependencies = [
"rustc_span",
"rustc_symbol_mangling",
"rustc_target",
"rustc_trait_selection",
"rustc_type_ir",
"serde_json",
"smallvec",
Expand Down
17 changes: 3 additions & 14 deletions compiler/rustc_codegen_cranelift/src/unsize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
42 changes: 36 additions & 6 deletions compiler/rustc_codegen_ssa/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ 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::TyCtxtInferExt;
use rustc_trait_selection::traits::{ObligationCause, ObligationCtxt};
use tracing::{debug, info};

use crate::assert_module_sources::CguReuse;
Expand Down Expand Up @@ -101,6 +103,38 @@ 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>,
data_a: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
data_b: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
) -> bool {
match (data_a.principal(), data_b.principal()) {
(Some(principal_a), Some(principal_b)) => {
let infcx = tcx.infer_ctxt().build();
let ocx = ObligationCtxt::new(&infcx);
let Ok(()) = ocx.sub(
&ObligationCause::dummy(),
ty::ParamEnv::reveal_all(),
principal_a,
principal_b,
) else {
return false;
};
ocx.select_all_or_error().is_empty()
}
(None, None) => true,
_ => false,
}
}

/// Retrieves the information we are losing (making dynamic) in an unsizing
/// adjustment.
///
Expand Down Expand Up @@ -133,12 +167,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}"
);

Expand Down
8 changes: 8 additions & 0 deletions tests/ui/codegen/sub-principals-in-codegen.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//@ build-pass

// Regression test for an overly aggressive assertion in #130855.

fn main() {
let subtype: &(dyn for<'a> Fn(&'a i32) -> &'a i32) = &|x| x;
let supertype: &(dyn Fn(&'static i32) -> &'static i32) = subtype;
}

0 comments on commit eb75d20

Please sign in to comment.