Skip to content

Commit

Permalink
Fix missed same-sized member clash in ClashingExternDeclarations.
Browse files Browse the repository at this point in the history
  • Loading branch information
jumbatm committed Jul 14, 2020
1 parent 9182910 commit 470d759
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 12 deletions.
37 changes: 32 additions & 5 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ use rustc_hir::def_id::DefId;
use rustc_hir::{ForeignItemKind, GenericParamKind, PatKind};
use rustc_hir::{HirId, HirIdSet, Node};
use rustc_middle::lint::LintDiagnosticBuilder;
use rustc_middle::ty::subst::GenericArgKind;
use rustc_middle::ty::subst::{GenericArgKind, Subst};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_session::lint::FutureIncompatibleInfo;
use rustc_span::edition::Edition;
use rustc_span::source_map::Spanned;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{BytePos, Span};
use rustc_target::abi::VariantIdx;
use rustc_target::abi::{LayoutOf, VariantIdx};
use rustc_trait_selection::traits::misc::can_type_implement_copy;

use crate::nonstandard_style::{method_context, MethodLateContext};
Expand Down Expand Up @@ -2131,6 +2131,7 @@ impl ClashingExternDeclarations {
/// clash. We need this so we don't emit a lint when two modules both declare an extern struct,
/// with the same members (as the declarations shouldn't clash).
fn structurally_same_type<'tcx>(cx: &LateContext<'tcx>, a: Ty<'tcx>, b: Ty<'tcx>) -> bool {
debug!("structurally_same_type(cx, a = {:?}, b = {:?})", a, b);
let tcx = cx.tcx;
if a == b || rustc_middle::ty::TyS::same_type(a, b) {
// All nominally-same types are structurally same, too.
Expand All @@ -2141,17 +2142,43 @@ impl ClashingExternDeclarations {
let a_kind = &a.kind;
let b_kind = &b.kind;

use rustc_target::abi::LayoutOf;
let compare_layouts = |a, b| -> bool {
&cx.layout_of(a).unwrap().layout.abi == &cx.layout_of(b).unwrap().layout.abi
let a_layout = &cx.layout_of(a).unwrap().layout.abi;
let b_layout = &cx.layout_of(b).unwrap().layout.abi;
debug!("{:?} == {:?} = {}", a_layout, b_layout, a_layout == b_layout);
a_layout == b_layout
};

#[allow(rustc::usage_of_ty_tykind)]
let is_primitive_or_pointer =
|kind: &ty::TyKind<'_>| kind.is_primitive() || matches!(kind, RawPtr(..));

match (a_kind, b_kind) {
(Adt(..), Adt(..)) => compare_layouts(a, b),
(Adt(_, a_substs), Adt(_, b_substs)) => {
let a = a.subst(cx.tcx, a_substs);
let b = b.subst(cx.tcx, b_substs);
debug!("Comparing {:?} and {:?}", a, b);

if let (Adt(a_def, ..), Adt(b_def, ..)) = (&a.kind, &b.kind) {
// Grab a flattened representation of all fields.
let a_fields = a_def.variants.iter().flat_map(|v| v.fields.iter());
let b_fields = b_def.variants.iter().flat_map(|v| v.fields.iter());
compare_layouts(a, b)
&& a_fields.eq_by(
b_fields,
|&ty::FieldDef { did: a_did, .. },
&ty::FieldDef { did: b_did, .. }| {
Self::structurally_same_type(
cx,
tcx.type_of(a_did),
tcx.type_of(b_did),
)
},
)
} else {
unreachable!()
}
}
(Array(a_ty, a_const), Array(b_ty, b_const)) => {
// For arrays, we also check the constness of the type.
a_const.val == b_const.val
Expand Down
22 changes: 22 additions & 0 deletions src/test/ui/lint/clashing-extern-fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,28 @@ mod sameish_members {
}
}

mod same_sized_members_clash {
mod a {
#[repr(C)]
struct Point3 {
x: f32,
y: f32,
z: f32,
}
extern "C" { fn origin() -> Point3; }
}
mod b {
#[repr(C)]
struct Point3 {
x: i32,
y: i32,
z: i32, // NOTE: Incorrectly redeclared as i32
}
extern "C" { fn origin() -> Point3; }
//~^ WARN `origin` redeclared with a different signature
}
}

mod transparent {
#[repr(transparent)]
struct T(usize);
Expand Down
26 changes: 19 additions & 7 deletions src/test/ui/lint/clashing-extern-fn.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,20 @@ LL | fn draw_point(p: Point);
= note: expected `unsafe extern "C" fn(sameish_members::a::Point)`
found `unsafe extern "C" fn(sameish_members::b::Point)`

warning: `origin` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:194:22
|
LL | extern "C" { fn origin() -> Point3; }
| ---------------------- `origin` previously declared here
...
LL | extern "C" { fn origin() -> Point3; }
| ^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
|
= note: expected `unsafe extern "C" fn() -> same_sized_members_clash::a::Point3`
found `unsafe extern "C" fn() -> same_sized_members_clash::b::Point3`

warning: `transparent_incorrect` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:195:13
--> $DIR/clashing-extern-fn.rs:217:13
|
LL | fn transparent_incorrect() -> T;
| -------------------------------- `transparent_incorrect` previously declared here
Expand All @@ -118,7 +130,7 @@ LL | fn transparent_incorrect() -> isize;
found `unsafe extern "C" fn() -> isize`

warning: `missing_return_type` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:213:13
--> $DIR/clashing-extern-fn.rs:235:13
|
LL | fn missing_return_type() -> usize;
| ---------------------------------- `missing_return_type` previously declared here
Expand All @@ -130,7 +142,7 @@ LL | fn missing_return_type();
found `unsafe extern "C" fn()`

warning: `non_zero_usize` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:231:13
--> $DIR/clashing-extern-fn.rs:253:13
|
LL | fn non_zero_usize() -> core::num::NonZeroUsize;
| ----------------------------------------------- `non_zero_usize` previously declared here
Expand All @@ -142,7 +154,7 @@ LL | fn non_zero_usize() -> usize;
found `unsafe extern "C" fn() -> usize`

warning: `non_null_ptr` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:233:13
--> $DIR/clashing-extern-fn.rs:255:13
|
LL | fn non_null_ptr() -> core::ptr::NonNull<usize>;
| ----------------------------------------------- `non_null_ptr` previously declared here
Expand All @@ -154,7 +166,7 @@ LL | fn non_null_ptr() -> *const usize;
found `unsafe extern "C" fn() -> *const usize`

warning: `option_non_zero_usize_incorrect` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:259:13
--> $DIR/clashing-extern-fn.rs:281:13
|
LL | fn option_non_zero_usize_incorrect() -> usize;
| ---------------------------------------------- `option_non_zero_usize_incorrect` previously declared here
Expand All @@ -166,7 +178,7 @@ LL | fn option_non_zero_usize_incorrect() -> isize;
found `unsafe extern "C" fn() -> isize`

warning: `option_non_null_ptr_incorrect` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:261:13
--> $DIR/clashing-extern-fn.rs:283:13
|
LL | fn option_non_null_ptr_incorrect() -> *const usize;
| --------------------------------------------------- `option_non_null_ptr_incorrect` previously declared here
Expand All @@ -177,5 +189,5 @@ LL | fn option_non_null_ptr_incorrect() -> *const isize;
= note: expected `unsafe extern "C" fn() -> *const usize`
found `unsafe extern "C" fn() -> *const isize`

warning: 14 warnings emitted
warning: 15 warnings emitted

0 comments on commit 470d759

Please sign in to comment.