Skip to content

Commit

Permalink
Rollup merge of rust-lang#75885 - jumbatm:issue75739-clashing-extern-…
Browse files Browse the repository at this point in the history
…declarations-transparent-nonzero, r=lcnr

Fix another clashing_extern_declarations false positive.

Fixes rust-lang#75739.

Fix another clashing_extern_declarations false positive, this time for transparent newtype with a non-zero member.

r? @lcnr
  • Loading branch information
pietroalbini committed Aug 28, 2020
2 parents 7185766 + 352df40 commit 47f296f
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 21 deletions.
35 changes: 35 additions & 0 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::{ForeignItemKind, GenericParamKind, PatKind};
use rustc_hir::{HirId, HirIdSet, Node};
use rustc_index::vec::Idx;
use rustc_middle::lint::LintDiagnosticBuilder;
use rustc_middle::ty::subst::{GenericArgKind, Subst};
use rustc_middle::ty::{self, layout::LayoutError, Ty, TyCtxt};
Expand Down Expand Up @@ -2162,6 +2163,40 @@ impl ClashingExternDeclarations {
ckind: CItemKind,
) -> bool {
debug!("structurally_same_type_impl(cx, a = {:?}, b = {:?})", a, b);
let tcx = cx.tcx;

// Given a transparent newtype, reach through and grab the inner
// type unless the newtype makes the type non-null.
let non_transparent_ty = |ty: Ty<'tcx>| -> Ty<'tcx> {
let mut ty = ty;
loop {
if let ty::Adt(def, substs) = ty.kind {
let is_transparent = def.subst(tcx, substs).repr.transparent();
let is_non_null = crate::types::nonnull_optimization_guaranteed(tcx, &def);
debug!(
"non_transparent_ty({:?}) -- type is transparent? {}, type is non-null? {}",
ty, is_transparent, is_non_null
);
if is_transparent && !is_non_null {
debug_assert!(def.variants.len() == 1);
let v = &def.variants[VariantIdx::new(0)];
ty = v
.transparent_newtype_field(tcx)
.expect(
"single-variant transparent structure with zero-sized field",
)
.ty(tcx, substs);
continue;
}
}
debug!("non_transparent_ty -> {:?}", ty);
return ty;
}
};

let a = non_transparent_ty(a);
let b = non_transparent_ty(b);

if !seen_types.insert((a, b)) {
// We've encountered a cycle. There's no point going any further -- the types are
// structurally the same.
Expand Down
18 changes: 11 additions & 7 deletions src/librustc_lint/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc_index::vec::Idx;
use rustc_middle::mir::interpret::{sign_extend, truncate};
use rustc_middle::ty::layout::{IntegerExt, SizeSkeleton};
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::{self, AdtKind, Ty, TypeFoldable};
use rustc_middle::ty::{self, AdtKind, Ty, TyCtxt, TypeFoldable};
use rustc_span::source_map;
use rustc_span::symbol::sym;
use rustc_span::{Span, DUMMY_SP};
Expand Down Expand Up @@ -527,22 +527,26 @@ enum FfiResult<'tcx> {
FfiUnsafe { ty: Ty<'tcx>, reason: String, help: Option<String> },
}

crate fn nonnull_optimization_guaranteed<'tcx>(tcx: TyCtxt<'tcx>, def: &ty::AdtDef) -> bool {
tcx.get_attrs(def.did)
.iter()
.any(|a| tcx.sess.check_name(a, sym::rustc_nonnull_optimization_guaranteed))
}

/// Is type known to be non-null?
fn ty_is_known_nonnull<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, mode: CItemKind) -> bool {
crate fn ty_is_known_nonnull<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, mode: CItemKind) -> bool {
let tcx = cx.tcx;
match ty.kind {
ty::FnPtr(_) => true,
ty::Ref(..) => true,
ty::Adt(def, _) if def.is_box() && matches!(mode, CItemKind::Definition) => true,
ty::Adt(def, substs) if def.repr.transparent() && !def.is_union() => {
let guaranteed_nonnull_optimization = tcx
.get_attrs(def.did)
.iter()
.any(|a| tcx.sess.check_name(a, sym::rustc_nonnull_optimization_guaranteed));
let marked_non_null = nonnull_optimization_guaranteed(tcx, &def);

if guaranteed_nonnull_optimization {
if marked_non_null {
return true;
}

for variant in &def.variants {
if let Some(field) = variant.transparent_newtype_field(tcx) {
if ty_is_known_nonnull(cx, field.ty(tcx, substs), mode) {
Expand Down
81 changes: 78 additions & 3 deletions src/test/ui/lint/clashing-extern-fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,9 @@ mod same_sized_members_clash {
y: f32,
z: f32,
}
extern "C" { fn origin() -> Point3; }
extern "C" {
fn origin() -> Point3;
}
}
mod b {
#[repr(C)]
Expand All @@ -191,8 +193,9 @@ mod same_sized_members_clash {
y: i32,
z: i32, // NOTE: Incorrectly redeclared as i32
}
extern "C" { fn origin() -> Point3; }
//~^ WARN `origin` redeclared with a different signature
extern "C" {
fn origin() -> Point3; //~ WARN `origin` redeclared with a different signature
}
}
}

Expand Down Expand Up @@ -258,6 +261,78 @@ mod non_zero_and_non_null {
}
}

// See #75739
mod non_zero_transparent {
mod a1 {
use std::num::NonZeroUsize;
extern "C" {
fn f1() -> NonZeroUsize;
}
}

mod b1 {
#[repr(transparent)]
struct X(NonZeroUsize);
use std::num::NonZeroUsize;
extern "C" {
fn f1() -> X;
}
}

mod a2 {
use std::num::NonZeroUsize;
extern "C" {
fn f2() -> NonZeroUsize;
}
}

mod b2 {
#[repr(transparent)]
struct X1(NonZeroUsize);

#[repr(transparent)]
struct X(X1);

use std::num::NonZeroUsize;
extern "C" {
// Same case as above, but with two layers of newtyping.
fn f2() -> X;
}
}

mod a3 {
#[repr(transparent)]
struct X(core::ptr::NonNull<i32>);

use std::num::NonZeroUsize;
extern "C" {
fn f3() -> X;
}
}

mod b3 {
extern "C" {
fn f3() -> core::ptr::NonNull<i32>;
}
}

mod a4 {
#[repr(transparent)]
enum E {
X(std::num::NonZeroUsize),
}
extern "C" {
fn f4() -> E;
}
}

mod b4 {
extern "C" {
fn f4() -> std::num::NonZeroUsize;
}
}
}

mod null_optimised_enums {
mod a {
extern "C" {
Expand Down
22 changes: 11 additions & 11 deletions src/test/ui/lint/clashing-extern-fn.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -106,19 +106,19 @@ LL | fn draw_point(p: Point);
found `unsafe extern "C" fn(sameish_members::b::Point)`

warning: `origin` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:194:22
--> $DIR/clashing-extern-fn.rs:197:13
|
LL | extern "C" { fn origin() -> Point3; }
| ---------------------- `origin` previously declared here
LL | fn origin() -> Point3;
| ---------------------- `origin` previously declared here
...
LL | extern "C" { fn origin() -> Point3; }
| ^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
LL | 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:217:13
--> $DIR/clashing-extern-fn.rs:220:13
|
LL | fn transparent_incorrect() -> T;
| -------------------------------- `transparent_incorrect` previously declared here
Expand All @@ -130,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:235:13
--> $DIR/clashing-extern-fn.rs:238:13
|
LL | fn missing_return_type() -> usize;
| ---------------------------------- `missing_return_type` previously declared here
Expand All @@ -142,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:253:13
--> $DIR/clashing-extern-fn.rs:256:13
|
LL | fn non_zero_usize() -> core::num::NonZeroUsize;
| ----------------------------------------------- `non_zero_usize` previously declared here
Expand All @@ -154,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:255:13
--> $DIR/clashing-extern-fn.rs:258:13
|
LL | fn non_null_ptr() -> core::ptr::NonNull<usize>;
| ----------------------------------------------- `non_null_ptr` previously declared here
Expand All @@ -166,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:281:13
--> $DIR/clashing-extern-fn.rs:356:13
|
LL | fn option_non_zero_usize_incorrect() -> usize;
| ---------------------------------------------- `option_non_zero_usize_incorrect` previously declared here
Expand All @@ -178,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:283:13
--> $DIR/clashing-extern-fn.rs:358:13
|
LL | fn option_non_null_ptr_incorrect() -> *const usize;
| --------------------------------------------------- `option_non_null_ptr_incorrect` previously declared here
Expand Down

0 comments on commit 47f296f

Please sign in to comment.