Auto merge of #70946 - jumbatm:clashing-extern-decl, r=nagisa

Add a lint to catch clashing `extern` fn declarations.

Closes #69390.

Adds lint `clashing_extern_decl` to detect when, within a single crate, an extern function of the same name is declared with different types. Because two symbols of the same name cannot be resolved to two different functions at link time, and one function cannot possibly have two types, a clashing extern declaration is almost certainly a mistake.

This lint does not run between crates because a project may have dependencies which both rely on the same extern function, but declare it in a different (but valid) way. For example, they may both declare an opaque type for one or more of the arguments (which would end up distinct types), or use types that are valid conversions in the language the extern fn is defined in. In these cases, we can't say that the clashing declaration is incorrect.

r? @eddyb

10 files changed, 335 insertions, 6 deletions

diff --git a/src/test/ui/issues/issue-1866.rs b/src/test/ui/issues/issue-1866.rs
index e4fe26800ef..668baefa5e4 100644
--- a/src/test/ui/issues/issue-1866.rs
+++ b/src/test/ui/issues/issue-1866.rs
@@ -1,6 +1,7 @@
 // build-pass
 #![allow(dead_code)]
 #![allow(non_camel_case_types)]
+#![warn(clashing_extern_decl)]
 
 // pretty-expanded FIXME #23616
 
@@ -20,6 +21,7 @@ mod b {
         use super::rust_task;
         extern {
             pub fn rust_task_is_unwinding(rt: *const rust_task) -> bool;
+            //~^ WARN `rust_task_is_unwinding` redeclared with a different signature
         }
     }
 }
diff --git a/src/test/ui/issues/issue-1866.stderr b/src/test/ui/issues/issue-1866.stderr
new file mode 100644
index 00000000000..13c08ebd373
--- /dev/null
+++ b/src/test/ui/issues/issue-1866.stderr
@@ -0,0 +1,19 @@
+warning: `rust_task_is_unwinding` redeclared with a different signature
+  --> $DIR/issue-1866.rs:23:13
+   |
+LL |             pub fn rust_task_is_unwinding(rt: *const rust_task) -> bool;
+   |             ------------------------------------------------------------ `rust_task_is_unwinding` previously declared here
+...
+LL |             pub fn rust_task_is_unwinding(rt: *const rust_task) -> bool;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
+   |
+note: the lint level is defined here
+  --> $DIR/issue-1866.rs:4:9
+   |
+LL | #![warn(clashing_extern_decl)]
+   |         ^^^^^^^^^^^^^^^^^^^^
+   = note: expected `unsafe extern "C" fn(*const usize) -> bool`
+              found `unsafe extern "C" fn(*const bool) -> bool`
+
+warning: 1 warning emitted
+
diff --git a/src/test/ui/issues/issue-5791.rs b/src/test/ui/issues/issue-5791.rs
index 2f8bf1e9369..fda72a1b20e 100644
--- a/src/test/ui/issues/issue-5791.rs
+++ b/src/test/ui/issues/issue-5791.rs
@@ -1,11 +1,13 @@
 // run-pass
 #![allow(dead_code)]
+#![warn(clashing_extern_decl)]
 // pretty-expanded FIXME #23616
 
 extern {
     #[link_name = "malloc"]
     fn malloc1(len: i32) -> *const u8;
     #[link_name = "malloc"]
+    //~^ WARN `malloc2` redeclares `malloc` with a different signature
     fn malloc2(len: i32, foo: i32) -> *const u8;
 }
 
diff --git a/src/test/ui/issues/issue-5791.stderr b/src/test/ui/issues/issue-5791.stderr
new file mode 100644
index 00000000000..7ae83c43f13
--- /dev/null
+++ b/src/test/ui/issues/issue-5791.stderr
@@ -0,0 +1,21 @@
+warning: `malloc2` redeclares `malloc` with a different signature
+  --> $DIR/issue-5791.rs:9:5
+   |
+LL | /     #[link_name = "malloc"]
+LL | |     fn malloc1(len: i32) -> *const u8;
+   | |______________________________________- `malloc` previously declared here
+LL | /     #[link_name = "malloc"]
+LL | |
+LL | |     fn malloc2(len: i32, foo: i32) -> *const u8;
+   | |________________________________________________^ this signature doesn't match the previous declaration
+   |
+note: the lint level is defined here
+  --> $DIR/issue-5791.rs:3:9
+   |
+LL | #![warn(clashing_extern_decl)]
+   |         ^^^^^^^^^^^^^^^^^^^^
+   = note: expected `unsafe extern "C" fn(i32) -> *const u8`
+              found `unsafe extern "C" fn(i32, i32) -> *const u8`
+
+warning: 1 warning emitted
+
diff --git a/src/test/ui/lint/auxiliary/external_extern_fn.rs b/src/test/ui/lint/auxiliary/external_extern_fn.rs
new file mode 100644
index 00000000000..b2caebc6fee
--- /dev/null
+++ b/src/test/ui/lint/auxiliary/external_extern_fn.rs
@@ -0,0 +1,3 @@
+extern {
+    pub fn extern_fn(x: u8);
+}
diff --git a/src/test/ui/lint/clashing-extern-fn.rs b/src/test/ui/lint/clashing-extern-fn.rs
new file mode 100644
index 00000000000..32f3a78f4e9
--- /dev/null
+++ b/src/test/ui/lint/clashing-extern-fn.rs
@@ -0,0 +1,159 @@
+// check-pass
+// aux-build:external_extern_fn.rs
+#![crate_type = "lib"]
+#![warn(clashing_extern_decl)]
+
+extern crate external_extern_fn;
+
+extern {
+    fn clash(x: u8);
+    fn no_clash(x: u8);
+}
+
+fn redeclared_different_signature() {
+    extern {
+        fn clash(x: u64); //~ WARN `clash` redeclared with a different signature
+    }
+
+    unsafe {
+        clash(123);
+        no_clash(123);
+    }
+}
+
+fn redeclared_same_signature() {
+    extern {
+        fn no_clash(x: u8);
+    }
+    unsafe {
+        no_clash(123);
+    }
+}
+
+extern {
+    fn extern_fn(x: u64);
+}
+
+fn extern_clash() {
+    extern {
+        fn extern_fn(x: u32); //~ WARN `extern_fn` redeclared with a different signature
+    }
+    unsafe {
+        extern_fn(123);
+    }
+}
+
+fn extern_no_clash() {
+    unsafe {
+        external_extern_fn::extern_fn(123);
+        crate::extern_fn(123);
+    }
+}
+extern {
+    fn some_other_new_name(x: i16);
+
+    #[link_name = "extern_link_name"]
+    fn some_new_name(x: i16);
+
+    #[link_name = "link_name_same"]
+    fn both_names_different(x: i16);
+}
+
+fn link_name_clash() {
+    extern {
+        fn extern_link_name(x: u32);
+        //~^ WARN `extern_link_name` redeclared with a different signature
+
+        #[link_name = "some_other_new_name"]
+        //~^ WARN `some_other_extern_link_name` redeclares `some_other_new_name` with a different
+        fn some_other_extern_link_name(x: u32);
+
+        #[link_name = "link_name_same"]
+        //~^ WARN `other_both_names_different` redeclares `link_name_same` with a different
+        fn other_both_names_different(x: u32);
+    }
+}
+
+mod a {
+    extern {
+        fn different_mod(x: u8);
+    }
+}
+mod b {
+    extern {
+        fn different_mod(x: u64); //~ WARN `different_mod` redeclared with a different signature
+    }
+}
+
+extern {
+    fn variadic_decl(x: u8, ...);
+}
+
+fn variadic_clash() {
+    extern {
+        fn variadic_decl(x: u8); //~ WARN `variadic_decl` redeclared with a different signature
+    }
+}
+
+#[no_mangle]
+fn no_mangle_name(x: u8) { }
+
+extern {
+    #[link_name = "unique_link_name"]
+    fn link_name_specified(x: u8);
+}
+
+fn tricky_no_clash() {
+    extern {
+        // Shouldn't warn, because the declaration above actually declares a different symbol (and
+        // Rust's name resolution rules around shadowing will handle this gracefully).
+        fn link_name_specified() -> u32;
+
+        // The case of a no_mangle name colliding with an extern decl (see #28179) is related but
+        // shouldn't be reported by ClashingExternDecl, because this is an example of unmangled
+        // name clash causing bad behaviour in functions with a defined body.
+        fn no_mangle_name() -> u32;
+    }
+}
+
+mod banana {
+    mod one {
+        #[repr(C)] struct Banana { weight: u32, length: u16 }
+        extern "C" { fn weigh_banana(count: *const Banana) -> u64; }
+    }
+
+    mod two {
+        #[repr(C)] struct Banana { weight: u32, length: u16 } // note: distinct type
+        // This should not trigger the lint because two::Banana is structurally equivalent to
+        // one::Banana.
+        extern "C" { fn weigh_banana(count: *const Banana) -> u64; }
+    }
+
+    mod three {
+        // This _should_ trigger the lint, because repr(packed) should generate a struct that has a
+        // different layout.
+        #[repr(packed)] struct Banana { weight: u32, length: u16 }
+        #[allow(improper_ctypes)]
+        extern "C" { fn weigh_banana(count: *const Banana) -> u64; }
+        //~^ WARN `weigh_banana` redeclared with a different signature
+    }
+}
+
+mod sameish_members {
+    mod a {
+        #[repr(C)]
+        struct Point { x: i16, y: i16 }
+
+        extern "C" { fn draw_point(p: Point); }
+    }
+    mod b {
+        #[repr(C)]
+        struct Point { coordinates: [i16; 2] }
+
+        // It's possible we are overconservative for this case, as accessing the elements of the
+        // coordinates array might end up correctly accessing `.x` and `.y`. However, this may not
+        // always be the case, for every architecture and situation. This is also a really odd
+        // thing to do anyway.
+        extern "C" { fn draw_point(p: Point); } //~ WARN `draw_point` redeclared with a different
+    }
+}
diff --git a/src/test/ui/lint/clashing-extern-fn.stderr b/src/test/ui/lint/clashing-extern-fn.stderr
new file mode 100644
index 00000000000..fb7bf135f53
--- /dev/null
+++ b/src/test/ui/lint/clashing-extern-fn.stderr
@@ -0,0 +1,121 @@
+warning: `clash` redeclared with a different signature
+  --> $DIR/clashing-extern-fn.rs:15:9
+   |
+LL |     fn clash(x: u8);
+   |     ---------------- `clash` previously declared here
+...
+LL |         fn clash(x: u64);
+   |         ^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
+   |
+note: the lint level is defined here
+  --> $DIR/clashing-extern-fn.rs:4:9
+   |
+LL | #![warn(clashing_extern_decl)]
+   |         ^^^^^^^^^^^^^^^^^^^^
+   = note: expected `unsafe extern "C" fn(u8)`
+              found `unsafe extern "C" fn(u64)`
+
+warning: `extern_fn` redeclared with a different signature
+  --> $DIR/clashing-extern-fn.rs:39:9
+   |
+LL |     fn extern_fn(x: u64);
+   |     --------------------- `extern_fn` previously declared here
+...
+LL |         fn extern_fn(x: u32);
+   |         ^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
+   |
+   = note: expected `unsafe extern "C" fn(u64)`
+              found `unsafe extern "C" fn(u32)`
+
+warning: `extern_link_name` redeclared with a different signature
+  --> $DIR/clashing-extern-fn.rs:64:9
+   |
+LL | /     #[link_name = "extern_link_name"]
+LL | |     fn some_new_name(x: i16);
+   | |_____________________________- `extern_link_name` previously declared here
+...
+LL |           fn extern_link_name(x: u32);
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
+   |
+   = note: expected `unsafe extern "C" fn(i16)`
+              found `unsafe extern "C" fn(u32)`
+
+warning: `some_other_extern_link_name` redeclares `some_other_new_name` with a different signature
+  --> $DIR/clashing-extern-fn.rs:67:9
+   |
+LL |       fn some_other_new_name(x: i16);
+   |       ------------------------------- `some_other_new_name` previously declared here
+...
+LL | /         #[link_name = "some_other_new_name"]
+LL | |
+LL | |         fn some_other_extern_link_name(x: u32);
+   | |_______________________________________________^ this signature doesn't match the previous declaration
+   |
+   = note: expected `unsafe extern "C" fn(i16)`
+              found `unsafe extern "C" fn(u32)`
+
+warning: `other_both_names_different` redeclares `link_name_same` with a different signature
+  --> $DIR/clashing-extern-fn.rs:71:9
+   |
+LL | /     #[link_name = "link_name_same"]
+LL | |     fn both_names_different(x: i16);
+   | |____________________________________- `link_name_same` previously declared here
+...
+LL | /         #[link_name = "link_name_same"]
+LL | |
+LL | |         fn other_both_names_different(x: u32);
+   | |______________________________________________^ this signature doesn't match the previous declaration
+   |
+   = note: expected `unsafe extern "C" fn(i16)`
+              found `unsafe extern "C" fn(u32)`
+
+warning: `different_mod` redeclared with a different signature
+  --> $DIR/clashing-extern-fn.rs:84:9
+   |
+LL |         fn different_mod(x: u8);
+   |         ------------------------ `different_mod` previously declared here
+...
+LL |         fn different_mod(x: u64);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
+   |
+   = note: expected `unsafe extern "C" fn(u8)`
+              found `unsafe extern "C" fn(u64)`
+
+warning: `variadic_decl` redeclared with a different signature
+  --> $DIR/clashing-extern-fn.rs:94:9
+   |
+LL |     fn variadic_decl(x: u8, ...);
+   |     ----------------------------- `variadic_decl` previously declared here
+...
+LL |         fn variadic_decl(x: u8);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
+   |
+   = note: expected `unsafe extern "C" fn(u8, ...)`
+              found `unsafe extern "C" fn(u8)`
+
+warning: `weigh_banana` redeclared with a different signature
+  --> $DIR/clashing-extern-fn.rs:137:22
+   |
+LL |         extern "C" { fn weigh_banana(count: *const Banana) -> u64; }
+   |                      --------------------------------------------- `weigh_banana` previously declared here
+...
+LL |         extern "C" { fn weigh_banana(count: *const Banana) -> u64; }
+   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
+   |
+   = note: expected `unsafe extern "C" fn(*const banana::one::Banana) -> u64`
+              found `unsafe extern "C" fn(*const banana::three::Banana) -> u64`
+
+warning: `draw_point` redeclared with a different signature
+  --> $DIR/clashing-extern-fn.rs:157:22
+   |
+LL |         extern "C" { fn draw_point(p: Point); }
+   |                      ------------------------ `draw_point` previously declared here
+...
+LL |         extern "C" { fn draw_point(p: Point); }
+   |                      ^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
+   |
+   = note: expected `unsafe extern "C" fn(sameish_members::a::Point)`
+              found `unsafe extern "C" fn(sameish_members::b::Point)`
+
+warning: 9 warnings emitted
+
diff --git a/src/test/ui/lint/dead-code/lint-dead-code-3.rs b/src/test/ui/lint/dead-code/lint-dead-code-3.rs
index 6826d2cd67e..ff33abfa645 100644
--- a/src/test/ui/lint/dead-code/lint-dead-code-3.rs
+++ b/src/test/ui/lint/dead-code/lint-dead-code-3.rs
@@ -1,5 +1,6 @@
 #![allow(unused_variables)]
 #![allow(non_camel_case_types)]
+#![allow(clashing_extern_decl)]
 #![deny(dead_code)]
 
 #![crate_type="lib"]
diff --git a/src/test/ui/lint/dead-code/lint-dead-code-3.stderr b/src/test/ui/lint/dead-code/lint-dead-code-3.stderr
index 6d174e8d9bc..cf8f01ea19f 100644
--- a/src/test/ui/lint/dead-code/lint-dead-code-3.stderr
+++ b/src/test/ui/lint/dead-code/lint-dead-code-3.stderr
@@ -1,35 +1,35 @@
 error: struct is never constructed: `Foo`
-  --> $DIR/lint-dead-code-3.rs:13:8
+  --> $DIR/lint-dead-code-3.rs:14:8
    |
 LL | struct Foo;
    |        ^^^
    |
 note: the lint level is defined here
-  --> $DIR/lint-dead-code-3.rs:3:9
+  --> $DIR/lint-dead-code-3.rs:4:9
    |
 LL | #![deny(dead_code)]
    |         ^^^^^^^^^
 
 error: associated function is never used: `foo`
-  --> $DIR/lint-dead-code-3.rs:15:8
+  --> $DIR/lint-dead-code-3.rs:16:8
    |
 LL |     fn foo(&self) {
    |        ^^^
 
 error: function is never used: `bar`
-  --> $DIR/lint-dead-code-3.rs:20:4
+  --> $DIR/lint-dead-code-3.rs:21:4
    |
 LL | fn bar() {
    |    ^^^
 
 error: enum is never used: `c_void`
-  --> $DIR/lint-dead-code-3.rs:59:6
+  --> $DIR/lint-dead-code-3.rs:60:6
    |
 LL | enum c_void {}
    |      ^^^^^^
 
 error: function is never used: `free`
-  --> $DIR/lint-dead-code-3.rs:61:5
+  --> $DIR/lint-dead-code-3.rs:62:5
    |
 LL |     fn free(p: *const c_void);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/src/test/ui/parser/extern-abi-from-mac-literal-frag.rs b/src/test/ui/parser/extern-abi-from-mac-literal-frag.rs
index 4ecb21d26ab..a516aa44c65 100644
--- a/src/test/ui/parser/extern-abi-from-mac-literal-frag.rs
+++ b/src/test/ui/parser/extern-abi-from-mac-literal-frag.rs
@@ -1,3 +1,4 @@
+#![allow(clashing_extern_decl)]
 // check-pass
 
 // In this test we check that the parser accepts an ABI string when it