Skip to content

Commit

Permalink
Auto merge of #57760 - dlrobertson:varargs1, r=alexreg
Browse files Browse the repository at this point in the history
Support defining C compatible variadic functions

## Summary

Add support for defining C compatible variadic functions in unsafe rust with
`extern "C"` according to [RFC 2137].

## Details

### Parsing
When parsing a user defined function that is `unsafe` and `extern "C"` allow
variadic signatures and inject a "spoofed" `VaList` in the new functions
signature. This allows the user to interact with the variadic arguments via a
`VaList` instead of manually using `va_start` and `va_end` (See [RFC 2137] for
details).

### Codegen

When running codegen for a variadic function, remove the "spoofed" `VaList`
from the function signature and inject `va_start` when the arg local
references are created for the function and `va_end` on return.

## TODO

 - [x] Get feedback on injecting `va_start/va_end` in MIR vs codegen
 - [x] Properly inject `va_end` - It seems like it should be possible to inject
       `va_end` on the `TerminatorKind::Return`. I just need to figure out how
       to get the `LocalRef` here.
 - [x] Properly call Rust defined C variadic functions in Rust - The spoofed
       `VaList` causes problems here.

Related to: #44930

r? @ghost

[RFC 2137]: https://github.com/rust-lang/rfcs/blob/master/text/2137-variadic.md
  • Loading branch information
bors committed Feb 28, 2019
2 parents 190feb6 + f7dd438 commit 1999a22
Show file tree
Hide file tree
Showing 78 changed files with 1,778 additions and 902 deletions.
24 changes: 24 additions & 0 deletions src/doc/unstable-book/src/language-features/c-variadic.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# `c_variadic`

The tracking issue for this feature is: [#44930]

[#44930]: https://github.com/rust-lang/rust/issues/44930

------------------------

The `c_variadic` language feature enables C-variadic functions to be
defined in Rust. The may be called both from within Rust and via FFI.

## Examples

```rust
#![feature(c_variadic)]

pub unsafe extern "C" fn add(n: usize, mut args: ...) -> usize {
let mut sum = 0;
for _ in 0..n {
sum += args.arg::<usize>();
}
sum
}
```
26 changes: 26 additions & 0 deletions src/doc/unstable-book/src/library-features/c-variadic.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# `c_variadic`

The tracking issue for this feature is: [#44930]

[#44930]: https://github.com/rust-lang/rust/issues/44930

------------------------

The `c_variadic` library feature exposes the `VaList` structure,
Rust's analogue of C's `va_list` type.

## Examples

```rust
#![feature(c_variadic)]

use std::ffi::VaList;

pub unsafe extern "C" fn vadd(n: usize, mut args: VaList) -> usize {
let mut sum = 0;
for _ in 0..n {
sum += args.arg::<usize>();
}
sum
}
```
3 changes: 3 additions & 0 deletions src/librustc/hir/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,9 @@ 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
44 changes: 25 additions & 19 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ const HIR_ID_COUNTER_LOCKED: u32 = 0xFFFFFFFF;
pub struct LoweringContext<'a> {
crate_root: Option<&'static str>,

// Used to assign ids to HIR nodes that do not directly correspond to an AST node.
/// Used to assign ids to HIR nodes that do not directly correspond to an AST node.
sess: &'a Session,

cstore: &'a dyn CrateStore,
Expand Down Expand Up @@ -107,25 +107,25 @@ pub struct LoweringContext<'a> {
/// written at all (e.g., `&T` or `std::cell::Ref<T>`).
anonymous_lifetime_mode: AnonymousLifetimeMode,

// Used to create lifetime definitions from in-band lifetime usages.
// e.g., `fn foo(x: &'x u8) -> &'x u8` to `fn foo<'x>(x: &'x u8) -> &'x u8`
// When a named lifetime is encountered in a function or impl header and
// has not been defined
// (i.e., it doesn't appear in the in_scope_lifetimes list), it is added
// to this list. The results of this list are then added to the list of
// lifetime definitions in the corresponding impl or function generics.
/// Used to create lifetime definitions from in-band lifetime usages.
/// e.g., `fn foo(x: &'x u8) -> &'x u8` to `fn foo<'x>(x: &'x u8) -> &'x u8`
/// When a named lifetime is encountered in a function or impl header and
/// has not been defined
/// (i.e., it doesn't appear in the in_scope_lifetimes list), it is added
/// to this list. The results of this list are then added to the list of
/// lifetime definitions in the corresponding impl or function generics.
lifetimes_to_define: Vec<(Span, ParamName)>,

// Whether or not in-band lifetimes are being collected. This is used to
// indicate whether or not we're in a place where new lifetimes will result
// in in-band lifetime definitions, such a function or an impl header,
// including implicit lifetimes from `impl_header_lifetime_elision`.
/// Whether or not in-band lifetimes are being collected. This is used to
/// indicate whether or not we're in a place where new lifetimes will result
/// in in-band lifetime definitions, such a function or an impl header,
/// including implicit lifetimes from `impl_header_lifetime_elision`.
is_collecting_in_band_lifetimes: bool,

// Currently in-scope lifetimes defined in impl headers, fn headers, or HRTB.
// When `is_collectin_in_band_lifetimes` is true, each lifetime is checked
// against this list to see if it is already in-scope, or if a definition
// needs to be created for it.
/// Currently in-scope lifetimes defined in impl headers, fn headers, or HRTB.
/// When `is_collectin_in_band_lifetimes` is true, each lifetime is checked
/// against this list to see if it is already in-scope, or if a definition
/// needs to be created for it.
in_scope_lifetimes: Vec<Ident>,

current_module: NodeId,
Expand Down Expand Up @@ -954,7 +954,7 @@ impl<'a> LoweringContext<'a> {
let decl = FnDecl {
inputs: vec![],
output,
variadic: false
c_variadic: false
};
let body_id = self.record_body(body_expr, Some(&decl));
self.is_generator = prev_is_generator;
Expand Down Expand Up @@ -1345,6 +1345,12 @@ impl<'a> LoweringContext<'a> {
}
}
TyKind::Mac(_) => panic!("TyMac should have been expanded by now."),
TyKind::CVarArgs => {
// Create the implicit lifetime of the "spoofed" `VaList`.
let span = self.sess.source_map().next_point(t.span.shrink_to_lo());
let lt = self.new_implicit_lifetime(span);
hir::TyKind::CVarArgs(lt)
},
};

let LoweredNodeId { node_id: _, hir_id } = self.lower_node_id(t.id);
Expand Down Expand Up @@ -2112,7 +2118,7 @@ impl<'a> LoweringContext<'a> {
P(hir::FnDecl {
inputs,
output,
variadic: decl.variadic,
c_variadic: decl.c_variadic,
implicit_self: decl.inputs.get(0).map_or(
hir::ImplicitSelfKind::None,
|arg| {
Expand Down Expand Up @@ -3967,7 +3973,7 @@ impl<'a> LoweringContext<'a> {
let outer_decl = FnDecl {
inputs: decl.inputs.clone(),
output: FunctionRetTy::Default(fn_decl_span),
variadic: false,
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
5 changes: 4 additions & 1 deletion src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1829,6 +1829,9 @@ pub enum TyKind {
Infer,
/// Placeholder for a type that has failed to be defined.
Err,
/// Placeholder for C-variadic arguments. We "spoof" the `VaList` created
/// from the variadic arguments. This type is only valid up to typeck.
CVarArgs(Lifetime),
}

#[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
Expand Down Expand Up @@ -1865,7 +1868,7 @@ pub struct Arg {
pub struct FnDecl {
pub inputs: HirVec<Ty>,
pub output: FunctionRetTy,
pub variadic: bool,
pub c_variadic: bool,
/// Does the function have an implicit self?
pub implicit_self: ImplicitSelfKind,
}
Expand Down
5 changes: 4 additions & 1 deletion src/librustc/hir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,9 @@ impl<'a> State<'a> {
self.s.word("/*ERROR*/")?;
self.pclose()?;
}
hir::TyKind::CVarArgs(_) => {
self.s.word("...")?;
}
}
self.end()
}
Expand Down Expand Up @@ -2004,7 +2007,7 @@ impl<'a> State<'a> {
s.print_type(ty)?;
s.end()
})?;
if decl.variadic {
if decl.c_variadic {
self.s.word(", ...")?;
}
self.pclose()?;
Expand Down
5 changes: 3 additions & 2 deletions src/librustc/ich/impls_hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,13 +361,14 @@ impl_stable_hash_for!(enum hir::TyKind {
TraitObject(trait_refs, lifetime),
Typeof(body_id),
Err,
Infer
Infer,
CVarArgs(lt),
});

impl_stable_hash_for!(struct hir::FnDecl {
inputs,
output,
variadic,
c_variadic,
implicit_self
});

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ impl_stable_hash_for!(struct ty::GenSig<'tcx> {

impl_stable_hash_for!(struct ty::FnSig<'tcx> {
inputs_and_output,
variadic,
c_variadic,
unsafety,
abi
});
Expand Down
31 changes: 21 additions & 10 deletions src/librustc/middle/resolve_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,13 @@ 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 @@ -2225,18 +2232,22 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
if let hir::TyKind::BareFn(_) = ty.node {
self.outer_index.shift_in(1);
}
if let hir::TyKind::TraitObject(ref bounds, ref lifetime) = ty.node {
for bound in bounds {
self.visit_poly_trait_ref(bound, hir::TraitBoundModifier::None);
}
match ty.node {
hir::TyKind::TraitObject(ref bounds, ref lifetime) => {
for bound in bounds {
self.visit_poly_trait_ref(bound, hir::TraitBoundModifier::None);
}

// Stay on the safe side and don't include the object
// lifetime default (which may not end up being used).
if !lifetime.is_elided() {
self.visit_lifetime(lifetime);
// Stay on the safe side and don't include the object
// lifetime default (which may not end up being used).
if !lifetime.is_elided() {
self.visit_lifetime(lifetime);
}
}
hir::TyKind::CVarArgs(_) => {}
_ => {
intravisit::walk_ty(self, ty);
}
} else {
intravisit::walk_ty(self, ty);
}
if let hir::TyKind::BareFn(_) = ty.node {
self.outer_index.shift_out(1);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1944,7 +1944,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
if let ty::FnSig {
unsafety: hir::Unsafety::Normal,
abi: Abi::Rust,
variadic: false,
c_variadic: false,
..
} = self_ty.fn_sig(self.tcx()).skip_binder()
{
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2453,7 +2453,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
self.mk_fn_sig(
params_iter,
s.output(),
s.variadic,
s.c_variadic,
hir::Unsafety::Normal,
abi::Abi::Rust,
)
Expand Down Expand Up @@ -2779,7 +2779,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
pub fn mk_fn_sig<I>(self,
inputs: I,
output: I::Item,
variadic: bool,
c_variadic: bool,
unsafety: hir::Unsafety,
abi: abi::Abi)
-> <I::Item as InternIteratorElement<Ty<'tcx>, ty::FnSig<'tcx>>>::Output
Expand All @@ -2788,7 +2788,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
{
inputs.chain(iter::once(output)).intern_with(|xs| ty::FnSig {
inputs_and_output: self.intern_type_list(xs),
variadic, unsafety, abi
c_variadic, unsafety, abi
})
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl<'a, 'tcx> Instance<'tcx> {
sig.map_bound(|sig| tcx.mk_fn_sig(
iter::once(*env_ty.skip_binder()).chain(sig.inputs().iter().cloned()),
sig.output(),
sig.variadic,
sig.c_variadic,
sig.unsafety,
sig.abi
))
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/ty/relate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,9 @@ impl<'tcx> Relate<'tcx> for ty::FnSig<'tcx> {
{
let tcx = relation.tcx();

if a.variadic != b.variadic {
if a.c_variadic != b.c_variadic {
return Err(TypeError::VariadicMismatch(
expected_found(relation, &a.variadic, &b.variadic)));
expected_found(relation, &a.c_variadic, &b.c_variadic)));
}
let unsafety = relation.relate(&a.unsafety, &b.unsafety)?;
let abi = relation.relate(&a.abi, &b.abi)?;
Expand All @@ -171,7 +171,7 @@ impl<'tcx> Relate<'tcx> for ty::FnSig<'tcx> {
});
Ok(ty::FnSig {
inputs_and_output: tcx.mk_type_list(inputs_and_output)?,
variadic: a.variadic,
c_variadic: a.c_variadic,
unsafety,
abi,
})
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ impl<'a, 'tcx> Lift<'tcx> for ty::FnSig<'a> {
tcx.lift(&self.inputs_and_output).map(|x| {
ty::FnSig {
inputs_and_output: x,
variadic: self.variadic,
c_variadic: self.c_variadic,
unsafety: self.unsafety,
abi: self.abi,
}
Expand Down Expand Up @@ -832,7 +832,7 @@ BraceStructTypeFoldableImpl! {

BraceStructTypeFoldableImpl! {
impl<'tcx> TypeFoldable<'tcx> for ty::FnSig<'tcx> {
inputs_and_output, variadic, unsafety, abi
inputs_and_output, c_variadic, unsafety, abi
}
}

Expand Down
12 changes: 6 additions & 6 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -977,13 +977,13 @@ impl<'tcx> PolyGenSig<'tcx> {
/// Signature of a function type, which I have arbitrarily
/// decided to use to refer to the input/output types.
///
/// - `inputs` is the list of arguments and their modes.
/// - `output` is the return type.
/// - `variadic` indicates whether this is a variadic function. (only true for foreign fns)
/// - `inputs`: is the list of arguments and their modes.
/// - `output`: is the return type.
/// - `c_variadic`: indicates whether this is a C-variadic function.
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
pub struct FnSig<'tcx> {
pub inputs_and_output: &'tcx List<Ty<'tcx>>,
pub variadic: bool,
pub c_variadic: bool,
pub unsafety: hir::Unsafety,
pub abi: abi::Abi,
}
Expand Down Expand Up @@ -1016,8 +1016,8 @@ impl<'tcx> PolyFnSig<'tcx> {
pub fn output(&self) -> ty::Binder<Ty<'tcx>> {
self.map_bound_ref(|fn_sig| fn_sig.output())
}
pub fn variadic(&self) -> bool {
self.skip_binder().variadic
pub fn c_variadic(&self) -> bool {
self.skip_binder().c_variadic
}
pub fn unsafety(&self) -> hir::Unsafety {
self.skip_binder().unsafety
Expand Down

0 comments on commit 1999a22

Please sign in to comment.