Rollup merge of rust-lang#63492 - eddyb:cvarargs, r=nagisa,matthewjasper

Remove redundancy from the implementation of C variadics.

This cleanup was first described in rust-lang#44930 (comment):

* AST doesn't track `c_variadic: bool` anymore, relying solely on a trailing `CVarArgs` type in fn signatures
* HIR doesn't have a `CVarArgs` anymore, relying solely on `c_variadic: bool`
  * same for `ty::FnSig` (see tests for diagnostics improvements from that)
  * `{hir,mir}::Body` have one extra argument than the signature when `c_variadic == true`
  * `rustc_typeck` and `rustc_mir::{build,borrowck}` need to give that argument the right type (which no longer uses a lifetime parameter, but a function-internal scope)
* `rustc_target::abi::call` doesn't need special hacks anymore (since it never sees the `VaListImpl` now, it's all inside the body)

r? @nagisa / @rkruppe cc @dlrobertson @oli-obk

Author: Centril

src/librustc/hir/intravisit.rs

@@ -633,9 +633,6 @@ pub fn walk_ty<'v, V: Visitor<'v>>(visitor: &mut V, typ: &'v Ty) {
         TyKind::Typeof(ref expression) => {
             visitor.visit_anon_const(expression)
         }
-        TyKind::CVarArgs(ref lt) => {
-            visitor.visit_lifetime(lt)
-        }
         TyKind::Infer | TyKind::Err => {}
     }
 }

src/librustc/hir/lowering.rs

@@ -1335,13 +1335,8 @@ impl<'a> LoweringContext<'a> {
                     }
                 }
             }
-            TyKind::Mac(_) => bug!("`TyMac` should have been expanded by now"),
-            TyKind::CVarArgs => {
-                // Create the implicit lifetime of the "spoofed" `VaListImpl`.
-                let span = self.sess.source_map().next_point(t.span.shrink_to_lo());
-                let lt = self.new_implicit_lifetime(span);
-                hir::TyKind::CVarArgs(lt)
-            },
+            TyKind::Mac(_) => bug!("`TyKind::Mac` should have been expanded by now"),
+            TyKind::CVarArgs => bug!("`TyKind::CVarArgs` should have been handled elsewhere"),
         };
 
         hir::Ty {
@@ -2093,7 +2088,14 @@ impl<'a> LoweringContext<'a> {
     }
 
     fn lower_fn_params_to_names(&mut self, decl: &FnDecl) -> hir::HirVec<Ident> {
-        decl.inputs
+        // Skip the `...` (`CVarArgs`) trailing arguments from the AST,
+        // as they are not explicit in HIR/Ty function signatures.
+        // (instead, the `c_variadic` flag is set to `true`)
+        let mut inputs = &decl.inputs[..];
+        if decl.c_variadic() {
+            inputs = &inputs[..inputs.len() - 1];
+        }
+        inputs
             .iter()
             .map(|param| match param.pat.kind {
                 PatKind::Ident(_, ident, _) => ident,
@@ -2130,10 +2132,19 @@ impl<'a> LoweringContext<'a> {
             self.anonymous_lifetime_mode
         };
 
+        let c_variadic = decl.c_variadic();
+
         // Remember how many lifetimes were already around so that we can
         // only look at the lifetime parameters introduced by the arguments.
         let inputs = self.with_anonymous_lifetime_mode(lt_mode, |this| {
-            decl.inputs
+            // Skip the `...` (`CVarArgs`) trailing arguments from the AST,
+            // as they are not explicit in HIR/Ty function signatures.
+            // (instead, the `c_variadic` flag is set to `true`)
+            let mut inputs = &decl.inputs[..];
+            if c_variadic {
+                inputs = &inputs[..inputs.len() - 1];
+            }
+            inputs
                 .iter()
                 .map(|param| {
                     if let Some((_, ibty)) = &mut in_band_ty_params {
@@ -2168,7 +2179,7 @@ impl<'a> LoweringContext<'a> {
         P(hir::FnDecl {
             inputs,
             output,
-            c_variadic: decl.c_variadic,
+            c_variadic,
             implicit_self: decl.inputs.get(0).map_or(
                 hir::ImplicitSelfKind::None,
                 |arg| {

src/librustc/hir/lowering/expr.rs

@@ -450,7 +450,6 @@ impl LoweringContext<'_> {
         let ast_decl = FnDecl {
             inputs: vec![],
             output,
-            c_variadic: false
         };
         let decl = self.lower_fn_decl(&ast_decl, None, /* impl trait allowed */ false, None);
         let body_id = self.lower_fn_body(&ast_decl, |this| {
@@ -739,7 +738,6 @@ impl LoweringContext<'_> {
         let outer_decl = FnDecl {
             inputs: decl.inputs.clone(),
             output: FunctionRetTy::Default(fn_decl_span),
-            c_variadic: false,
         };
         // We need to lower the declaration outside the new scope, because we
         // have to conserve the state of being inside a loop condition for the

src/librustc/hir/mod.rs

@@ -2016,9 +2016,6 @@ pub enum TyKind {
     Infer,
     /// Placeholder for a type that has failed to be defined.
     Err,
-    /// Placeholder for C-variadic arguments. We "spoof" the `VaListImpl` created
-    /// from the variadic arguments. This type is only valid up to typeck.
-    CVarArgs(Lifetime),
 }
 
 #[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug, HashStable)]

src/librustc/hir/print.rs

@@ -361,9 +361,6 @@ impl<'a> State<'a> {
                 self.s.word("/*ERROR*/");
                 self.pclose();
             }
-            hir::TyKind::CVarArgs(_) => {
-                self.s.word("...");
-            }
         }
         self.end()
     }

src/librustc/middle/resolve_lifetime.rs

@@ -764,13 +764,6 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
                     });
                 }
             }
-            hir::TyKind::CVarArgs(ref lt) => {
-                // Resolve the generated lifetime for the C-variadic arguments.
-                // The lifetime is generated in AST -> HIR lowering.
-                if lt.name.is_elided() {
-                    self.resolve_elided_lifetimes(vec![lt])
-                }
-            }
             _ => intravisit::walk_ty(self, ty),
         }
     }
@@ -2378,7 +2371,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
                             self.visit_lifetime(lifetime);
                         }
                     }
-                    hir::TyKind::CVarArgs(_) => {}
                     _ => {
                         intravisit::walk_ty(self, ty);
                     }

src/librustc/ty/layout.rs

@@ -25,7 +25,7 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher,
 pub use rustc_target::abi::*;
 use rustc_target::spec::{HasTargetSpec, abi::Abi as SpecAbi};
 use rustc_target::abi::call::{
-    ArgAttribute, ArgAttributes, ArgType, Conv, FnType, IgnoreMode, PassMode, Reg, RegKind
+    ArgAttribute, ArgAttributes, ArgType, Conv, FnType, PassMode, Reg, RegKind
 };
 
 pub trait IntegerExt {
@@ -2722,14 +2722,6 @@ where
             }
         };
 
-        // Store the index of the last argument. This is useful for working with
-        // C-compatible variadic arguments.
-        let last_arg_idx = if sig.inputs().is_empty() {
-            None
-        } else {
-            Some(sig.inputs().len() - 1)
-        };
-
         let arg_of = |ty: Ty<'tcx>, arg_idx: Option<usize>| {
             let is_return = arg_idx.is_none();
             let mut arg = mk_arg_type(ty, arg_idx);
@@ -2739,30 +2731,7 @@ where
                 // The same is true for s390x-unknown-linux-gnu
                 // and sparc64-unknown-linux-gnu.
                 if is_return || rust_abi || (!win_x64_gnu && !linux_s390x && !linux_sparc64) {
-                    arg.mode = PassMode::Ignore(IgnoreMode::Zst);
-                }
-            }
-
-            // If this is a C-variadic function, this is not the return value,
-            // and there is one or more fixed arguments; ensure that the `VaListImpl`
-            // is ignored as an argument.
-            if sig.c_variadic {
-                match (last_arg_idx, arg_idx) {
-                    (Some(last_idx), Some(cur_idx)) if last_idx == cur_idx => {
-                        let va_list_did = match cx.tcx().lang_items().va_list() {
-                            Some(did) => did,
-                            None => bug!("`va_list` lang item required for C-variadic functions"),
-                        };
-                        match ty.kind {
-                            ty::Adt(def, _) if def.did == va_list_did => {
-                                // This is the "spoofed" `VaListImpl`. Set the arguments mode
-                                // so that it will be ignored.
-                                arg.mode = PassMode::Ignore(IgnoreMode::CVarArgs);
-                            }
-                            _ => (),
-                        }
-                    }
-                    _ => {}
+                    arg.mode = PassMode::Ignore;
                 }
             }
 

src/librustc_codegen_llvm/abi.rs

@@ -264,7 +264,7 @@ impl ArgTypeExt<'ll, 'tcx> for ArgType<'tcx, Ty<'tcx>> {
             val
         };
         match self.mode {
-            PassMode::Ignore(_) => {}
+            PassMode::Ignore => {}
             PassMode::Pair(..) => {
                 OperandValue::Pair(next(), next()).store(bx, dst);
             }
@@ -319,9 +319,7 @@ impl<'tcx> FnTypeLlvmExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
         );
 
         let llreturn_ty = match self.ret.mode {
-            PassMode::Ignore(IgnoreMode::Zst) => cx.type_void(),
-            PassMode::Ignore(IgnoreMode::CVarArgs) =>
-                bug!("`va_list` should never be a return type"),
+            PassMode::Ignore => cx.type_void(),
             PassMode::Direct(_) | PassMode::Pair(..) => {
                 self.ret.layout.immediate_llvm_type(cx)
             }
@@ -339,7 +337,7 @@ impl<'tcx> FnTypeLlvmExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
             }
 
             let llarg_ty = match arg.mode {
-                PassMode::Ignore(_) => continue,
+                PassMode::Ignore => continue,
                 PassMode::Direct(_) => arg.layout.immediate_llvm_type(cx),
                 PassMode::Pair(..) => {
                     llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 0, true));
@@ -408,7 +406,7 @@ impl<'tcx> FnTypeLlvmExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
                 apply(&ArgAttributes::new(), None);
             }
             match arg.mode {
-                PassMode::Ignore(_) => {}
+                PassMode::Ignore => {}
                 PassMode::Direct(ref attrs) |
                 PassMode::Indirect(ref attrs, None) => apply(attrs, Some(arg.layout.llvm_type(cx))),
                 PassMode::Indirect(ref attrs, Some(ref extra_attrs)) => {
@@ -455,7 +453,7 @@ impl<'tcx> FnTypeLlvmExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
                 apply(&ArgAttributes::new(), None);
             }
             match arg.mode {
-                PassMode::Ignore(_) => {}
+                PassMode::Ignore => {}
                 PassMode::Direct(ref attrs) |
                 PassMode::Indirect(ref attrs, None) => apply(attrs, Some(arg.layout.llvm_type(bx))),
                 PassMode::Indirect(ref attrs, Some(ref extra_attrs)) => {

src/librustc_codegen_ssa/mir/block.rs

@@ -1,9 +1,10 @@
+use rustc_data_structures::indexed_vec::Idx;
 use rustc::middle::lang_items;
 use rustc::ty::{self, Ty, TypeFoldable, Instance};
 use rustc::ty::layout::{self, LayoutOf, HasTyCtxt, FnTypeExt};
 use rustc::mir::{self, Place, PlaceBase, Static, StaticKind};
 use rustc::mir::interpret::PanicInfo;
-use rustc_target::abi::call::{ArgType, FnType, PassMode, IgnoreMode};
+use rustc_target::abi::call::{ArgType, FnType, PassMode};
 use rustc_target::spec::abi::Abi;
 use crate::base;
 use crate::MemFlags;
@@ -224,14 +225,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
     }
 
     fn codegen_return_terminator(&mut self, mut bx: Bx) {
+        // Call `va_end` if this is the definition of a C-variadic function.
         if self.fn_ty.c_variadic {
-            match self.va_list_ref {
-                Some(va_list) => {
+            // The `VaList` "spoofed" argument is just after all the real arguments.
+            let va_list_arg_idx = self.fn_ty.args.len();
+            match self.locals[mir::Local::new(1 + va_list_arg_idx)] {
+                LocalRef::Place(va_list) => {
                     bx.va_end(va_list.llval);
                 }
-                None => {
-                    bug!("C-variadic function must have a `va_list_ref`");
-                }
+                _ => bug!("C-variadic function must have a `VaList` place"),
             }
         }
         if self.fn_ty.ret.layout.abi.is_uninhabited() {
@@ -242,15 +244,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             return;
         }
         let llval = match self.fn_ty.ret.mode {
-            PassMode::Ignore(IgnoreMode::Zst) | PassMode::Indirect(..) => {
+            PassMode::Ignore | PassMode::Indirect(..) => {
                 bx.ret_void();
                 return;
             }
 
-            PassMode::Ignore(IgnoreMode::CVarArgs) => {
-                bug!("C-variadic arguments should never be the return type");
-            }
-
             PassMode::Direct(_) | PassMode::Pair(..) => {
                 let op =
                     self.codegen_consume(&mut bx, &mir::Place::return_place().as_ref());
@@ -502,10 +500,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             return;
         }
 
-        // The "spoofed" `VaListImpl` added to a C-variadic functions signature
-        // should not be included in the `extra_args` calculation.
-        let extra_args_start_idx = sig.inputs().len() - if sig.c_variadic { 1 } else { 0 };
-        let extra_args = &args[extra_args_start_idx..];
+        let extra_args = &args[sig.inputs().len()..];
         let extra_args = extra_args.iter().map(|op_arg| {
             let op_ty = op_arg.ty(self.mir, bx.tcx());
             self.monomorphize(&op_ty)
@@ -691,26 +686,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             (&args[..], None)
         };
 
-        // Useful determining if the current argument is the "spoofed" `VaListImpl`
-        let last_arg_idx = if sig.inputs().is_empty() {
-            None
-        } else {
-            Some(sig.inputs().len() - 1)
-        };
         'make_args: for (i, arg) in first_args.iter().enumerate() {
-            // If this is a C-variadic function the function signature contains
-            // an "spoofed" `VaListImpl`. This argument is ignored, but we need to
-            // populate it with a dummy operand so that the users real arguments
-            // are not overwritten.
-            let i = if sig.c_variadic && last_arg_idx.map(|x| i >= x).unwrap_or(false) {
-                if i + 1 < fn_ty.args.len() {
-                    i + 1
-                } else {
-                    break 'make_args
-                }
-            } else {
-                i
-            };
             let mut op = self.codegen_operand(&mut bx, arg);
 
             if let (0, Some(ty::InstanceDef::Virtual(_, idx))) = (i, def) {