Auto merge of #34365 - petrochenkov:deferr, r=eddyb

Some more pattern cleanup and bugfixing

The next part of #34095

The most significant fixed mistake is definitions for partially resolved associated types not being updated after full resolution.
```
fn f<T: Fn()>(arg: T::Output) { .... } // <- the definition of T::Output was not updated in def_map
```
For this reason unstable associated types of stable traits, like `FnOnce::Output`, could be used in stable code when written in unqualified form. Now they are properly checked, this is a **[breaking-change]** (pretty minor one, but a crater run would be nice). The fix is not to use unstable library features in stable code, alternatively `FnOnce::Output` can be stabilized.

Besides that, paths in struct patterns and expressions `S::A { .. }` are now fully resolved as associated types. Such types cannot be identified as structs at the moment, i.e. the change doesn't make previously invalid code valid, but it improves error diagnostics.

Other changes: `Def::Err` is supported better (less chances for ICEs for erroneous code), some incorrect error messages are corrected, some duplicated error messages are not reported, ADT definitions are now available through constructor IDs, everything else is cleanup and code audit.

Fixes #34209
Closes #22933 (adds tests)

r? @eddyb

src/libcore/ops.rs

@@ -1929,7 +1929,7 @@ pub trait FnMut<Args> : FnOnce<Args> {
 #[fundamental] // so that regex can rely that `&str: !FnMut`
 pub trait FnOnce<Args> {
     /// The returned type after the call operator is used.
-    #[unstable(feature = "fn_traits", issue = "29625")]
+    #[stable(feature = "fn_once_output", since = "1.12.0")]
     type Output;
 
     /// This is called when the call operator is used.

src/librustc/cfg/construct.rs

@@ -101,7 +101,6 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
         match pat.node {
             PatKind::Binding(_, _, None) |
             PatKind::Path(..) |
-            PatKind::QPath(..) |
             PatKind::Lit(..) |
             PatKind::Range(..) |
             PatKind::Wild => {

src/librustc/hir/def.rs

@@ -137,15 +137,6 @@ impl Def {
         }
     }
 
-    pub fn variant_def_ids(&self) -> Option<(DefId, DefId)> {
-        match *self {
-            Def::Variant(enum_id, var_id) => {
-                Some((enum_id, var_id))
-            }
-            _ => None
-        }
-    }
-
     pub fn kind_name(&self) -> &'static str {
         match *self {
             Def::Fn(..) => "function",

src/librustc/hir/fold.rs

@@ -930,12 +930,11 @@ pub fn noop_fold_pat<T: Folder>(p: P<Pat>, folder: &mut T) -> P<Pat> {
                     PatKind::TupleStruct(folder.fold_path(pth),
                             pats.move_map(|x| folder.fold_pat(x)), ddpos)
                 }
-                PatKind::Path(pth) => {
-                    PatKind::Path(folder.fold_path(pth))
-                }
-                PatKind::QPath(qself, pth) => {
-                    let qself = QSelf { ty: folder.fold_ty(qself.ty), ..qself };
-                    PatKind::QPath(qself, folder.fold_path(pth))
+                PatKind::Path(opt_qself, pth) => {
+                    let opt_qself = opt_qself.map(|qself| {
+                        QSelf { ty: folder.fold_ty(qself.ty), position: qself.position }
+                    });
+                    PatKind::Path(opt_qself, folder.fold_path(pth))
                 }
                 PatKind::Struct(pth, fields, etc) => {
                     let pth = folder.fold_path(pth);

src/librustc/hir/intravisit.rs

@@ -460,11 +460,10 @@ pub fn walk_pat<'v, V: Visitor<'v>>(visitor: &mut V, pattern: &'v Pat) {
             visitor.visit_path(path, pattern.id);
             walk_list!(visitor, visit_pat, children);
         }
-        PatKind::Path(ref path) => {
-            visitor.visit_path(path, pattern.id);
-        }
-        PatKind::QPath(ref qself, ref path) => {
-            visitor.visit_ty(&qself.ty);
+        PatKind::Path(ref opt_qself, ref path) => {
+            if let Some(ref qself) = *opt_qself {
+                visitor.visit_ty(&qself.ty);
+            }
             visitor.visit_path(path, pattern.id)
         }
         PatKind::Struct(ref path, ref fields, _) => {

src/librustc/hir/lowering.rs

@@ -862,7 +862,8 @@ impl<'a> LoweringContext<'a> {
                                                       respan(pth1.span, pth1.node.name),
                                                       sub.as_ref().map(|x| this.lower_pat(x)))
                             }
-                            _ => hir::PatKind::Path(hir::Path::from_name(pth1.span, pth1.node.name))
+                            _ => hir::PatKind::Path(None, hir::Path::from_name(pth1.span,
+                                                                               pth1.node.name))
                         }
                     })
                 }
@@ -872,15 +873,11 @@ impl<'a> LoweringContext<'a> {
                                               pats.iter().map(|x| self.lower_pat(x)).collect(),
                                               ddpos)
                 }
-                PatKind::Path(None, ref pth) => {
-                    hir::PatKind::Path(self.lower_path(pth))
-                }
-                PatKind::Path(Some(ref qself), ref pth) => {
-                    let qself = hir::QSelf {
-                        ty: self.lower_ty(&qself.ty),
-                        position: qself.position,
-                    };
-                    hir::PatKind::QPath(qself, self.lower_path(pth))
+                PatKind::Path(ref opt_qself, ref path) => {
+                    let opt_qself = opt_qself.as_ref().map(|qself| {
+                        hir::QSelf { ty: self.lower_ty(&qself.ty), position: qself.position }
+                    });
+                    hir::PatKind::Path(opt_qself, self.lower_path(path))
                 }
                 PatKind::Struct(ref pth, ref fields, etc) => {
                     let pth = self.lower_path(pth);
@@ -1831,7 +1828,7 @@ impl<'a> LoweringContext<'a> {
                 -> P<hir::Pat> {
         let def = self.resolver.resolve_generated_global_path(&path, true);
         let pt = if subpats.is_empty() {
-            hir::PatKind::Path(path)
+            hir::PatKind::Path(None, path)
         } else {
             hir::PatKind::TupleStruct(path, subpats, None)
         };

src/librustc/hir/mod.rs

@@ -487,8 +487,7 @@ impl Pat {
             PatKind::Lit(_) |
             PatKind::Range(_, _) |
             PatKind::Binding(..) |
-            PatKind::Path(..) |
-            PatKind::QPath(_, _) => {
+            PatKind::Path(..) => {
                 true
             }
         }
@@ -538,15 +537,9 @@ pub enum PatKind {
     /// 0 <= position <= subpats.len()
     TupleStruct(Path, HirVec<P<Pat>>, Option<usize>),
 
-    /// A path pattern.
+    /// A possibly qualified path pattern.
     /// Such pattern can be resolved to a unit struct/variant or a constant.
-    Path(Path),
-
-    /// An associated const named using the qualified path `<T>::CONST` or
-    /// `<T as Trait>::CONST`. Associated consts from inherent impls can be
-    /// referred to as simply `T::CONST`, in which case they will end up as
-    /// PatKind::Path, and the resolver will have to sort that out.
-    QPath(QSelf, Path),
+    Path(Option<QSelf>, Path),
 
     /// A tuple pattern `(a, b)`.
     /// If the `..` pattern fragment is present, then `Option<usize>` denotes its position.

src/librustc/hir/pat_util.rs

@@ -12,15 +12,12 @@ use hir::def::*;
 use hir::def_id::DefId;
 use hir::{self, PatKind};
 use ty::TyCtxt;
-use util::nodemap::FnvHashMap;
 use syntax::ast;
 use syntax::codemap::Spanned;
 use syntax_pos::{Span, DUMMY_SP};
 
 use std::iter::{Enumerate, ExactSizeIterator};
 
-pub type PatIdMap = FnvHashMap<ast::Name, ast::NodeId>;
-
 pub struct EnumerateAndAdjust<I> {
     enumerate: Enumerate<I>,
     gap_pos: usize,
@@ -56,7 +53,7 @@ impl<T: ExactSizeIterator> EnumerateAndAdjustIterator for T {
 
 pub fn pat_is_refutable(dm: &DefMap, pat: &hir::Pat) -> bool {
     match pat.node {
-        PatKind::Lit(_) | PatKind::Range(_, _) | PatKind::QPath(..) => true,
+        PatKind::Lit(_) | PatKind::Range(_, _) | PatKind::Path(Some(..), _) => true,
         PatKind::TupleStruct(..) |
         PatKind::Path(..) |
         PatKind::Struct(..) => {
@@ -70,23 +67,9 @@ pub fn pat_is_refutable(dm: &DefMap, pat: &hir::Pat) -> bool {
     }
 }
 
-pub fn pat_is_variant_or_struct(dm: &DefMap, pat: &hir::Pat) -> bool {
-    match pat.node {
-        PatKind::TupleStruct(..) |
-        PatKind::Path(..) |
-        PatKind::Struct(..) => {
-            match dm.get(&pat.id).map(|d| d.full_def()) {
-                Some(Def::Variant(..)) | Some(Def::Struct(..)) | Some(Def::TyAlias(..)) => true,
-                _ => false
-            }
-        }
-        _ => false
-    }
-}
-
 pub fn pat_is_const(dm: &DefMap, pat: &hir::Pat) -> bool {
     match pat.node {
-        PatKind::Path(..) | PatKind::QPath(..) => {
+        PatKind::Path(..) => {
             match dm.get(&pat.id).map(|d| d.full_def()) {
                 Some(Def::Const(..)) | Some(Def::AssociatedConst(..)) => true,
                 _ => false
@@ -96,22 +79,6 @@ pub fn pat_is_const(dm: &DefMap, pat: &hir::Pat) -> bool {
     }
 }
 
-// Same as above, except that partially-resolved defs cause `false` to be
-// returned instead of a panic.
-pub fn pat_is_resolved_const(dm: &DefMap, pat: &hir::Pat) -> bool {
-    match pat.node {
-        PatKind::Path(..) | PatKind::QPath(..) => {
-            match dm.get(&pat.id)
-                    .and_then(|d| if d.depth == 0 { Some(d.base_def) }
-                                  else { None } ) {
-                Some(Def::Const(..)) | Some(Def::AssociatedConst(..)) => true,
-                _ => false
-            }
-        }
-        _ => false
-    }
-}
-
 /// Call `f` on every "binding" in a pattern, e.g., on `a` in
 /// `match foo() { Some(a) => (), None => () }`
 pub fn pat_bindings<F>(pat: &hir::Pat, mut f: F)

src/librustc/hir/print.rs

@@ -1750,10 +1750,10 @@ impl<'a> State<'a> {
                 }
                 try!(self.pclose());
             }
-            PatKind::Path(ref path) => {
+            PatKind::Path(None, ref path) => {
                 self.print_path(path, true, 0)?;
             }
-            PatKind::QPath(ref qself, ref path) => {
+            PatKind::Path(Some(ref qself), ref path) => {
                 self.print_qpath(path, qself, false)?;
             }
             PatKind::Struct(ref path, ref fields, etc) => {

src/librustc/middle/expr_use_visitor.rs

@@ -945,52 +945,41 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
     /// The core driver for walking a pattern; `match_mode` must be
     /// established up front, e.g. via `determine_pat_move_mode` (see
     /// also `walk_irrefutable_pat` for patterns that stand alone).
-    fn walk_pat(&mut self,
-                cmt_discr: mc::cmt<'tcx>,
-                pat: &hir::Pat,
-                match_mode: MatchMode) {
-        debug!("walk_pat cmt_discr={:?} pat={:?}", cmt_discr,
-               pat);
+    fn walk_pat(&mut self, cmt_discr: mc::cmt<'tcx>, pat: &hir::Pat, match_mode: MatchMode) {
+        debug!("walk_pat cmt_discr={:?} pat={:?}", cmt_discr, pat);
 
         let tcx = &self.tcx();
         let mc = &self.mc;
         let infcx = self.mc.infcx;
         let delegate = &mut self.delegate;
         return_if_err!(mc.cat_pattern(cmt_discr.clone(), pat, |mc, cmt_pat, pat| {
-            match pat.node {
-                PatKind::Binding(bmode, _, _) => {
-                    debug!("binding cmt_pat={:?} pat={:?} match_mode={:?}",
-                           cmt_pat,
-                           pat,
-                           match_mode);
-
-                    // pat_ty: the type of the binding being produced.
-                    let pat_ty = return_if_err!(infcx.node_ty(pat.id));
-
-                    // Each match binding is effectively an assignment to the
-                    // binding being produced.
-                    if let Ok(binding_cmt) = mc.cat_def(pat.id, pat.span, pat_ty,
-                                                        tcx.expect_def(pat.id)) {
-                        delegate.mutate(pat.id, pat.span, binding_cmt, MutateMode::Init);
-                    }
+            if let PatKind::Binding(bmode, _, _) = pat.node {
+                debug!("binding cmt_pat={:?} pat={:?} match_mode={:?}", cmt_pat, pat, match_mode);
 
-                    // It is also a borrow or copy/move of the value being matched.
-                    match bmode {
-                        hir::BindByRef(m) => {
-                            if let ty::TyRef(&r, _) = pat_ty.sty {
-                                let bk = ty::BorrowKind::from_mutbl(m);
-                                delegate.borrow(pat.id, pat.span, cmt_pat,
-                                                r, bk, RefBinding);
-                            }
-                        }
-                        hir::BindByValue(..) => {
-                            let mode = copy_or_move(infcx, &cmt_pat, PatBindingMove);
-                            debug!("walk_pat binding consuming pat");
-                            delegate.consume_pat(pat, cmt_pat, mode);
+                // pat_ty: the type of the binding being produced.
+                let pat_ty = return_if_err!(infcx.node_ty(pat.id));
+
+                // Each match binding is effectively an assignment to the
+                // binding being produced.
+                if let Ok(binding_cmt) = mc.cat_def(pat.id, pat.span, pat_ty,
+                                                    tcx.expect_def(pat.id)) {
+                    delegate.mutate(pat.id, pat.span, binding_cmt, MutateMode::Init);
+                }
+
+                // It is also a borrow or copy/move of the value being matched.
+                match bmode {
+                    hir::BindByRef(m) => {
+                        if let ty::TyRef(&r, _) = pat_ty.sty {
+                            let bk = ty::BorrowKind::from_mutbl(m);
+                            delegate.borrow(pat.id, pat.span, cmt_pat, r, bk, RefBinding);
                         }
                     }
+                    hir::BindByValue(..) => {
+                        let mode = copy_or_move(infcx, &cmt_pat, PatBindingMove);
+                        debug!("walk_pat binding consuming pat");
+                        delegate.consume_pat(pat, cmt_pat, mode);
+                    }
                 }
-                _ => {}
             }
         }));
 
@@ -999,72 +988,23 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
         // to the above loop's visit of than the bindings that form
         // the leaves of the pattern tree structure.
         return_if_err!(mc.cat_pattern(cmt_discr, pat, |mc, cmt_pat, pat| {
-            match pat.node {
-                PatKind::Struct(..) | PatKind::TupleStruct(..) |
-                PatKind::Path(..) | PatKind::QPath(..) => {
-                    match tcx.expect_def(pat.id) {
-                        Def::Variant(enum_did, variant_did) => {
-                            let downcast_cmt =
-                                if tcx.lookup_adt_def(enum_did).is_univariant() {
-                                    cmt_pat
-                                } else {
-                                    let cmt_pat_ty = cmt_pat.ty;
-                                    mc.cat_downcast(pat, cmt_pat, cmt_pat_ty, variant_did)
-                                };
-
-                            debug!("variant downcast_cmt={:?} pat={:?}",
-                                   downcast_cmt,
-                                   pat);
-
-                            delegate.matched_pat(pat, downcast_cmt, match_mode);
-                        }
-
-                        Def::Struct(..) | Def::TyAlias(..) => {
-                            // A struct (in either the value or type
-                            // namespace; we encounter the former on
-                            // e.g. patterns for unit structs).
-
-                            debug!("struct cmt_pat={:?} pat={:?}",
-                                   cmt_pat,
-                                   pat);
-
-                            delegate.matched_pat(pat, cmt_pat, match_mode);
-                        }
-
-                        Def::Const(..) | Def::AssociatedConst(..) => {
-                            // This is a leaf (i.e. identifier binding
-                            // or constant value to match); thus no
-                            // `matched_pat` call.
-                        }
+            match tcx.expect_def_or_none(pat.id) {
+                Some(Def::Variant(enum_did, variant_did)) => {
+                    let downcast_cmt = if tcx.lookup_adt_def(enum_did).is_univariant() {
+                        cmt_pat
+                    } else {
+                        let cmt_pat_ty = cmt_pat.ty;
+                        mc.cat_downcast(pat, cmt_pat, cmt_pat_ty, variant_did)
+                    };
 
-                        def => {
-                            // An enum type should never be in a pattern.
-                            // Remaining cases are e.g. Def::Fn, to
-                            // which identifiers within patterns
-                            // should not resolve. However, we do
-                            // encouter this when using the
-                            // expr-use-visitor during typeck. So just
-                            // ignore it, an error should have been
-                            // reported.
-
-                            if !tcx.sess.has_errors() {
-                                span_bug!(pat.span,
-                                          "Pattern has unexpected def: {:?} and type {:?}",
-                                          def,
-                                          cmt_pat.ty);
-                            }
-                        }
-                    }
+                    debug!("variant downcast_cmt={:?} pat={:?}", downcast_cmt, pat);
+                    delegate.matched_pat(pat, downcast_cmt, match_mode);
                 }
-
-                PatKind::Wild | PatKind::Tuple(..) | PatKind::Box(..) |
-                PatKind::Ref(..) | PatKind::Lit(..) | PatKind::Range(..) |
-                PatKind::Vec(..) | PatKind::Binding(..) => {
-                    // Each of these cases does not
-                    // correspond to an enum variant or struct, so we
-                    // do not do any `matched_pat` calls for these
-                    // cases either.
+                Some(Def::Struct(..)) | Some(Def::TyAlias(..)) | Some(Def::AssociatedTy(..)) => {
+                    debug!("struct cmt_pat={:?} pat={:?}", cmt_pat, pat);
+                    delegate.matched_pat(pat, cmt_pat, match_mode);
                 }
+                _ => {}
             }
         }));
     }