rustc: async fn drop order lowering in HIR

This commit re-implements the async fn drop order lowering changes so
that it all takes place in HIR lowering, building atop the work done by
`@eddyb` to refactor `Res::Upvar`.

Previously, this types involved in the lowering were constructed in
libsyntax as they had to be used during name resolution and HIR
lowering. This was awful because none of that logic should have existed
in libsyntax.

This commit also changes `ArgSource` to keep a `HirId` to the original
argument pattern rather than a cloned copy of the pattern.

Author: davidtwco

src/librustc/hir/intravisit.rs

@@ -262,9 +262,6 @@ pub trait Visitor<'v> : Sized {
     fn visit_pat(&mut self, p: &'v Pat) {
         walk_pat(self, p)
     }
-    fn visit_argument_source(&mut self, s: &'v ArgSource) {
-        walk_argument_source(self, s)
-    }
     fn visit_anon_const(&mut self, c: &'v AnonConst) {
         walk_anon_const(self, c)
     }
@@ -402,17 +399,10 @@ pub fn walk_body<'v, V: Visitor<'v>>(visitor: &mut V, body: &'v Body) {
     for argument in &body.arguments {
         visitor.visit_id(argument.hir_id);
         visitor.visit_pat(&argument.pat);
-        visitor.visit_argument_source(&argument.source);
     }
     visitor.visit_expr(&body.value);
 }
 
-pub fn walk_argument_source<'v, V: Visitor<'v>>(visitor: &mut V, source: &'v ArgSource) {
-    if let ArgSource::AsyncFn(pat) = source {
-        visitor.visit_pat(pat);
-    }
-}
-
 pub fn walk_local<'v, V: Visitor<'v>>(visitor: &mut V, local: &'v Local) {
     // Intentionally visiting the expr first - the initialization expr
     // dominates the local's definition.

src/librustc/hir/lowering.rs

@@ -699,6 +699,19 @@ impl<'hir> Map<'hir> {
         }
     }
 
+    /// Returns the `HirId` of this pattern, or, if this is an `async fn` desugaring, the `HirId`
+    /// of the original pattern that the user wrote.
+    pub fn original_pat_of_argument(&self, arg: &'hir Arg) -> &'hir Pat {
+        match &arg.source {
+            ArgSource::Normal => &*arg.pat,
+            ArgSource::AsyncFn(hir_id) => match self.find_by_hir_id(*hir_id) {
+                Some(Node::Pat(pat)) | Some(Node::Binding(pat)) => &pat,
+                Some(Node::Local(local)) => &*local.pat,
+                x => bug!("ArgSource::AsyncFn HirId not a pattern/binding/local: {:?}", x),
+            },
+        }
+    }
+
     pub fn is_const_scope(&self, hir_id: HirId) -> bool {
         self.walk_parent_nodes(hir_id, |node| match *node {
             Node::Item(Item { node: ItemKind::Const(_, _), .. }) => true,

src/librustc/hir/map/mod.rs

@@ -1932,23 +1932,13 @@ pub struct Arg {
     pub source: ArgSource,
 }
 
-impl Arg {
-    /// Returns the pattern representing the original binding for this argument.
-    pub fn original_pat(&self) -> &P<Pat> {
-        match &self.source {
-            ArgSource::Normal => &self.pat,
-            ArgSource::AsyncFn(pat) => &pat,
-        }
-    }
-}
-
 /// Represents the source of an argument in a function header.
 #[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable)]
 pub enum ArgSource {
     /// Argument as specified by the user.
     Normal,
-    /// Generated argument from `async fn` lowering, contains the original binding pattern.
-    AsyncFn(P<Pat>),
+    /// Generated argument from `async fn` lowering, `HirId` is the original pattern.
+    AsyncFn(HirId),
 }
 
 /// Represents the header (not the body) of a function declaration.

src/librustc/hir/mod.rs

@@ -86,12 +86,14 @@ impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> {
         let sub_is_ret_type =
             self.is_return_type_anon(scope_def_id_sub, bregion_sub, ty_fndecl_sub);
 
-        let span_label_var1 = match anon_arg_sup.original_pat().simple_ident() {
+        let arg_sup_pat = self.tcx().hir().original_pat_of_argument(anon_arg_sup);
+        let span_label_var1 = match arg_sup_pat.simple_ident() {
             Some(simple_ident) => format!(" from `{}`", simple_ident),
             None => String::new(),
         };
 
-        let span_label_var2 = match anon_arg_sub.original_pat().simple_ident() {
+        let arg_sub_pat = self.tcx().hir().original_pat_of_argument(anon_arg_sub);
+        let span_label_var2 = match arg_sub_pat.simple_ident() {
             Some(simple_ident) => format!(" into `{}`", simple_ident),
             None => String::new(),
         };

src/librustc/infer/error_reporting/nice_region_error/different_lifetimes.rs

@@ -95,7 +95,8 @@ impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> {
             }
         }
 
-        let (error_var, span_label_var) = match arg.original_pat().simple_ident() {
+        let arg_pat = self.tcx().hir().original_pat_of_argument(arg);
+        let (error_var, span_label_var) = match arg_pat.simple_ident() {
             Some(simple_ident) => (
                 format!("the type of `{}`", simple_ident),
                 format!("the type of `{}`", simple_ident),

src/librustc/infer/error_reporting/nice_region_error/named_anon_conflict.rs

@@ -2416,7 +2416,8 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
 
             let help_name = if let Some(body) = parent {
                 let arg = &self.tcx.hir().body(body).arguments[index];
-                format!("`{}`", self.tcx.hir().hir_to_pretty_string(arg.original_pat().hir_id))
+                let original_pat = self.tcx.hir().original_pat_of_argument(arg);
+                format!("`{}`", self.tcx.hir().hir_to_pretty_string(original_pat.hir_id))
             } else {
                 format!("argument {}", index + 1)
             };

src/librustc/middle/resolve_lifetime.rs

@@ -84,11 +84,23 @@ pub fn mir_build<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Body<'
                     // HACK(eddyb) Avoid having RustCall on closures,
                     // as it adds unnecessary (and wrong) auto-tupling.
                     abi = Abi::Rust;
-                    Some(ArgInfo(liberated_closure_env_ty(tcx, id, body_id), None, None, None))
+                    Some(ArgInfo {
+                        ty: liberated_closure_env_ty(tcx, id, body_id),
+                        span: None,
+                        pattern: None,
+                        user_pattern: None,
+                        self_kind: None,
+                    })
                 }
                 ty::Generator(..) => {
                     let gen_ty = tcx.body_tables(body_id).node_type(id);
-                    Some(ArgInfo(gen_ty, None, None, None))
+                    Some(ArgInfo {
+                        ty: gen_ty,
+                        span: None,
+                        pattern: None,
+                        user_pattern: None,
+                        self_kind: None,
+                    })
                 }
                 _ => None,
             };
@@ -126,7 +138,15 @@ pub fn mir_build<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Body<'
                             opt_ty_info = None;
                             self_arg = None;
                         }
-                        ArgInfo(fn_sig.inputs()[index], opt_ty_info, Some(&*arg.pat), self_arg)
+
+                        let original_pat = tcx.hir().original_pat_of_argument(arg);
+                        ArgInfo {
+                            ty: fn_sig.inputs()[index],
+                            span: opt_ty_info,
+                            pattern: Some(&*arg.pat),
+                            user_pattern: Some(&original_pat),
+                            self_kind: self_arg,
+                        }
                     });
 
             let arguments = implicit_argument.into_iter().chain(explicit_arguments);
@@ -614,10 +634,13 @@ fn should_abort_on_panic<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
 ///////////////////////////////////////////////////////////////////////////
 /// the main entry point for building MIR for a function
 
-struct ArgInfo<'gcx>(Ty<'gcx>,
-                     Option<Span>,
-                     Option<&'gcx hir::Pat>,
-                     Option<ImplicitSelfKind>);
+struct ArgInfo<'gcx> {
+    ty: Ty<'gcx>,
+    span: Option<Span>,
+    pattern: Option<&'gcx hir::Pat>,
+    user_pattern: Option<&'gcx hir::Pat>,
+    self_kind: Option<ImplicitSelfKind>,
+}
 
 fn construct_fn<'a, 'gcx, 'tcx, A>(hir: Cx<'a, 'gcx, 'tcx>,
                                    fn_id: hir::HirId,
@@ -878,26 +901,23 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                      -> BlockAnd<()>
     {
         // Allocate locals for the function arguments
-        for &ArgInfo(ty, _, pattern, _) in arguments.iter() {
+        for &ArgInfo { ty, span: _, pattern, user_pattern, self_kind: _ } in arguments.iter() {
             // If this is a simple binding pattern, give the local a name for
             // debuginfo and so that error reporting knows that this is a user
             // variable. For any other pattern the pattern introduces new
             // variables which will be named instead.
-            let mut name = None;
-            if let Some(pat) = pattern {
+            let (name, span) = if let Some(pat) = user_pattern {
                 match pat.node {
                     hir::PatKind::Binding(hir::BindingAnnotation::Unannotated, _, ident, _)
-                    | hir::PatKind::Binding(hir::BindingAnnotation::Mutable, _, ident, _) => {
-                        name = Some(ident.name);
-                    }
-                    _ => (),
+                    | hir::PatKind::Binding(hir::BindingAnnotation::Mutable, _, ident, _) =>
+                        (Some(ident.name), pat.span),
+                    _ => (None, pattern.map_or(self.fn_span, |pat| pat.span))
                 }
-            }
-
-            let source_info = SourceInfo {
-                scope: OUTERMOST_SOURCE_SCOPE,
-                span: pattern.map_or(self.fn_span, |pat| pat.span)
+            } else {
+                (None, self.fn_span)
             };
+
+            let source_info = SourceInfo { scope: OUTERMOST_SOURCE_SCOPE, span, };
             self.local_decls.push(LocalDecl {
                 mutability: Mutability::Mut,
                 ty,
@@ -917,7 +937,13 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
             // Function arguments always get the first Local indices after the return place
             let local = Local::new(index + 1);
             let place = Place::Base(PlaceBase::Local(local));
-            let &ArgInfo(ty, opt_ty_info, pattern, ref self_binding) = arg_info;
+            let &ArgInfo {
+                ty,
+                span: opt_ty_info,
+                pattern,
+                user_pattern: _,
+                self_kind: ref self_binding
+            } = arg_info;
 
             // Make sure we drop (parts of) the argument even when not matched on.
             self.schedule_drop(

src/librustc_mir/build/mod.rs

@@ -948,16 +948,6 @@ impl<'a, 'tcx> Visitor<'tcx> for NamePrivacyVisitor<'a, 'tcx> {
 
         intravisit::walk_pat(self, pat);
     }
-
-    fn visit_argument_source(&mut self, s: &'tcx hir::ArgSource) {
-        match s {
-            // Don't visit the pattern in `ArgSource::AsyncFn`, it contains a pattern which has
-            // a `NodeId` w/out a type, as it is only used for getting the name of the original
-            // pattern for diagnostics where only an `hir::Arg` is present.
-            hir::ArgSource::AsyncFn(..) => {},
-            _ => intravisit::walk_argument_source(self, s),
-        }
-    }
 }
 
 ////////////////////////////////////////////////////////////////////////////////////////////
@@ -1147,16 +1137,6 @@ impl<'a, 'tcx> Visitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
         intravisit::walk_pat(self, pattern);
     }
 
-    fn visit_argument_source(&mut self, s: &'tcx hir::ArgSource) {
-        match s {
-            // Don't visit the pattern in `ArgSource::AsyncFn`, it contains a pattern which has
-            // a `NodeId` w/out a type, as it is only used for getting the name of the original
-            // pattern for diagnostics where only an `hir::Arg` is present.
-            hir::ArgSource::AsyncFn(..) => {},
-            _ => intravisit::walk_argument_source(self, s),
-        }
-    }
-
     fn visit_local(&mut self, local: &'tcx hir::Local) {
         if let Some(ref init) = local.init {
             if self.check_expr_pat_type(init.hir_id, init.span) {

src/librustc_privacy/lib.rs

@@ -1009,16 +1009,6 @@ impl<'a, 'gcx, 'tcx> Visitor<'gcx> for GatherLocalsVisitor<'a, 'gcx, 'tcx> {
     // Don't descend into the bodies of nested closures
     fn visit_fn(&mut self, _: intravisit::FnKind<'gcx>, _: &'gcx hir::FnDecl,
                 _: hir::BodyId, _: Span, _: hir::HirId) { }
-
-    fn visit_argument_source(&mut self, s: &'gcx hir::ArgSource) {
-        match s {
-            // Don't visit the pattern in `ArgSource::AsyncFn`, it contains a pattern which has
-            // a `NodeId` w/out a type, as it is only used for getting the name of the original
-            // pattern for diagnostics where only an `hir::Arg` is present.
-            hir::ArgSource::AsyncFn(..) => {},
-            _ => intravisit::walk_argument_source(self, s),
-        }
-    }
 }
 
 /// When `check_fn` is invoked on a generator (i.e., a body that