libsyntax/librustc: Allow calling variadic foreign functions.

[luqmana]

Fixes #2057.

Example:

#[no_std];

type c_char = u8;
type c_int = i32;
type size_t = uint;

extern {
    fn printf(format: *c_char, ...) -> c_int;
}

#[lang="fail_bounds_check"]
fn fail_bounds_check(_: *c_char, _: size_t, _: size_t, _: size_t) {}

#[start]
#[fixed_stack_segment]
fn main(_: int, _: **u8) -> int {
    unsafe {
        let msg = bytes!("Hello World!\n\0");
        printf(&msg[0]);

        let msg = bytes!("The answer:\t%d\n\0");
        printf(&msg[0], 42);

        let msg = bytes!("Null pointer:\t%p\n\0");
        printf(&msg[0], 0 as *c_char);

        let a: c_int = 45;
        let b: c_int = 468;
        let msg = bytes!("%d + %d = %d\n%d - %d = %d\n\0");
        printf(&msg[0], a, b, a + b, a, b, a - b);
    }

    0
}

Output:

Hello World!
The answer:     42
Null pointer:   (nil)
45 + 468 = 513
45 - 468 = -423

[huonw]

I feel like these ifs might be clearer worded as positives:

bump(rdr);
return if nextch(rdr) == '.' {
    bump(rdr);
    if nextch(rdr) == '.' {
        bump(rdr);
        token::DOTDOTDOT
    } else {
        token::DOTDOT
   }
} else {
    token::DOT
};

(at the very least, the inner most if is clearer as a postive, and factoring out the common bumps seems nicer.)

[alexcrichton]

Cool! This should definitely be discussed. Some thoughts:

• This looks like the signature is being allowed on all functions, not just extern ones?
• It seems like a very unsafe operation to call any variadic function, the types don't appear to be checked (of the variadic arguments).
• If you can define a variadic rust function, it doesn't look like you have the ability to access the variadic arguments?
• I agree that tests are necessary!

[luqmana]

@alexcrichton I just put the variadic flag on FnSig because it was easier than special casing it and it does make sense since I would say it being variadic is a part of its signature. Also, this doesn't parse for non-foreign functions so you can't define a variadic rust function. Notice I changed the parse_fn_decl in the parser to take a flag to indicate whether it should parse ... but that flag is only set for foreign fns in an extern block. This is only to allow interop with foreign variadic functions like printf. I don't think there are any plans to allow var args for rust functions.

[alexcrichton]

Ah excellent! I need to read more closely next time. Otherwise this looks good to me with some tests.

[nikomatsakis]

I'd like to review this too.

[nikomatsakis]

This is definitely wrong. This bit of code checks for subtypes and combines types, so just ignoring variadic means that you could combine variadic and non-variadic fn types indiscriminately. What you want to do is to require that both a.variadic and b.variadic agree, and then use a.variadic in the result. Something like:

if a.variadic != b.variadic { return Err(ty::terr_variadic_mismatch(expected_found(a.variadic, b.variadic)); }
...
Ok(FnSig {
...
variadic: a.variadic
})

[emberian]

I'm ok with having this for extern fns, but am very, very strongly opposed to adding this feature for anything other than foreign function declarations.

[Kimundi]

Does this need ... as a new token as opposed to ..?

[sanxiyn]

@Kimundi Not needed, but ... is what C uses and I am against gratuitous differences.

[nikomatsakis]

OK, @luqmana sorry for the confusion, some of my comments were a bit outdated by commit 2eeb83d. However, I've updated them as appropriate.

In general, this looks pretty good. I made a few comments inline about things that should be updated. but the biggest thing is that I think we need more and better tests.

First, we should test the type system parts better:

• The test that calls variadic functoins should not call printf but rather sprintf or something similar, so that we can test that the results look correct. Right now I have no idea.
• A test where a variadic function is called indirectly. This also tests the syntax being parsed. Something like:

fn foo(f: extern "C" fn(*c_char, ...)) { ... }
fn main() {
    foo(sprintf);
}

• Some tests in compile_fail where we try to use extern "C" fn(uint, ...) where extern "C" fn(uint) is expected and vice versa.

r+ with tests added and comments addressed.

[nikomatsakis]

@luqmana looking good!

I thought of one last set of tests I think we should have, which is to check that attempts to declare rust fns that use ... do not parse, e.g.

fn foo(x: uint, ...) { ... }

and especially:

extern "C" fn foo(uint, ...) { ... }

[pcwalton]

This is essential for Objective-C bindings. Awesome!