Make clang::Cursor::specialized return an Option #191
Conversation
| @@ -187,7 +187,7 @@ impl Cursor { | |||
|
|
|||
| /// Is the referent a template specialization? | |||
| pub fn is_template(&self) -> bool { | |||
| self.specialized().is_valid() | |||
| self.specialized().map_or(false, |c| c.is_valid()) | |||
There was a problem hiding this comment.
This can be self.specialized().is_some() now, given we check if the cursor is valid in specialized.
There was a problem hiding this comment.
Checking is_some() will only work after my requested changes to the is_valid() checking. This is a good insight, though.
There was a problem hiding this comment.
Whoops, I misread the implementation, but yeah :)
| pub fn specialized(&self) -> Cursor { | ||
| pub fn specialized(&self) -> Option<Cursor> { | ||
| if !self.is_valid() { | ||
| return None; |
There was a problem hiding this comment.
Rather than checking if self.is_valid(), we should make the FFI call to clang, and then check if the result of that FFI call .is_valid() and return None/Some based on that.
Does that make sense?
There was a problem hiding this comment.
Oops, I misread the issue description. I thought the concern was that the existing Cursor was valid, not that the resulting one was valid. Thanks for the correction!
| @@ -187,7 +187,7 @@ impl Cursor { | |||
|
|
|||
| /// Is the referent a template specialization? | |||
| pub fn is_template(&self) -> bool { | |||
| self.specialized().is_valid() | |||
| self.specialized().map_or(false, |c| c.is_valid()) | |||
| x: clang_getSpecializedCursorTemplate(self.x), | ||
| }) | ||
| let clang_specialized = clang_getSpecializedCursorTemplate(self.x); | ||
| if clang_isInvalid(clang_getCursorKind(clang_specialized)) == 0 { |
There was a problem hiding this comment.
Maybe:
let ret = Cursor {
x: clang_getSpecializedCursorTemplate(self.x),
};
if ret.is_valid() { Some(ret) } else { None }There was a problem hiding this comment.
I thought about doing that, but it made me feel uncomfortable to have a Cursor object that didn't necessarily correspond to a real cursor. Also, I misunderstood what @fitzgen said in his comment above. But I agree that it's better to reuse the definition of is_valid. Let me make that change.
Thanks @emilio for the suggestion.
|
@bors-servo r+ |
|
📌 Commit 94136d2 has been approved by |
|
Thanks for doing this! :) |
|
☀️ Test successful - status-travis |
Fixes #122.
pair=@Natim