Added checks for clang::Cursor::template_arg_value

[nikhilshagri]

Fixes #134.
r? @fitzgen

[highfive]

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @fitzgen (or someone else) soon.

[highfive]

Warning

• These commits modify unsafe code. Please review it carefully!

[emilio]

Is there any caller in this function? If not, let's rip it out.

[fitzgen]

Thanks @cynicaldevil!

If we aren't using this method anywhere, then I agree with @emilio that the best thing to do is to remove the method (sorry for not catching this when filing the issue!). Would you like to morph this PR into the method's removal?

That said, I've left a couple comments for your benefit.

I would probably write this code in a way such that the bounds checking isn't "distant" from the FFI call, like this:

pub fn template_arg_value(&self, i: c_int) -> Option<c_longlong> {
    match self.num_template_args() {
        // Fold bounds check into match arm.
        Some(n) if 0 <= i && i < n => {
            // Ok, it is safe to make the FFI call.
            Some(unsafe {
                clang_Cursor_getTemplateArgumentValue(self.x, i as c_uint)
            })
        _ => None,
    }
}

One could also use https://doc.rust-lang.org/nightly/std/option/enum.Option.html#method.map or https://doc.rust-lang.org/nightly/std/option/enum.Option.html#method.and_then here instead of a match, if one preferred.

[fitzgen]

In general, using if is usually better than match for boolean values:

if within_bounds {
    ...
} else {
    ...
}

[nikhilshagri]

@fitzgen I've removed the method and updated the PR. Think I'll pick up another issue to work on as well, since this one turned out to be really trivial.

[bors-servo]

☀️ Test successful - status-travis

[wafflespeanut]

Could you also remove the c_longlong imported at the top? It's causing the warnings (which we don't allow).

[nikhilshagri]

@wafflespeanut I had forgotten to to run the build step after removing the method. All good now!

@cynicaldevil has made the changes :)

[wafflespeanut]

@bors-servo r=emilio

[bors-servo]

📌 Commit 0e63240 has been approved by emilio

[bors-servo]

⌛ Testing commit 0e63240 with merge 09e6fe4...