Auto merge of rust-lang#73206 - Dylan-DPC:rollup-rha9g8q, r=Dylan-DPC

Rollup of 9 pull requests

Successful merges:

 - rust-lang#72706 (Add windows group to triagebot)
 - rust-lang#72789 (resolve: Do not suggest imports from the same module in which we are resolving)
 - rust-lang#72890 (improper ctypes: normalize return types and transparent structs)
 - rust-lang#72897 (normalize adt fields during structural match checking)
 - rust-lang#73005 (Don't create impl candidates when obligation contains errors)
 - rust-lang#73023 (Remove noisy suggestion of hash_map )
 - rust-lang#73070 (Add regression test for const generic ICE in rust-lang#72819)
 - rust-lang#73157 (Don't lose empty `where` clause when pretty-printing)
 - rust-lang#73184 (Reoder order in which MinGW libs are linked to fix recent breakage)

Failed merges:

r? @ghost

Author: bors

src/librustc_ast/ast.rs

@@ -362,7 +362,11 @@ impl Default for Generics {
     fn default() -> Generics {
         Generics {
             params: Vec::new(),
-            where_clause: WhereClause { predicates: Vec::new(), span: DUMMY_SP },
+            where_clause: WhereClause {
+                has_where_token: false,
+                predicates: Vec::new(),
+                span: DUMMY_SP,
+            },
             span: DUMMY_SP,
         }
     }
@@ -371,6 +375,11 @@ impl Default for Generics {
 /// A where-clause in a definition.
 #[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
 pub struct WhereClause {
+    /// `true` if we ate a `where` token: this can happen
+    /// if we parsed no predicates (e.g. `struct Foo where {}
+    /// This allows us to accurately pretty-print
+    /// in `nt_to_tokenstream`
+    pub has_where_token: bool,
     pub predicates: Vec<WherePredicate>,
     pub span: Span,
 }

src/librustc_ast/mut_visit.rs

@@ -786,7 +786,7 @@ pub fn noop_visit_generics<T: MutVisitor>(generics: &mut Generics, vis: &mut T)
 }
 
 pub fn noop_visit_where_clause<T: MutVisitor>(wc: &mut WhereClause, vis: &mut T) {
-    let WhereClause { predicates, span } = wc;
+    let WhereClause { has_where_token: _, predicates, span } = wc;
     visit_vec(predicates, |predicate| vis.visit_where_predicate(predicate));
     vis.visit_span(span);
 }

src/librustc_ast_pretty/pprust.rs

@@ -2593,7 +2593,7 @@ impl<'a> State<'a> {
     }
 
     crate fn print_where_clause(&mut self, where_clause: &ast::WhereClause) {
-        if where_clause.predicates.is_empty() {
+        if where_clause.predicates.is_empty() && !where_clause.has_where_token {
             return;
         }
 
@@ -2739,7 +2739,11 @@ impl<'a> State<'a> {
         }
         let generics = ast::Generics {
             params: Vec::new(),
-            where_clause: ast::WhereClause { predicates: Vec::new(), span: rustc_span::DUMMY_SP },
+            where_clause: ast::WhereClause {
+                has_where_token: false,
+                predicates: Vec::new(),
+                span: rustc_span::DUMMY_SP,
+            },
             span: rustc_span::DUMMY_SP,
         };
         let header = ast::FnHeader { unsafety, ext, ..ast::FnHeader::default() };

src/librustc_builtin_macros/deriving/generic/ty.rs

@@ -216,7 +216,11 @@ fn mk_ty_param(
 }
 
 fn mk_generics(params: Vec<ast::GenericParam>, span: Span) -> Generics {
-    Generics { params, where_clause: ast::WhereClause { predicates: Vec::new(), span }, span }
+    Generics {
+        params,
+        where_clause: ast::WhereClause { has_where_token: false, predicates: Vec::new(), span },
+        span,
+    }
 }
 
 /// Lifetimes and bounds on type parameters

src/librustc_lint/types.rs

@@ -6,7 +6,6 @@ use rustc_attr as attr;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
-use rustc_hir::def_id::DefId;
 use rustc_hir::{is_range_literal, ExprKind, Node};
 use rustc_index::vec::Idx;
 use rustc_middle::mir::interpret::{sign_extend, truncate};
@@ -511,10 +510,6 @@ enum FfiResult<'tcx> {
     FfiUnsafe { ty: Ty<'tcx>, reason: &'static str, help: Option<&'static str> },
 }
 
-fn is_zst<'tcx>(tcx: TyCtxt<'tcx>, did: DefId, ty: Ty<'tcx>) -> bool {
-    tcx.layout_of(tcx.param_env(did).and(ty)).map(|layout| layout.is_zst()).unwrap_or(false)
-}
-
 fn ty_is_known_nonnull<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool {
     match ty.kind {
         ty::FnPtr(_) => true,
@@ -523,7 +518,7 @@ fn ty_is_known_nonnull<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool {
             for field in field_def.all_fields() {
                 let field_ty =
                     tcx.normalize_erasing_regions(ParamEnv::reveal_all(), field.ty(tcx, substs));
-                if is_zst(tcx, field.did, field_ty) {
+                if field_ty.is_zst(tcx, field.did) {
                     continue;
                 }
 
@@ -653,32 +648,43 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                             };
                         }
 
-                        // We can't completely trust repr(C) and repr(transparent) markings;
-                        // make sure the fields are actually safe.
-                        let mut all_phantom = true;
-                        for field in &def.non_enum_variant().fields {
-                            let field_ty = cx.normalize_erasing_regions(
-                                ParamEnv::reveal_all(),
-                                field.ty(cx, substs),
-                            );
-                            // repr(transparent) types are allowed to have arbitrary ZSTs, not just
-                            // PhantomData -- skip checking all ZST fields
-                            if def.repr.transparent() && is_zst(cx, field.did, field_ty) {
-                                continue;
+                        if def.repr.transparent() {
+                            // Can assume that only one field is not a ZST, so only check
+                            // that field's type for FFI-safety.
+                            if let Some(field) =
+                                def.transparent_newtype_field(cx, self.cx.param_env)
+                            {
+                                let field_ty = cx.normalize_erasing_regions(
+                                    self.cx.param_env,
+                                    field.ty(cx, substs),
+                                );
+                                self.check_type_for_ffi(cache, field_ty)
+                            } else {
+                                FfiSafe
                             }
-                            let r = self.check_type_for_ffi(cache, field_ty);
-                            match r {
-                                FfiSafe => {
-                                    all_phantom = false;
-                                }
-                                FfiPhantom(..) => {}
-                                FfiUnsafe { .. } => {
-                                    return r;
+                        } else {
+                            // We can't completely trust repr(C) markings; make sure the fields are
+                            // actually safe.
+                            let mut all_phantom = true;
+                            for field in &def.non_enum_variant().fields {
+                                let field_ty = cx.normalize_erasing_regions(
+                                    self.cx.param_env,
+                                    field.ty(cx, substs),
+                                );
+                                let r = self.check_type_for_ffi(cache, field_ty);
+                                match r {
+                                    FfiSafe => {
+                                        all_phantom = false;
+                                    }
+                                    FfiPhantom(..) => {}
+                                    FfiUnsafe { .. } => {
+                                        return r;
+                                    }
                                 }
                             }
-                        }
 
-                        if all_phantom { FfiPhantom(ty) } else { FfiSafe }
+                            if all_phantom { FfiPhantom(ty) } else { FfiSafe }
+                        }
                     }
                     AdtKind::Union => {
                         if !def.repr.c() && !def.repr.transparent() {
@@ -708,7 +714,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                             );
                             // repr(transparent) types are allowed to have arbitrary ZSTs, not just
                             // PhantomData -- skip checking all ZST fields.
-                            if def.repr.transparent() && is_zst(cx, field.did, field_ty) {
+                            if def.repr.transparent() && field_ty.is_zst(cx, field.did) {
                                 continue;
                             }
                             let r = self.check_type_for_ffi(cache, field_ty);
@@ -774,7 +780,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                                 );
                                 // repr(transparent) types are allowed to have arbitrary ZSTs, not
                                 // just PhantomData -- skip checking all ZST fields.
-                                if def.repr.transparent() && is_zst(cx, field.did, field_ty) {
+                                if def.repr.transparent() && field_ty.is_zst(cx, field.did) {
                                     continue;
                                 }
                                 let r = self.check_type_for_ffi(cache, field_ty);
@@ -946,7 +952,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,19 +969,29 @@ 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) => {
                 self.emit_ffi_unsafe_type_lint(ty, sp, "composed only of `PhantomData`", None);
             }
+            // If `ty` is a `repr(transparent)` newtype, and the non-zero-sized type is a generic
+            // argument, which after substitution, is `()`, then this branch can be hit.
+            FfiResult::FfiUnsafe { ty, .. } if is_return_type && ty.is_unit() => return,
             FfiResult::FfiUnsafe { ty, reason, help } => {
                 self.emit_ffi_unsafe_type_lint(ty, sp, reason, help);
             }
@@ -982,21 +1004,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/librustc_middle/ty/mod.rs

@@ -2390,6 +2390,29 @@ impl<'tcx> AdtDef {
     pub fn sized_constraint(&self, tcx: TyCtxt<'tcx>) -> &'tcx [Ty<'tcx>] {
         tcx.adt_sized_constraint(self.did).0
     }
+
+    /// `repr(transparent)` structs can have a single non-ZST field, this function returns that
+    /// field.
+    pub fn transparent_newtype_field(
+        &self,
+        tcx: TyCtxt<'tcx>,
+        param_env: ParamEnv<'tcx>,
+    ) -> Option<&FieldDef> {
+        assert!(self.is_struct() && self.repr.transparent());
+
+        for field in &self.non_enum_variant().fields {
+            let field_ty = tcx.normalize_erasing_regions(
+                param_env,
+                field.ty(tcx, InternalSubsts::identity_for_item(tcx, self.did)),
+            );
+
+            if !field_ty.is_zst(tcx, self.did) {
+                return Some(field);
+            }
+        }
+
+        None
+    }
 }
 
 impl<'tcx> FieldDef {

src/librustc_middle/ty/sty.rs

@@ -2186,6 +2186,11 @@ impl<'tcx> TyS<'tcx> {
             }
         }
     }
+
+    /// Is this a zero-sized type?
+    pub fn is_zst(&'tcx self, tcx: TyCtxt<'tcx>, did: DefId) -> bool {
+        tcx.layout_of(tcx.param_env(did).and(self)).map(|layout| layout.is_zst()).unwrap_or(false)
+    }
 }
 
 /// Typed constant value.

src/librustc_parse/parser/generics.rs

@@ -157,6 +157,7 @@ impl<'a> Parser<'a> {
         Ok(ast::Generics {
             params,
             where_clause: WhereClause {
+                has_where_token: false,
                 predicates: Vec::new(),
                 span: self.prev_token.span.shrink_to_hi(),
             },
@@ -170,12 +171,16 @@ impl<'a> Parser<'a> {
     /// where T : Trait<U, V> + 'b, 'a : 'b
     /// ```
     pub(super) fn parse_where_clause(&mut self) -> PResult<'a, WhereClause> {
-        let mut where_clause =
-            WhereClause { predicates: Vec::new(), span: self.prev_token.span.shrink_to_hi() };
+        let mut where_clause = WhereClause {
+            has_where_token: false,
+            predicates: Vec::new(),
+            span: self.prev_token.span.shrink_to_hi(),
+        };
 
         if !self.eat_keyword(kw::Where) {
             return Ok(where_clause);
         }
+        where_clause.has_where_token = true;
         let lo = self.prev_token.span;
 
         // We are considering adding generics to the `where` keyword as an alternative higher-rank

src/librustc_resolve/diagnostics.rs

@@ -629,6 +629,7 @@ impl<'a> Resolver<'a> {
         &mut self,
         lookup_ident: Ident,
         namespace: Namespace,
+        parent_scope: &ParentScope<'a>,
         start_module: Module<'a>,
         crate_name: Ident,
         filter_fn: FilterFn,
@@ -655,7 +656,11 @@ impl<'a> Resolver<'a> {
                 }
 
                 // collect results based on the filter function
-                if ident.name == lookup_ident.name && ns == namespace {
+                // avoid suggesting anything from the same module in which we are resolving
+                if ident.name == lookup_ident.name
+                    && ns == namespace
+                    && !ptr::eq(in_module, parent_scope.module)
+                {
                     let res = name_binding.res();
                     if filter_fn(res) {
                         // create the path
@@ -680,7 +685,9 @@ impl<'a> Resolver<'a> {
                                 Res::Def(DefKind::Ctor(..), did) => this.parent(did),
                                 _ => res.opt_def_id(),
                             };
-                            candidates.push(ImportSuggestion { did, descr: res.descr(), path });
+                            if candidates.iter().all(|v: &ImportSuggestion| v.did != did) {
+                                candidates.push(ImportSuggestion { did, descr: res.descr(), path });
+                            }
                         }
                     }
                 }
@@ -722,6 +729,7 @@ impl<'a> Resolver<'a> {
         &mut self,
         lookup_ident: Ident,
         namespace: Namespace,
+        parent_scope: &ParentScope<'a>,
         filter_fn: FilterFn,
     ) -> Vec<ImportSuggestion>
     where
@@ -730,6 +738,7 @@ impl<'a> Resolver<'a> {
         let mut suggestions = self.lookup_import_candidates_from_module(
             lookup_ident,
             namespace,
+            parent_scope,
             self.graph_root,
             Ident::with_dummy_span(kw::Crate),
             &filter_fn,
@@ -754,6 +763,7 @@ impl<'a> Resolver<'a> {
                     suggestions.extend(self.lookup_import_candidates_from_module(
                         lookup_ident,
                         namespace,
+                        parent_scope,
                         crate_root,
                         ident,
                         &filter_fn,

src/librustc_resolve/late/diagnostics.rs

@@ -212,7 +212,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
         let ident = path.last().unwrap().ident;
         let candidates = self
             .r
-            .lookup_import_candidates(ident, ns, is_expected)
+            .lookup_import_candidates(ident, ns, &self.parent_scope, is_expected)
             .drain(..)
             .filter(|ImportSuggestion { did, .. }| {
                 match (did, res.and_then(|res| res.opt_def_id())) {
@@ -223,7 +223,8 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
             .collect::<Vec<_>>();
         let crate_def_id = DefId::local(CRATE_DEF_INDEX);
         if candidates.is_empty() && is_expected(Res::Def(DefKind::Enum, crate_def_id)) {
-            let enum_candidates = self.r.lookup_import_candidates(ident, ns, is_enum_variant);
+            let enum_candidates =
+                self.r.lookup_import_candidates(ident, ns, &self.parent_scope, is_enum_variant);
             let mut enum_candidates = enum_candidates
                 .iter()
                 .map(|suggestion| import_candidate_to_enum_paths(&suggestion))