lint: check for unit ret type after normalization

This commit moves the check that skips unit return types to after
where the return type has been normalized - therefore ensuring that
FFI-safety lints are not emitted for types which normalize to unit.

Signed-off-by: David Wood <david@davidtw.co>

src/librustc_lint/types.rs

@@ -946,7 +946,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
         }
     }
 
-    fn check_type_for_ffi_and_report_errors(&mut self, sp: Span, ty: Ty<'tcx>, is_static: bool) {
+    fn check_type_for_ffi_and_report_errors(
+        &mut self,
+        sp: Span,
+        ty: Ty<'tcx>,
+        is_static: bool,
+        is_return_type: bool,
+    ) {
         // We have to check for opaque types before `normalize_erasing_regions`,
         // which will replace opaque types with their underlying concrete type.
         if self.check_for_opaque_ty(sp, ty) {
@@ -957,14 +963,21 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
         // it is only OK to use this function because extern fns cannot have
         // any generic types right now:
         let ty = self.cx.tcx.normalize_erasing_regions(ParamEnv::reveal_all(), ty);
-        // C doesn't really support passing arrays by value.
-        // The only way to pass an array by value is through a struct.
-        // So we first test that the top level isn't an array,
-        // and then recursively check the types inside.
+
+        // C doesn't really support passing arrays by value - the only way to pass an array by value
+        // is through a struct. So, first test that the top level isn't an array, and then
+        // recursively check the types inside.
         if !is_static && self.check_for_array_ty(sp, ty) {
             return;
         }
 
+        // Don't report FFI errors for unit return types. This check exists here, and not in
+        // `check_foreign_fn` (where it would make more sense) so that normalization has definitely
+        // happened.
+        if is_return_type && ty.is_unit() {
+            return;
+        }
+
         match self.check_type_for_ffi(&mut FxHashSet::default(), ty) {
             FfiResult::FfiSafe => {}
             FfiResult::FfiPhantom(ty) => {
@@ -982,21 +995,19 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
         let sig = self.cx.tcx.erase_late_bound_regions(&sig);
 
         for (input_ty, input_hir) in sig.inputs().iter().zip(decl.inputs) {
-            self.check_type_for_ffi_and_report_errors(input_hir.span, input_ty, false);
+            self.check_type_for_ffi_and_report_errors(input_hir.span, input_ty, false, false);
         }
 
         if let hir::FnRetTy::Return(ref ret_hir) = decl.output {
             let ret_ty = sig.output();
-            if !ret_ty.is_unit() {
-                self.check_type_for_ffi_and_report_errors(ret_hir.span, ret_ty, false);
-            }
+            self.check_type_for_ffi_and_report_errors(ret_hir.span, ret_ty, false, true);
         }
     }
 
     fn check_foreign_static(&mut self, id: hir::HirId, span: Span) {
         let def_id = self.cx.tcx.hir().local_def_id(id);
         let ty = self.cx.tcx.type_of(def_id);
-        self.check_type_for_ffi_and_report_errors(span, ty, true);
+        self.check_type_for_ffi_and_report_errors(span, ty, true, false);
     }
 }
 

src/test/ui/lint/lint-ctypes-66202.rs

@@ -9,7 +9,6 @@ pub struct W<T>(T);
 extern "C" {
     pub fn bare() -> ();
     pub fn normalize() -> <() as ToOwned>::Owned;
-    //~^ ERROR uses type `()`
     pub fn transparent() -> W<()>;
     //~^ ERROR uses type `W<()>`
 }

src/test/ui/lint/lint-ctypes-66202.stderr

@@ -1,23 +1,14 @@
-error: `extern` block uses type `()`, which is not FFI-safe
-  --> $DIR/lint-ctypes-66202.rs:11:27
+error: `extern` block uses type `W<()>`, which is not FFI-safe
+  --> $DIR/lint-ctypes-66202.rs:12:29
    |
-LL |     pub fn normalize() -> <() as ToOwned>::Owned;
-   |                           ^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
+LL |     pub fn transparent() -> W<()>;
+   |                             ^^^^^ not FFI-safe
    |
 note: the lint level is defined here
   --> $DIR/lint-ctypes-66202.rs:1:9
    |
 LL | #![deny(improper_ctypes)]
    |         ^^^^^^^^^^^^^^^
-   = help: consider using a struct instead
-   = note: tuples have unspecified layout
-
-error: `extern` block uses type `W<()>`, which is not FFI-safe
-  --> $DIR/lint-ctypes-66202.rs:13:29
-   |
-LL |     pub fn transparent() -> W<()>;
-   |                             ^^^^^ not FFI-safe
-   |
    = note: composed only of `PhantomData`
 note: the type is defined here
   --> $DIR/lint-ctypes-66202.rs:7:1
@@ -25,5 +16,5 @@ note: the type is defined here
 LL | pub struct W<T>(T);
    | ^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 2 previous errors
+error: aborting due to previous error