clang::Cursor::args should return an Option<Vec<Cursor>>

[Natim]

Attempt to fix #130

[Natim]

This is an option, do you think it would be better than not using num_args?

[emilio]

We use an iterator pattern somewhere else (for comment children and attributes), so something like that can also work, and it'd be better IMO.

If you want to give a crack at it, that'd be awesome.

[glasserc]

Could you elaborate on this, please? Does this mean we should be returning an iterator instead of a Vec? Or that there's some sort of iterator that we can "build on" to write this method?

[emilio]

Returning an iterator is definitely preferable, since it's more efficient. You can implement something like TypeTemplateArgIterator (in that same file). That being said, only a Vec is also fine if you prefer :)

[Natim]

Is expect the right way to handle this there?
Can we rely on the fact that cursor.args() is now None in case it is not a function or method?

[emilio]

So I'd expect we might want to test this with stuff like variadic functions, but if all tests are passing seems like we can keep it, and then if it fails in some weird situation returning an empty vector in the case it's none to preser behavior is a really easy change.

[emilio]

Actually, could you write a test with a template and a variadic function here? Otherwise I think yeah, we should be pretty confident.

[fitzgen]

@Natim, everything going OK? Have any questions we can help answer or clarifications we can make?

[Natim]

I think I understood we should write a test for that. I hope to find some bandwith do to it with @glasserc later this week.

[bors-servo]

☔ The latest upstream changes (presumably #204) made this pull request unmergeable. Please resolve the merge conflicts.

[Natim]

We tried to work on a test, writing this hpp file:

template <typename T>
class VariadicFunctionObject {
public:
    int add_em_up(T count,...);
};

And hoping to get this RS file:

/* automatically generated by rust-bindgen */


#![allow(non_snake_case)]


#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct VariadicFunctionObject<T> {
    pub _address: u8,
    pub _phantom_0: ::std::marker::PhantomData<T>,
    #[inline]
    pub unsafe fn add_em_up(&mut self) {
        RealAbstractionWithTonsOfMethods_bar1(&mut *self)
    }
}

But instead we get:

/* automatically generated by rust-bindgen */


#![allow(non_snake_case)]


#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct VariadicFunctionObject<T> {
    pub _address: u8,
    pub _phantom_0: ::std::marker::PhantomData<T>,
}

Even without our patch we get the last one so I guess rust-bindgen is not handling variadic function yet.

Can you provide some guidance there?

[emilio]

Yeah, I don't think it would be possible to autogenerate those in rust, mainly because those functions don't always have symbols until they're instantiated. In general any method for a template class can't be automatically generated.

We could generate those for specializations though, but specializations should be in rust too.

[glasserc]

Hmm, so then is this what you meant by "could you write a test with a template and a variadic function here?" Or should the test have the a template and a variadic function but not one inside the other?

[emilio]

Yeah, you should be able to add a test with that, and ensure we're not crashing, the output doesn't need to change :)

[Natim]

So we can just add the two file in the comment I made?

[emilio]

Yup! That'd be fine :)

[emilio]

Sorry for being confusing, totally my fault :(

[Natim]

No worries, let us know what you think is the next step then 👍

[emilio]

Looks good!

@bors-servo r+

[bors-servo]

📌 Commit 3f0f868 has been approved by emilio

[bors-servo]

⌛ Testing commit 3f0f868 with merge 1a8a2ac...

[bors-servo]

☀️ Test successful - status-travis