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

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
         }
     }
 }

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
+

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;
 }
 

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
+

src/test/ui/lint/auxiliary/external_extern_fn.rs

@@ -0,0 +1,3 @@
+extern {
+    pub fn extern_fn(x: u8);
+}

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
+    }
+}

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
+

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"]

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);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^

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