Skip to content

repr(transparent): do not consider repr(C) types to be 1-ZST #147185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+582 −123
Open

repr(transparent): do not consider repr(C) types to be 1-ZST #147185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 68 additions & 43 deletions compiler/rustc_hir_analysis/src/check/check.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ use rustc_hir::def::{CtorKind, DefKind};
use rustc_hir::{LangItem, Node, attrs, find_attr, intravisit};
use rustc_infer::infer::{RegionVariableOrigin, TyCtxtInferExt};
use rustc_infer::traits::{Obligation, ObligationCauseCode, WellFormedLoc};
use rustc_lint_defs::builtin::{
REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS, UNSUPPORTED_CALLING_CONVENTIONS,
};
use rustc_lint_defs::builtin::{REPR_TRANSPARENT_NON_ZST_FIELDS, UNSUPPORTED_CALLING_CONVENTIONS};
use rustc_middle::hir::nested_filter;
use rustc_middle::middle::resolve_bound_vars::ResolvedArg;
use rustc_middle::middle::stability::EvalResult;
Expand Down Expand Up @@ -1511,31 +1509,46 @@ pub(super) fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, adt: ty::AdtDef<'tcx>)
}

let typing_env = ty::TypingEnv::non_body_analysis(tcx, adt.did());
// For each field, figure out if it's known to have "trivial" layout (i.e., is a 1-ZST), with
// "known" respecting #[non_exhaustive] attributes.
// For each field, figure out if it has "trivial" layout (i.e., is a 1-ZST).
// Even some 1-ZST fields are not allowed though, if they have `non_exhaustive` or private
// fields or `repr(C)` or uninhabited. We call those fields "unsuited".
struct FieldInfo<'tcx> {
span: Span,
trivial: bool,
unsuited: Option<UnsuitedInfo<'tcx>>,
}
struct UnsuitedInfo<'tcx> {
/// The source of the problem, a type that is found somewhere within the field type.
ty: Ty<'tcx>,
reason: UnsuitedReason,
}
enum UnsuitedReason {
NonExhaustive,
PrivateField,
ReprC,
}

let field_infos = adt.all_fields().map(|field| {
let ty = field.ty(tcx, GenericArgs::identity_for_item(tcx, field.did));
let layout = tcx.layout_of(typing_env.as_query_input(ty));
// We are currently checking the type this field came from, so it must be local
let span = tcx.hir_span_if_local(field.did).unwrap();
let trivial = layout.is_ok_and(|layout| layout.is_1zst());
if !trivial {
return (span, trivial, None);
// No need to even compute `unsuited`.
return FieldInfo { span, trivial, unsuited: None };
}
// Even some 1-ZST fields are not allowed though, if they have `non_exhaustive`.

fn check_non_exhaustive<'tcx>(
fn check_unsuited<'tcx>(
tcx: TyCtxt<'tcx>,
typing_env: ty::TypingEnv<'tcx>,
t: Ty<'tcx>,
) -> ControlFlow<(&'static str, DefId, GenericArgsRef<'tcx>, bool)> {
ty: Ty<'tcx>,
) -> ControlFlow<UnsuitedInfo<'tcx>> {
// We can encounter projections during traversal, so ensure the type is normalized.
let t = tcx.try_normalize_erasing_regions(typing_env, t).unwrap_or(t);
match t.kind() {
ty::Tuple(list) => {
list.iter().try_for_each(|t| check_non_exhaustive(tcx, typing_env, t))
}
ty::Array(ty, _) => check_non_exhaustive(tcx, typing_env, *ty),
let ty = tcx.try_normalize_erasing_regions(typing_env, ty).unwrap_or(ty);
match ty.kind() {
ty::Tuple(list) => list.iter().try_for_each(|t| check_unsuited(tcx, typing_env, t)),
ty::Array(ty, _) => check_unsuited(tcx, typing_env, *ty),
ty::Adt(def, args) => {
if !def.did().is_local()
&& !find_attr!(
Expand All @@ -1550,28 +1563,36 @@ pub(super) fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, adt: ty::AdtDef<'tcx>)
.any(ty::VariantDef::is_field_list_non_exhaustive);
let has_priv = def.all_fields().any(|f| !f.vis.is_public());
if non_exhaustive || has_priv {
return ControlFlow::Break((
def.descr(),
def.did(),
args,
non_exhaustive,
));
return ControlFlow::Break(UnsuitedInfo {
ty,
reason: if non_exhaustive {
UnsuitedReason::NonExhaustive
} else {
UnsuitedReason::PrivateField
},
});
}
}
if def.repr().c() {
return ControlFlow::Break(UnsuitedInfo {
ty,
reason: UnsuitedReason::ReprC,
});
}
def.all_fields()
.map(|field| field.ty(tcx, args))
.try_for_each(|t| check_non_exhaustive(tcx, typing_env, t))
.try_for_each(|t| check_unsuited(tcx, typing_env, t))
}
_ => ControlFlow::Continue(()),
}
}

(span, trivial, check_non_exhaustive(tcx, typing_env, ty).break_value())
FieldInfo { span, trivial, unsuited: check_unsuited(tcx, typing_env, ty).break_value() }
});

let non_trivial_fields = field_infos
.clone()
.filter_map(|(span, trivial, _non_exhaustive)| if !trivial { Some(span) } else { None });
.filter_map(|field| if !field.trivial { Some(field.span) } else { None });
let non_trivial_count = non_trivial_fields.clone().count();
if non_trivial_count >= 2 {
bad_non_zero_sized_fields(
Expand All @@ -1583,36 +1604,40 @@ pub(super) fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, adt: ty::AdtDef<'tcx>)
);
return;
}
let mut prev_non_exhaustive_1zst = false;
for (span, _trivial, non_exhaustive_1zst) in field_infos {
if let Some((descr, def_id, args, non_exhaustive)) = non_exhaustive_1zst {

let mut prev_unsuited_1zst = false;
for field in field_infos {
if let Some(unsuited) = field.unsuited {
assert!(field.trivial);
// If there are any non-trivial fields, then there can be no non-exhaustive 1-zsts.
// Otherwise, it's only an issue if there's >1 non-exhaustive 1-zst.
if non_trivial_count > 0 || prev_non_exhaustive_1zst {
if non_trivial_count > 0 || prev_unsuited_1zst {
tcx.node_span_lint(
REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS,
REPR_TRANSPARENT_NON_ZST_FIELDS,
tcx.local_def_id_to_hir_id(adt.did().expect_local()),
span,
field.span,
|lint| {
let title = match unsuited.reason {
UnsuitedReason::NonExhaustive => "external non-exhaustive types",
UnsuitedReason::PrivateField => "external types with private fields",
UnsuitedReason::ReprC => "`repr(C)` types",
};
lint.primary_message(
"zero-sized fields in `repr(transparent)` cannot \
contain external non-exhaustive types",
format!("zero-sized fields in `repr(transparent)` cannot contain {title}"),
);
let note = if non_exhaustive {
"is marked with `#[non_exhaustive]`"
} else {
"contains private fields"
let note = match unsuited.reason {
UnsuitedReason::NonExhaustive => "is marked with `#[non_exhaustive]`, so it could become non-zero-sized in the future.",
UnsuitedReason::PrivateField => "contains private fields, so it could become non-zero-sized in the future.",
UnsuitedReason::ReprC => "is a `#[repr(C)]` type, so it is not guaranteed to be zero-sized on all targets.",
};
let field_ty = tcx.def_path_str_with_args(def_id, args);
lint.note(format!(
"this {descr} contains `{field_ty}`, which {note}, \
and makes it not a breaking change to become \
non-zero-sized in the future."
"this field contains `{field_ty}`, which {note}",
field_ty = unsuited.ty,
));
},
)
);
} else {
prev_non_exhaustive_1zst = true;
prev_unsuited_1zst = true;
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_lint/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,10 @@ fn register_builtins(store: &mut LintStore) {
store.register_renamed("static_mut_ref", "static_mut_refs");
store.register_renamed("temporary_cstring_as_ptr", "dangling_pointers_from_temporaries");
store.register_renamed("elided_named_lifetimes", "mismatched_lifetime_syntaxes");
store.register_renamed(
"repr_transparent_external_private_fields",
"repr_transparent_non_zst_fields",
);

// These were moved to tool lints, but rustc still sees them when compiling normally, before
// tool lints are registered, so `check_tool_name_for_backwards_compat` doesn't work. Use
Expand Down
37 changes: 27 additions & 10 deletions compiler/rustc_lint_defs/src/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ declare_lint_pass! {
REFINING_IMPL_TRAIT_INTERNAL,
REFINING_IMPL_TRAIT_REACHABLE,
RENAMED_AND_REMOVED_LINTS,
REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS,
REPR_TRANSPARENT_NON_ZST_FIELDS,
RUST_2021_INCOMPATIBLE_CLOSURE_CAPTURES,
RUST_2021_INCOMPATIBLE_OR_PATTERNS,
RUST_2021_PREFIXES_INCOMPATIBLE_SYNTAX,
Expand Down Expand Up @@ -3011,19 +3011,23 @@ declare_lint! {
}

declare_lint! {
/// The `repr_transparent_external_private_fields` lint
/// The `repr_transparent_non_zst_fields` lint
/// detects types marked `#[repr(transparent)]` that (transitively)
/// contain an external ZST type marked `#[non_exhaustive]` or containing
/// private fields
/// contain a type that is not guaranteed to remain a ZST type under all configurations.
///
/// ### Example
///
/// ```rust,ignore (needs external crate)
/// #![deny(repr_transparent_external_private_fields)]
/// use foo::NonExhaustiveZst;
///
/// #[repr(C)]
/// struct CZst([u8; 0]);
///
/// #[repr(transparent)]
/// struct Bar(u32, ([u32; 0], NonExhaustiveZst));
/// #[repr(transparent)]
/// struct Baz(u32, CZst);
/// ```
///
/// This will produce:
Expand All @@ -3042,26 +3046,39 @@ declare_lint! {
/// | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/// = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
/// = note: for more information, see issue #78586 <https://github.com/rust-lang/rust/issues/78586>
/// = note: this struct contains `NonExhaustiveZst`, which is marked with `#[non_exhaustive]`, and makes it not a breaking change to become non-zero-sized in the future.
/// = note: this field contains `NonExhaustiveZst`, which is marked with `#[non_exhaustive]`, so it could become non-zero-sized in the future.
///
/// error: zero-sized fields in repr(transparent) cannot contain `#[repr(C)]` types
/// --> src/main.rs:5:28
/// |
/// 5 | struct Baz(u32, CZst);
/// | ^^^^
/// = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
/// = note: for more information, see issue #78586 <https://github.com/rust-lang/rust/issues/78586>
/// = note: this field contains `CZst`, which is a `#[repr(C)]` type, so it is not guaranteed to be zero-sized on all targets.
/// ```
///
/// ### Explanation
///
/// Previous, Rust accepted fields that contain external private zero-sized types,
/// even though it should not be a breaking change to add a non-zero-sized field to
/// that private type.
/// Previous, Rust accepted fields that contain external private zero-sized types, even though
/// those types could gain a non-zero-sized field in a future, semver-compatible update.
///
/// Rust also accepted fields that contain `repr(C)` zero-sized types, even though those types
/// are not guaranteed to be zero-sized on all targets, and even though those types can
/// make a difference for the ABI (and therefore cannot be ignored by `repr(transparent)`).
///
/// This is a [future-incompatible] lint to transition this
/// to a hard error in the future. See [issue #78586] for more details.
///
/// [issue #78586]: https://github.com/rust-lang/rust/issues/78586
/// [future-incompatible]: ../index.md#future-incompatible-lints
pub REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS,
Warn,
pub REPR_TRANSPARENT_NON_ZST_FIELDS,
Deny,
"transparent type contains an external ZST that is marked #[non_exhaustive] or contains private fields",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseError,
reference: "issue #78586 <https://github.com/rust-lang/rust/issues/78586>",
report_in_deps: true,
};
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint_defs/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ pub enum FutureIncompatibilityReason {
///
/// After a lint has been in this state for a while and you feel like it is ready to graduate
/// to warning everyone, consider setting [`FutureIncompatibleInfo::report_in_deps`] to true.
/// (see it's documentation for more guidance)
/// (see its documentation for more guidance)
///
/// After some period of time, lints with this variant can be turned into
/// hard errors (and the lint removed). Preferably when there is some
Expand Down
1 change: 1 addition & 0 deletions tests/ui/lint/improper-ctypes/lint-ctypes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub struct TransparentLifetime<'a>(*const u8, PhantomData<&'a ()>);
#[repr(transparent)]
pub struct TransparentUnit<U>(f32, PhantomData<U>);
#[repr(transparent)]
#[allow(repr_transparent_non_zst_fields)]
pub struct TransparentCustomZst(i32, ZeroSize);

#[repr(C)]
Expand Down
Loading
Loading