Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove redundancy from the implementation of C variadics. #63492

Merged
merged 4 commits into from
Sep 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/librustc/hir/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {}
}
}
Expand Down
31 changes: 21 additions & 10 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea to skip this here.

// 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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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| {
Expand Down
2 changes: 0 additions & 2 deletions src/librustc/hir/lowering/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand Down Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
3 changes: 0 additions & 3 deletions src/librustc/hir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,9 +361,6 @@ impl<'a> State<'a> {
self.s.word("/*ERROR*/");
self.pclose();
}
hir::TyKind::CVarArgs(_) => {
self.s.word("...");
}
}
self.end()
}
Expand Down
8 changes: 0 additions & 8 deletions src/librustc/middle/resolve_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
Expand Down Expand Up @@ -2378,7 +2371,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
self.visit_lifetime(lifetime);
}
}
hir::TyKind::CVarArgs(_) => {}
_ => {
intravisit::walk_ty(self, ty);
}
Expand Down
35 changes: 2 additions & 33 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}
}

Expand Down
12 changes: 5 additions & 7 deletions src/librustc_codegen_llvm/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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));
Expand Down Expand Up @@ -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)) => {
Expand Down Expand Up @@ -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)) => {
Expand Down
44 changes: 10 additions & 34 deletions src/librustc_codegen_ssa/mir/block.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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() {
Expand All @@ -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());
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down
Loading