Skip to content

Commit

Permalink
Rollup merge of rust-lang#133226 - compiler-errors:opt-in-pointer-lik…
Browse files Browse the repository at this point in the history
…e, r=lcnr

Make `PointerLike` opt-in instead of built-in

The `PointerLike` trait currently is a built-in trait that computes the layout of the type. This is a bit problematic, because types implement this trait automatically. Since this can be broken due to semver-compatible changes to a type's layout, this is undesirable. Also, calling `layout_of` in the trait system also causes cycles.

This PR makes the trait implemented via regular impls, and adds additional validation on top to make sure that those impls are valid. This could eventually be `derive()`d for custom smart pointers, and we can trust *that* as a semver promise rather than risking library authors accidentally breaking it.

On the other hand, we may never expose `PointerLike`, but at least now the implementation doesn't invoke `layout_of` which could cause ICEs or cause cycles.

Right now for a `PointerLike` impl to be valid, it must be an ADT that is `repr(transparent)` and the non-1zst field needs to implement `PointerLike`. There are also some primitive impls for `&T`/ `&mut T`/`*const T`/`*mut T`/`Box<T>`.
  • Loading branch information
matthiaskrgr authored Nov 20, 2024
2 parents 4f53fa2 + 228068b commit eb9fbb0
Show file tree
Hide file tree
Showing 25 changed files with 195 additions and 156 deletions.
85 changes: 71 additions & 14 deletions compiler/rustc_hir_analysis/src/coherence/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,22 +37,19 @@ pub(super) fn check_trait<'tcx>(
) -> Result<(), ErrorGuaranteed> {
let lang_items = tcx.lang_items();
let checker = Checker { tcx, trait_def_id, impl_def_id, impl_header };
let mut res = checker.check(lang_items.drop_trait(), visit_implementation_of_drop);
res = res.and(checker.check(lang_items.copy_trait(), visit_implementation_of_copy));
res = res.and(checker.check(lang_items.const_param_ty_trait(), |checker| {
checker.check(lang_items.drop_trait(), visit_implementation_of_drop)?;
checker.check(lang_items.copy_trait(), visit_implementation_of_copy)?;
checker.check(lang_items.const_param_ty_trait(), |checker| {
visit_implementation_of_const_param_ty(checker, LangItem::ConstParamTy)
}));
res = res.and(checker.check(lang_items.unsized_const_param_ty_trait(), |checker| {
})?;
checker.check(lang_items.unsized_const_param_ty_trait(), |checker| {
visit_implementation_of_const_param_ty(checker, LangItem::UnsizedConstParamTy)
}));

res = res.and(
checker.check(lang_items.coerce_unsized_trait(), visit_implementation_of_coerce_unsized),
);
res.and(
checker
.check(lang_items.dispatch_from_dyn_trait(), visit_implementation_of_dispatch_from_dyn),
)
})?;
checker.check(lang_items.coerce_unsized_trait(), visit_implementation_of_coerce_unsized)?;
checker
.check(lang_items.dispatch_from_dyn_trait(), visit_implementation_of_dispatch_from_dyn)?;
checker.check(lang_items.pointer_like(), visit_implementation_of_pointer_like)?;
Ok(())
}

struct Checker<'tcx> {
Expand Down Expand Up @@ -663,3 +660,63 @@ fn infringing_fields_error<'tcx>(

err.emit()
}

fn visit_implementation_of_pointer_like(checker: &Checker<'_>) -> Result<(), ErrorGuaranteed> {
let tcx = checker.tcx;
let typing_env = ty::TypingEnv::non_body_analysis(tcx, checker.impl_def_id);
let impl_span = tcx.def_span(checker.impl_def_id);
let self_ty = tcx.impl_trait_ref(checker.impl_def_id).unwrap().instantiate_identity().self_ty();

// If an ADT is repr(transparent)...
if let ty::Adt(def, args) = *self_ty.kind()
&& def.repr().transparent()
{
// FIXME(compiler-errors): This should and could be deduplicated into a query.
// Find the nontrivial field.
let adt_typing_env = ty::TypingEnv::non_body_analysis(tcx, def.did());
let nontrivial_field = def.all_fields().find(|field_def| {
let field_ty = tcx.type_of(field_def.did).instantiate_identity();
!tcx.layout_of(adt_typing_env.as_query_input(field_ty))
.is_ok_and(|layout| layout.layout.is_1zst())
});

if let Some(nontrivial_field) = nontrivial_field {
// Check that the nontrivial field implements `PointerLike`.
let nontrivial_field = nontrivial_field.ty(tcx, args);
let (infcx, param_env) = tcx.infer_ctxt().build_with_typing_env(typing_env);
let ocx = ObligationCtxt::new(&infcx);
ocx.register_bound(
ObligationCause::misc(impl_span, checker.impl_def_id),
param_env,
nontrivial_field,
tcx.lang_items().pointer_like().unwrap(),
);
// FIXME(dyn-star): We should regionck this implementation.
if ocx.select_all_or_error().is_empty() {
return Ok(());
}
}
}

let is_permitted_primitive = match *self_ty.kind() {
ty::Adt(def, _) => def.is_box(),
ty::Uint(..) | ty::Int(..) | ty::RawPtr(..) | ty::Ref(..) | ty::FnPtr(..) => true,
_ => false,
};

if is_permitted_primitive
&& let Ok(layout) = tcx.layout_of(typing_env.as_query_input(self_ty))
&& layout.layout.is_pointer_like(&tcx.data_layout)
{
return Ok(());
}

Err(tcx
.dcx()
.struct_span_err(
impl_span,
"implementation must be applied to type that has the same ABI as a pointer, \
or is `repr(transparent)` and whose field is `PointerLike`",
)
.emit())
}
14 changes: 0 additions & 14 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,19 +602,6 @@ impl<'tcx> Interner for TyCtxt<'tcx> {
self.coroutine_is_async_gen(coroutine_def_id)
}

// We don't use `TypingEnv` here as it's only defined in `rustc_middle` and
// `rustc_next_trait_solver` shouldn't have to know about it.
fn layout_is_pointer_like(
self,
typing_mode: ty::TypingMode<'tcx>,
param_env: ty::ParamEnv<'tcx>,
ty: Ty<'tcx>,
) -> bool {
let typing_env = ty::TypingEnv { typing_mode, param_env };
self.layout_of(self.erase_regions(typing_env).as_query_input(self.erase_regions(ty)))
.is_ok_and(|layout| layout.layout.is_pointer_like(&self.data_layout))
}

type UnsizingParams = &'tcx rustc_index::bit_set::BitSet<u32>;
fn unsizing_params_for_adt(self, adt_def_id: DefId) -> Self::UnsizingParams {
self.unsizing_params_for_adt(adt_def_id)
Expand Down Expand Up @@ -688,7 +675,6 @@ bidirectional_lang_item_map! {
Metadata,
Option,
PointeeTrait,
PointerLike,
Poll,
Sized,
TransmuteTrait,
Expand Down
10 changes: 0 additions & 10 deletions compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,6 @@ where
goal: Goal<I, Self>,
) -> Result<Candidate<I>, NoSolution>;

/// A type is `PointerLike` if we can compute its layout, and that layout
/// matches the layout of `usize`.
fn consider_builtin_pointer_like_candidate(
ecx: &mut EvalCtxt<'_, D>,
goal: Goal<I, Self>,
) -> Result<Candidate<I>, NoSolution>;

/// A type is a `FnPtr` if it is of `FnPtr` type.
fn consider_builtin_fn_ptr_trait_candidate(
ecx: &mut EvalCtxt<'_, D>,
Expand Down Expand Up @@ -449,9 +442,6 @@ where
ty::ClosureKind::FnOnce,
)
}
Some(TraitSolverLangItem::PointerLike) => {
G::consider_builtin_pointer_like_candidate(self, goal)
}
Some(TraitSolverLangItem::FnPtrTrait) => {
G::consider_builtin_fn_ptr_trait_candidate(self, goal)
}
Expand Down
7 changes: 0 additions & 7 deletions compiler/rustc_next_trait_solver/src/solve/effect_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,13 +210,6 @@ where
Err(NoSolution)
}

fn consider_builtin_pointer_like_candidate(
_ecx: &mut EvalCtxt<'_, D>,
_goal: Goal<I, Self>,
) -> Result<Candidate<I>, NoSolution> {
unreachable!("PointerLike is not const")
}

fn consider_builtin_fn_ptr_trait_candidate(
_ecx: &mut EvalCtxt<'_, D>,
_goal: Goal<I, Self>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -363,13 +363,6 @@ where
panic!("`Copy`/`Clone` does not have an associated type: {:?}", goal);
}

fn consider_builtin_pointer_like_candidate(
_ecx: &mut EvalCtxt<'_, D>,
goal: Goal<I, Self>,
) -> Result<Candidate<I>, NoSolution> {
panic!("`PointerLike` does not have an associated type: {:?}", goal);
}

fn consider_builtin_fn_ptr_trait_candidate(
_ecx: &mut EvalCtxt<'_, D>,
goal: Goal<I, Self>,
Expand Down
26 changes: 0 additions & 26 deletions compiler/rustc_next_trait_solver/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,32 +248,6 @@ where
)
}

fn consider_builtin_pointer_like_candidate(
ecx: &mut EvalCtxt<'_, D>,
goal: Goal<I, Self>,
) -> Result<Candidate<I>, NoSolution> {
if goal.predicate.polarity != ty::PredicatePolarity::Positive {
return Err(NoSolution);
}

let cx = ecx.cx();
// But if there are inference variables, we have to wait until it's resolved.
if (goal.param_env, goal.predicate.self_ty()).has_non_region_infer() {
return ecx.forced_ambiguity(MaybeCause::Ambiguity);
}

if cx.layout_is_pointer_like(
ecx.typing_mode(goal.param_env),
goal.param_env,
goal.predicate.self_ty(),
) {
ecx.probe_builtin_trait_candidate(BuiltinImplSource::Misc)
.enter(|ecx| ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes))
} else {
Err(NoSolution)
}
}

fn consider_builtin_fn_ptr_trait_candidate(
ecx: &mut EvalCtxt<'_, D>,
goal: Goal<I, Self>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
self.assemble_candidates_for_transmutability(obligation, &mut candidates);
} else if tcx.is_lang_item(def_id, LangItem::Tuple) {
self.assemble_candidate_for_tuple(obligation, &mut candidates);
} else if tcx.is_lang_item(def_id, LangItem::PointerLike) {
self.assemble_candidate_for_pointer_like(obligation, &mut candidates);
} else if tcx.is_lang_item(def_id, LangItem::FnPtrTrait) {
self.assemble_candidates_for_fn_ptr_trait(obligation, &mut candidates);
} else {
Expand Down Expand Up @@ -1216,35 +1214,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}
}

fn assemble_candidate_for_pointer_like(
&mut self,
obligation: &PolyTraitObligation<'tcx>,
candidates: &mut SelectionCandidateSet<'tcx>,
) {
// The regions of a type don't affect the size of the type
let tcx = self.tcx();
let self_ty = tcx.instantiate_bound_regions_with_erased(obligation.predicate.self_ty());

// But if there are inference variables, we have to wait until it's resolved.
if (obligation.param_env, self_ty).has_non_region_infer() {
candidates.ambiguous = true;
return;
}

// We should erase regions from both the param-env and type, since both
// may have infer regions. Specifically, after canonicalizing and instantiating,
// early bound regions turn into region vars in both the new and old solver.
let key = self.infcx.pseudo_canonicalize_query(
tcx.erase_regions(obligation.param_env),
tcx.erase_regions(self_ty),
);
if let Ok(layout) = tcx.layout_of(key)
&& layout.layout.is_pointer_like(&tcx.data_layout)
{
candidates.vec.push(BuiltinCandidate { has_nested: false });
}
}

fn assemble_candidates_for_fn_ptr_trait(
&mut self,
obligation: &PolyTraitObligation<'tcx>,
Expand Down
9 changes: 1 addition & 8 deletions compiler/rustc_type_ir/src/interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::lang_items::TraitSolverLangItem;
use crate::relate::Relate;
use crate::solve::{CanonicalInput, ExternalConstraintsData, PredefinedOpaquesData, QueryResult};
use crate::visit::{Flags, TypeSuperVisitable, TypeVisitable};
use crate::{self as ty, TypingMode, search_graph};
use crate::{self as ty, search_graph};

pub trait Interner:
Sized
Expand Down Expand Up @@ -278,13 +278,6 @@ pub trait Interner:
fn coroutine_is_gen(self, coroutine_def_id: Self::DefId) -> bool;
fn coroutine_is_async_gen(self, coroutine_def_id: Self::DefId) -> bool;

fn layout_is_pointer_like(
self,
typing_mode: TypingMode<Self>,
param_env: Self::ParamEnv,
ty: Self::Ty,
) -> bool;

type UnsizingParams: Deref<Target = BitSet<u32>>;
fn unsizing_params_for_adt(self, adt_def_id: Self::DefId) -> Self::UnsizingParams;

Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_type_ir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ pub enum TraitSolverLangItem {
Metadata,
Option,
PointeeTrait,
PointerLike,
Poll,
Sized,
TransmuteTrait,
Expand Down
6 changes: 6 additions & 0 deletions library/alloc/src/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ use core::error::{self, Error};
use core::fmt;
use core::future::Future;
use core::hash::{Hash, Hasher};
#[cfg(not(bootstrap))]
use core::marker::PointerLike;
use core::marker::{Tuple, Unsize};
use core::mem::{self, SizedTypeProperties};
use core::ops::{
Expand Down Expand Up @@ -2131,3 +2133,7 @@ impl<E: Error> Error for Box<E> {
Error::provide(&**self, request);
}
}

#[cfg(not(bootstrap))]
#[unstable(feature = "pointer_like_trait", issue = "none")]
impl<T> PointerLike for Box<T> {}
1 change: 1 addition & 0 deletions library/alloc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@
#![feature(panic_internals)]
#![feature(pattern)]
#![feature(pin_coerce_unsized_trait)]
#![feature(pointer_like_trait)]
#![feature(ptr_internals)]
#![feature(ptr_metadata)]
#![feature(ptr_sub_ptr)]
Expand Down
12 changes: 12 additions & 0 deletions library/core/src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -981,6 +981,18 @@ pub trait Tuple {}
)]
pub trait PointerLike {}

#[cfg(not(bootstrap))]
marker_impls! {
#[unstable(feature = "pointer_like_trait", issue = "none")]
PointerLike for
usize,
{T} &T,
{T} &mut T,
{T} *const T,
{T} *mut T,
{T: PointerLike} crate::pin::Pin<T>,
}

/// A marker for types which can be used as types of `const` generic parameters.
///
/// These types must have a proper equivalence relation (`Eq`) and it must be automatically
Expand Down
16 changes: 0 additions & 16 deletions tests/crashes/113280.rs

This file was deleted.

8 changes: 0 additions & 8 deletions tests/crashes/127676.rs

This file was deleted.

9 changes: 9 additions & 0 deletions tests/ui/dyn-star/async-block-dyn-star.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//@ edition:2018

#![feature(dyn_star, const_async_blocks)]
//~^ WARN the feature `dyn_star` is incomplete

static S: dyn* Send + Sync = async { 42 };
//~^ needs to have the same ABI as a pointer

pub fn main() {}
20 changes: 20 additions & 0 deletions tests/ui/dyn-star/async-block-dyn-star.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
warning: the feature `dyn_star` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/async-block-dyn-star.rs:3:12
|
LL | #![feature(dyn_star, const_async_blocks)]
| ^^^^^^^^
|
= note: see issue #102425 <https://github.com/rust-lang/rust/issues/102425> for more information
= note: `#[warn(incomplete_features)]` on by default

error[E0277]: `{async block@$DIR/async-block-dyn-star.rs:6:30: 6:35}` needs to have the same ABI as a pointer
--> $DIR/async-block-dyn-star.rs:6:30
|
LL | static S: dyn* Send + Sync = async { 42 };
| ^^^^^^^^^^^^ `{async block@$DIR/async-block-dyn-star.rs:6:30: 6:35}` needs to be a pointer-like type
|
= help: the trait `PointerLike` is not implemented for `{async block@$DIR/async-block-dyn-star.rs:6:30: 6:35}`

error: aborting due to 1 previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0277`.
Loading

0 comments on commit eb9fbb0

Please sign in to comment.