Skip to content

Commit 65a96d9

Browse files
CentrilCentril
authored
Rollup merge of #63492 - eddyb:cvarargs, r=nagisa
Remove redundancy from the implementation of C variadics. This cleanup was first described in #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
2 parents c6f87c2 + 003c5ec commit 65a96d9

File tree

41 files changed

+294
-406
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+294
-406
lines changed

‎src/librustc/hir/intravisit.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -633,9 +633,6 @@ pub fn walk_ty<'v, V: Visitor<'v>>(visitor: &mut V, typ: &'v Ty) {
633633
TyKind::Typeof(ref expression) => {
634634
visitor.visit_anon_const(expression)
635635
}
636-
TyKind::CVarArgs(ref lt) => {
637-
visitor.visit_lifetime(lt)
638-
}
639636
TyKind::Infer | TyKind::Err => {}
640637
}
641638
}

‎src/librustc/hir/lowering.rs

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1335,13 +1335,8 @@ impl<'a> LoweringContext<'a> {
13351335
}
13361336
}
13371337
}
1338-
TyKind::Mac(_) => bug!("`TyMac` should have been expanded by now"),
1339-
TyKind::CVarArgs => {
1340-
// Create the implicit lifetime of the "spoofed" `VaListImpl`.
1341-
let span = self.sess.source_map().next_point(t.span.shrink_to_lo());
1342-
let lt = self.new_implicit_lifetime(span);
1343-
hir::TyKind::CVarArgs(lt)
1344-
},
1338+
TyKind::Mac(_) => bug!("`TyKind::Mac` should have been expanded by now"),
1339+
TyKind::CVarArgs => bug!("`TyKind::CVarArgs` should have been handled elsewhere"),
13451340
};
13461341

13471342
hir::Ty {
@@ -2093,7 +2088,14 @@ impl<'a> LoweringContext<'a> {
20932088
}
20942089

20952090
fn lower_fn_params_to_names(&mut self, decl: &FnDecl) -> hir::HirVec<Ident> {
2096-
decl.inputs
2091+
// Skip the `...` (`CVarArgs`) trailing arguments from the AST,
2092+
// as they are not explicit in HIR/Ty function signatures.
2093+
// (instead, the `c_variadic` flag is set to `true`)
2094+
let mut inputs = &decl.inputs[..];
2095+
if decl.c_variadic() {
2096+
inputs = &inputs[..inputs.len() - 1];
2097+
}
2098+
inputs
20972099
.iter()
20982100
.map(|param| match param.pat.kind {
20992101
PatKind::Ident(_, ident, _) => ident,
@@ -2130,10 +2132,19 @@ impl<'a> LoweringContext<'a> {
21302132
self.anonymous_lifetime_mode
21312133
};
21322134

2135+
let c_variadic = decl.c_variadic();
2136+
21332137
// Remember how many lifetimes were already around so that we can
21342138
// only look at the lifetime parameters introduced by the arguments.
21352139
let inputs = self.with_anonymous_lifetime_mode(lt_mode, |this| {
2136-
decl.inputs
2140+
// Skip the `...` (`CVarArgs`) trailing arguments from the AST,
2141+
// as they are not explicit in HIR/Ty function signatures.
2142+
// (instead, the `c_variadic` flag is set to `true`)
2143+
let mut inputs = &decl.inputs[..];
2144+
if c_variadic {
2145+
inputs = &inputs[..inputs.len() - 1];
2146+
}
2147+
inputs
21372148
.iter()
21382149
.map(|param| {
21392150
if let Some((_, ibty)) = &mut in_band_ty_params {
@@ -2168,7 +2179,7 @@ impl<'a> LoweringContext<'a> {
21682179
P(hir::FnDecl {
21692180
inputs,
21702181
output,
2171-
c_variadic: decl.c_variadic,
2182+
c_variadic,
21722183
implicit_self: decl.inputs.get(0).map_or(
21732184
hir::ImplicitSelfKind::None,
21742185
|arg| {

‎src/librustc/hir/lowering/expr.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,6 @@ impl LoweringContext<'_> {
450450
let ast_decl = FnDecl {
451451
inputs: vec![],
452452
output,
453-
c_variadic: false
454453
};
455454
let decl = self.lower_fn_decl(&ast_decl, None, /* impl trait allowed */ false, None);
456455
let body_id = self.lower_fn_body(&ast_decl, |this| {
@@ -740,7 +739,6 @@ impl LoweringContext<'_> {
740739
let outer_decl = FnDecl {
741740
inputs: decl.inputs.clone(),
742741
output: FunctionRetTy::Default(fn_decl_span),
743-
c_variadic: false,
744742
};
745743
// We need to lower the declaration outside the new scope, because we
746744
// have to conserve the state of being inside a loop condition for the

‎src/librustc/hir/mod.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2016,9 +2016,6 @@ pub enum TyKind {
20162016
Infer,
20172017
/// Placeholder for a type that has failed to be defined.
20182018
Err,
2019-
/// Placeholder for C-variadic arguments. We "spoof" the `VaListImpl` created
2020-
/// from the variadic arguments. This type is only valid up to typeck.
2021-
CVarArgs(Lifetime),
20222019
}
20232020

20242021
#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug, HashStable)]

‎src/librustc/hir/print.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -361,9 +361,6 @@ impl<'a> State<'a> {
361361
self.s.word("/*ERROR*/");
362362
self.pclose();
363363
}
364-
hir::TyKind::CVarArgs(_) => {
365-
self.s.word("...");
366-
}
367364
}
368365
self.end()
369366
}

‎src/librustc/middle/resolve_lifetime.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -764,13 +764,6 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
764764
});
765765
}
766766
}
767-
hir::TyKind::CVarArgs(ref lt) => {
768-
// Resolve the generated lifetime for the C-variadic arguments.
769-
// The lifetime is generated in AST -> HIR lowering.
770-
if lt.name.is_elided() {
771-
self.resolve_elided_lifetimes(vec![lt])
772-
}
773-
}
774767
_ => intravisit::walk_ty(self, ty),
775768
}
776769
}
@@ -2378,7 +2371,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
23782371
self.visit_lifetime(lifetime);
23792372
}
23802373
}
2381-
hir::TyKind::CVarArgs(_) => {}
23822374
_ => {
23832375
intravisit::walk_ty(self, ty);
23842376
}

‎src/librustc/ty/layout.rs

Lines changed: 2 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher,
2525
pub use rustc_target::abi::*;
2626
use rustc_target::spec::{HasTargetSpec, abi::Abi as SpecAbi};
2727
use rustc_target::abi::call::{
28-
ArgAttribute, ArgAttributes, ArgType, Conv, FnType, IgnoreMode, PassMode, Reg, RegKind
28+
ArgAttribute, ArgAttributes, ArgType, Conv, FnType, PassMode, Reg, RegKind
2929
};
3030

3131
pub trait IntegerExt {
@@ -2722,14 +2722,6 @@ where
27222722
}
27232723
};
27242724

2725-
// Store the index of the last argument. This is useful for working with
2726-
// C-compatible variadic arguments.
2727-
let last_arg_idx = if sig.inputs().is_empty() {
2728-
None
2729-
} else {
2730-
Some(sig.inputs().len() - 1)
2731-
};
2732-
27332725
let arg_of = |ty: Ty<'tcx>, arg_idx: Option<usize>| {
27342726
let is_return = arg_idx.is_none();
27352727
let mut arg = mk_arg_type(ty, arg_idx);
@@ -2739,30 +2731,7 @@ where
27392731
// The same is true for s390x-unknown-linux-gnu
27402732
// and sparc64-unknown-linux-gnu.
27412733
if is_return || rust_abi || (!win_x64_gnu && !linux_s390x && !linux_sparc64) {
2742-
arg.mode = PassMode::Ignore(IgnoreMode::Zst);
2743-
}
2744-
}
2745-
2746-
// If this is a C-variadic function, this is not the return value,
2747-
// and there is one or more fixed arguments; ensure that the `VaListImpl`
2748-
// is ignored as an argument.
2749-
if sig.c_variadic {
2750-
match (last_arg_idx, arg_idx) {
2751-
(Some(last_idx), Some(cur_idx)) if last_idx == cur_idx => {
2752-
let va_list_did = match cx.tcx().lang_items().va_list() {
2753-
Some(did) => did,
2754-
None => bug!("`va_list` lang item required for C-variadic functions"),
2755-
};
2756-
match ty.kind {
2757-
ty::Adt(def, _) if def.did == va_list_did => {
2758-
// This is the "spoofed" `VaListImpl`. Set the arguments mode
2759-
// so that it will be ignored.
2760-
arg.mode = PassMode::Ignore(IgnoreMode::CVarArgs);
2761-
}
2762-
_ => (),
2763-
}
2764-
}
2765-
_ => {}
2734+
arg.mode = PassMode::Ignore;
27662735
}
27672736
}
27682737

‎src/librustc_codegen_llvm/abi.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ impl ArgTypeExt<'ll, 'tcx> for ArgType<'tcx, Ty<'tcx>> {
264264
val
265265
};
266266
match self.mode {
267-
PassMode::Ignore(_) => {}
267+
PassMode::Ignore => {}
268268
PassMode::Pair(..) => {
269269
OperandValue::Pair(next(), next()).store(bx, dst);
270270
}
@@ -319,9 +319,7 @@ impl<'tcx> FnTypeLlvmExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
319319
);
320320

321321
let llreturn_ty = match self.ret.mode {
322-
PassMode::Ignore(IgnoreMode::Zst) => cx.type_void(),
323-
PassMode::Ignore(IgnoreMode::CVarArgs) =>
324-
bug!("`va_list` should never be a return type"),
322+
PassMode::Ignore => cx.type_void(),
325323
PassMode::Direct(_) | PassMode::Pair(..) => {
326324
self.ret.layout.immediate_llvm_type(cx)
327325
}
@@ -339,7 +337,7 @@ impl<'tcx> FnTypeLlvmExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
339337
}
340338

341339
let llarg_ty = match arg.mode {
342-
PassMode::Ignore(_) => continue,
340+
PassMode::Ignore => continue,
343341
PassMode::Direct(_) => arg.layout.immediate_llvm_type(cx),
344342
PassMode::Pair(..) => {
345343
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>> {
408406
apply(&ArgAttributes::new(), None);
409407
}
410408
match arg.mode {
411-
PassMode::Ignore(_) => {}
409+
PassMode::Ignore => {}
412410
PassMode::Direct(ref attrs) |
413411
PassMode::Indirect(ref attrs, None) => apply(attrs, Some(arg.layout.llvm_type(cx))),
414412
PassMode::Indirect(ref attrs, Some(ref extra_attrs)) => {
@@ -455,7 +453,7 @@ impl<'tcx> FnTypeLlvmExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
455453
apply(&ArgAttributes::new(), None);
456454
}
457455
match arg.mode {
458-
PassMode::Ignore(_) => {}
456+
PassMode::Ignore => {}
459457
PassMode::Direct(ref attrs) |
460458
PassMode::Indirect(ref attrs, None) => apply(attrs, Some(arg.layout.llvm_type(bx))),
461459
PassMode::Indirect(ref attrs, Some(ref extra_attrs)) => {

‎src/librustc_codegen_ssa/mir/block.rs

Lines changed: 10 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1+
use rustc_data_structures::indexed_vec::Idx;
12
use rustc::middle::lang_items;
23
use rustc::ty::{self, Ty, TypeFoldable, Instance};
34
use rustc::ty::layout::{self, LayoutOf, HasTyCtxt, FnTypeExt};
45
use rustc::mir::{self, Place, PlaceBase, Static, StaticKind};
56
use rustc::mir::interpret::PanicInfo;
6-
use rustc_target::abi::call::{ArgType, FnType, PassMode, IgnoreMode};
7+
use rustc_target::abi::call::{ArgType, FnType, PassMode};
78
use rustc_target::spec::abi::Abi;
89
use crate::base;
910
use crate::MemFlags;
@@ -224,14 +225,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
224225
}
225226

226227
fn codegen_return_terminator(&mut self, mut bx: Bx) {
228+
// Call `va_end` if this is the definition of a C-variadic function.
227229
if self.fn_ty.c_variadic {
228-
match self.va_list_ref {
229-
Some(va_list) => {
230+
// The `VaList` "spoofed" argument is just after all the real arguments.
231+
let va_list_arg_idx = self.fn_ty.args.len();
232+
match self.locals[mir::Local::new(1 + va_list_arg_idx)] {
233+
LocalRef::Place(va_list) => {
230234
bx.va_end(va_list.llval);
231235
}
232-
None => {
233-
bug!("C-variadic function must have a `va_list_ref`");
234-
}
236+
_ => bug!("C-variadic function must have a `VaList` place"),
235237
}
236238
}
237239
if self.fn_ty.ret.layout.abi.is_uninhabited() {
@@ -242,15 +244,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
242244
return;
243245
}
244246
let llval = match self.fn_ty.ret.mode {
245-
PassMode::Ignore(IgnoreMode::Zst) | PassMode::Indirect(..) => {
247+
PassMode::Ignore | PassMode::Indirect(..) => {
246248
bx.ret_void();
247249
return;
248250
}
249251

250-
PassMode::Ignore(IgnoreMode::CVarArgs) => {
251-
bug!("C-variadic arguments should never be the return type");
252-
}
253-
254252
PassMode::Direct(_) | PassMode::Pair(..) => {
255253
let op =
256254
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> {
502500
return;
503501
}
504502

505-
// The "spoofed" `VaListImpl` added to a C-variadic functions signature
506-
// should not be included in the `extra_args` calculation.
507-
let extra_args_start_idx = sig.inputs().len() - if sig.c_variadic { 1 } else { 0 };
508-
let extra_args = &args[extra_args_start_idx..];
503+
let extra_args = &args[sig.inputs().len()..];
509504
let extra_args = extra_args.iter().map(|op_arg| {
510505
let op_ty = op_arg.ty(self.mir, bx.tcx());
511506
self.monomorphize(&op_ty)
@@ -691,26 +686,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
691686
(&args[..], None)
692687
};
693688

694-
// Useful determining if the current argument is the "spoofed" `VaListImpl`
695-
let last_arg_idx = if sig.inputs().is_empty() {
696-
None
697-
} else {
698-
Some(sig.inputs().len() - 1)
699-
};
700689
'make_args: for (i, arg) in first_args.iter().enumerate() {
701-
// If this is a C-variadic function the function signature contains
702-
// an "spoofed" `VaListImpl`. This argument is ignored, but we need to
703-
// populate it with a dummy operand so that the users real arguments
704-
// are not overwritten.
705-
let i = if sig.c_variadic && last_arg_idx.map(|x| i >= x).unwrap_or(false) {
706-
if i + 1 < fn_ty.args.len() {
707-
i + 1
708-
} else {
709-
break 'make_args
710-
}
711-
} else {
712-
i
713-
};
714690
let mut op = self.codegen_operand(&mut bx, arg);
715691

716692
if let (0, Some(ty::InstanceDef::Virtual(_, idx))) = (i, def) {

0 commit comments

Comments
 (0)