Skip to content

Commit 3216a49

Browse files
compiler: Remove power alignment lint
The "power alignment" lint was based on a false premise: clang implemented a divergent layout from LLVM's given datalayout. Unfortunately, rustc, and other LLVM-based, non-clang frontends, like flang, assumed LLVM's AIX datalayout was correct. Exceedingly confusingly worded documentation, and things being hidden behind "preferred" alignment, obfuscated the matter for some time. It seemed insoluble or absurd to solve, thus resulting in the creation of the "power alignment" lint as a compromise, intermediate solution. Eventually, in llvm-project@b7c0452a9a3d895b14ec7c5735e4e4ddc311edb3 LLVM's AIX layout was fixed. Now rustc will have mostly-correct layouts on AIX from #149880 onwards. We may need a new lint that captures the remaining C layout problems, for AIX and other targets as well, but that is a larger conversation. For now, remove the incorrect lint.
1 parent 021fc25 commit 3216a49

3 files changed

Lines changed: 4 additions & 147 deletions

File tree

compiler/rustc_lint/messages.ftl

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,7 +1063,5 @@ lint_useless_ptr_null_checks_fn_ret = returned pointer of `{$fn_name}` call is n
10631063
lint_useless_ptr_null_checks_ref = references are not nullable, so checking them for null will always return false
10641064
.label = expression has type `{$orig_ty}`
10651065
1066-
lint_uses_power_alignment = repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type
1067-
10681066
lint_variant_size_differences =
10691067
enum variant is more than three times larger ({$largest} bytes) than the next largest

compiler/rustc_lint/src/lints.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1775,10 +1775,6 @@ pub(crate) struct TooLargeCharCast {
17751775
pub literal: u128,
17761776
}
17771777

1778-
#[derive(LintDiagnostic)]
1779-
#[diag(lint_uses_power_alignment)]
1780-
pub(crate) struct UsesPowerAlignment;
1781-
17821778
#[derive(LintDiagnostic)]
17831779
#[diag(lint_unused_comparisons)]
17841780
pub(crate) struct UnusedComparisons;

compiler/rustc_lint/src/types/improper_ctypes.rs

Lines changed: 4 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -2,25 +2,23 @@ use std::iter;
22
use std::ops::ControlFlow;
33

44
use bitflags::bitflags;
5-
use rustc_abi::VariantIdx;
65
use rustc_data_structures::fx::FxHashSet;
76
use rustc_errors::DiagMessage;
87
use rustc_hir::def::CtorKind;
98
use rustc_hir::intravisit::VisitorExt;
109
use rustc_hir::{self as hir, AmbigArg};
1110
use rustc_middle::bug;
1211
use rustc_middle::ty::{
13-
self, Adt, AdtDef, AdtKind, GenericArgsRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable,
12+
self, AdtDef, AdtKind, GenericArgsRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable,
1413
TypeVisitableExt,
1514
};
1615
use rustc_session::{declare_lint, declare_lint_pass};
1716
use rustc_span::def_id::LocalDefId;
1817
use rustc_span::{Span, sym};
19-
use rustc_target::spec::Os;
2018
use tracing::debug;
2119

2220
use super::repr_nullable_ptr;
23-
use crate::lints::{ImproperCTypes, UsesPowerAlignment};
21+
use crate::lints::ImproperCTypes;
2422
use crate::{LateContext, LateLintPass, LintContext, fluent_generated as fluent};
2523

2624
declare_lint! {
@@ -77,65 +75,9 @@ declare_lint! {
7775
"proper use of libc types in foreign item definitions"
7876
}
7977

80-
declare_lint! {
81-
/// The `uses_power_alignment` lint detects specific `repr(C)`
82-
/// aggregates on AIX.
83-
/// In its platform C ABI, AIX uses the "power" (as in PowerPC) alignment
84-
/// rule (detailed in https://www.ibm.com/docs/en/xl-c-and-cpp-aix/16.1?topic=data-using-alignment-modes#alignment),
85-
/// which can also be set for XLC by `#pragma align(power)` or
86-
/// `-qalign=power`. Aggregates with a floating-point type as the
87-
/// recursively first field (as in "at offset 0") modify the layout of
88-
/// *subsequent* fields of the associated structs to use an alignment value
89-
/// where the floating-point type is aligned on a 4-byte boundary.
90-
///
91-
/// Effectively, subsequent floating-point fields act as-if they are `repr(packed(4))`. This
92-
/// would be unsound to do in a `repr(C)` type without all the restrictions that come with
93-
/// `repr(packed)`. Rust instead chooses a layout that maintains soundness of Rust code, at the
94-
/// expense of incompatibility with C code.
95-
///
96-
/// ### Example
97-
///
98-
/// ```rust,ignore (fails on non-powerpc64-ibm-aix)
99-
/// #[repr(C)]
100-
/// pub struct Floats {
101-
/// a: f64,
102-
/// b: u8,
103-
/// c: f64,
104-
/// }
105-
/// ```
106-
///
107-
/// This will produce:
108-
///
109-
/// ```text
110-
/// warning: repr(C) does not follow the power alignment rule. This may affect platform C ABI compatibility for this type
111-
/// --> <source>:5:3
112-
/// |
113-
/// 5 | c: f64,
114-
/// | ^^^^^^
115-
/// |
116-
/// = note: `#[warn(uses_power_alignment)]` on by default
117-
/// ```
118-
///
119-
/// ### Explanation
120-
///
121-
/// The power alignment rule specifies that the above struct has the
122-
/// following alignment:
123-
/// - offset_of!(Floats, a) == 0
124-
/// - offset_of!(Floats, b) == 8
125-
/// - offset_of!(Floats, c) == 12
126-
///
127-
/// However, Rust currently aligns `c` at `offset_of!(Floats, c) == 16`.
128-
/// Using offset 12 would be unsound since `f64` generally must be 8-aligned on this target.
129-
/// Thus, a warning is produced for the above struct.
130-
USES_POWER_ALIGNMENT,
131-
Warn,
132-
"Structs do not follow the power alignment rule under repr(C)"
133-
}
134-
13578
declare_lint_pass!(ImproperCTypesLint => [
13679
IMPROPER_CTYPES,
13780
IMPROPER_CTYPES_DEFINITIONS,
138-
USES_POWER_ALIGNMENT
13981
]);
14082

14183
/// Check a variant of a non-exhaustive enum for improper ctypes
@@ -174,75 +116,6 @@ fn variant_has_complex_ctor(variant: &ty::VariantDef) -> bool {
174116
!matches!(variant.ctor_kind(), Some(CtorKind::Const))
175117
}
176118

177-
/// Per-struct-field function that checks if a struct definition follows
178-
/// the Power alignment Rule (see the `check_struct_for_power_alignment` function).
179-
fn check_arg_for_power_alignment<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
180-
let tcx = cx.tcx;
181-
assert!(tcx.sess.target.os == Os::Aix);
182-
// Structs (under repr(C)) follow the power alignment rule if:
183-
// - the first field of the struct is a floating-point type that
184-
// is greater than 4-bytes, or
185-
// - the first field of the struct is an aggregate whose
186-
// recursively first field is a floating-point type greater than
187-
// 4 bytes.
188-
if ty.is_floating_point() && ty.primitive_size(tcx).bytes() > 4 {
189-
return true;
190-
} else if let Adt(adt_def, _) = ty.kind()
191-
&& adt_def.is_struct()
192-
&& adt_def.repr().c()
193-
&& !adt_def.repr().packed()
194-
&& adt_def.repr().align.is_none()
195-
{
196-
let struct_variant = adt_def.variant(VariantIdx::ZERO);
197-
// Within a nested struct, all fields are examined to correctly
198-
// report if any fields after the nested struct within the
199-
// original struct are misaligned.
200-
for struct_field in &struct_variant.fields {
201-
let field_ty = tcx.type_of(struct_field.did).instantiate_identity();
202-
if check_arg_for_power_alignment(cx, field_ty) {
203-
return true;
204-
}
205-
}
206-
}
207-
return false;
208-
}
209-
210-
/// Check a struct definition for respect of the Power alignment Rule (as in PowerPC),
211-
/// which should be respected in the "aix" target OS.
212-
/// To do so, we must follow one of the two following conditions:
213-
/// - The first field of the struct must be floating-point type that
214-
/// is greater than 4-bytes.
215-
/// - The first field of the struct must be an aggregate whose
216-
/// recursively first field is a floating-point type greater than
217-
/// 4 bytes.
218-
fn check_struct_for_power_alignment<'tcx>(
219-
cx: &LateContext<'tcx>,
220-
item: &'tcx hir::Item<'tcx>,
221-
adt_def: AdtDef<'tcx>,
222-
) {
223-
let tcx = cx.tcx;
224-
225-
// Only consider structs (not enums or unions) on AIX.
226-
if tcx.sess.target.os != Os::Aix || !adt_def.is_struct() {
227-
return;
228-
}
229-
230-
// The struct must be repr(C), but ignore it if it explicitly specifies its alignment with
231-
// either `align(N)` or `packed(N)`.
232-
if adt_def.repr().c() && !adt_def.repr().packed() && adt_def.repr().align.is_none() {
233-
let struct_variant_data = item.expect_struct().2;
234-
for field_def in struct_variant_data.fields().iter().skip(1) {
235-
// Struct fields (after the first field) are checked for the
236-
// power alignment rule, as fields after the first are likely
237-
// to be the fields that are misaligned.
238-
let ty = tcx.type_of(field_def.def_id).instantiate_identity();
239-
if check_arg_for_power_alignment(cx, ty) {
240-
cx.emit_span_lint(USES_POWER_ALIGNMENT, field_def.span, UsesPowerAlignment);
241-
}
242-
}
243-
}
244-
}
245-
246119
#[derive(Clone, Copy)]
247120
enum CItemKind {
248121
Declaration,
@@ -845,20 +718,10 @@ impl<'tcx> ImproperCTypesLint {
845718
}
846719

847720
/// For a local definition of a #[repr(C)] struct/enum/union, check that it is indeed FFI-safe.
848-
fn check_reprc_adt(
849-
&mut self,
850-
cx: &LateContext<'tcx>,
851-
item: &'tcx hir::Item<'tcx>,
852-
adt_def: AdtDef<'tcx>,
853-
) {
721+
fn check_reprc_adt(&mut self, adt_def: AdtDef<'tcx>) {
854722
debug_assert!(
855723
adt_def.repr().c() && !adt_def.repr().packed() && adt_def.repr().align.is_none()
856724
);
857-
858-
// FIXME(ctypes): this following call is awkward.
859-
// is there a way to perform its logic in MIR space rather than HIR space?
860-
// (so that its logic can be absorbed into visitor.visit_struct_or_union)
861-
check_struct_for_power_alignment(cx, item, adt_def);
862725
}
863726

864727
fn check_foreign_static(&mut self, cx: &LateContext<'tcx>, id: hir::OwnerId, span: Span) {
@@ -1002,7 +865,7 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesLint {
1002865

1003866
if adt_def.repr().c() && !adt_def.repr().packed() && adt_def.repr().align.is_none()
1004867
{
1005-
self.check_reprc_adt(cx, item, adt_def);
868+
self.check_reprc_adt(adt_def);
1006869
}
1007870
}
1008871

0 commit comments

Comments
 (0)