-
Notifications
You must be signed in to change notification settings - Fork 745
clang::Type::template_args return Option<TypeTemplateArgIterator> #175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me with that, thanks!
|
||
impl ExactSizeIterator for TypeTemplateArgIterator { | ||
fn len(&self) -> usize { | ||
if self.index < self.length { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.index can never be greater than self.length right? In that case, this if is not needed, since we'll always return the correct value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or an assert!(self.index <= self.length)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I have just a few stylistic nitpicks :)
/// template arguments. Otherwise, return None. | ||
pub fn num_template_args(&self) -> Option<u32> { | ||
pub fn template_args(&self) -> Option<TypeTemplateArgIterator> | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: {
on same line as the signature.
@@ -747,6 +741,36 @@ impl Type { | |||
} | |||
} | |||
|
|||
/// An iterator for a type's template arguments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: add a period at the end of the sentence.
let args = arg_types | ||
.filter(|t| t.kind() != CXType_Invalid) | ||
.map(|t| Item::from_ty_or_ref(t, None, None, ctx)) | ||
.collect::<Vec<_>>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So much nicer :)
let n = unsafe { clang_Type_getNumTemplateArguments(self.x) }; | ||
if n >= 0 { | ||
Some(n as u32) | ||
Some(TypeTemplateArgIterator { x: self.x, length: n as u32, index: 0 }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: put each field on its own line, like this:
Some(TypeTemplateArgIterator {
x: self.x,
length: n as u32,
index: 0,
})
f6a2003
to
c8c2bdb
Compare
@bors-servo r=fitzgen,emilio |
📌 Commit c8c2bdb has been approved by |
clang::Type::template_args return Option<TypeTemplateArgIterator> Fix #136 by providing a bound checked iterator replacing the interface.
Thanks again, this is awesome :) |
☀️ Test successful - status-travis |
Fix #136 by providing a bound checked iterator replacing the interface.