Skip to content

change return value of Type::pointee_type() to Option<Type> #203

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

Merged
merged 1 commit into from
Nov 5, 2016
Merged

change return value of Type::pointee_type() to Option<Type> #203

merged 1 commit into from
Nov 5, 2016

Conversation

ajnirp
Copy link

@ajnirp ajnirp commented Nov 4, 2016

Fixes #137

@highfive
Copy link

highfive commented Nov 4, 2016

warning Warning warning

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

@wafflespeanut
Copy link
Contributor

Please claim the relevant issue before working on it, so that we can reduce duplicate efforts :)

Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, r=me with that :)

Type {
x: clang_getPointeeType(self.x),
pub fn pointee_type(&self) -> Option<Type> {
match self.x.kind {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you use self.kind()? not a big deal in this case, but better to be consistent.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realise we had that method, thank you!

@wafflespeanut
Copy link
Contributor

wafflespeanut commented Nov 4, 2016

Never mind. I was wrong.

CXType_MemberPointer => {
unsafe {
Some(Type { x: clang_getPointeeType(self.x) })
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, for some reason my other comment was lost.

Can you assert the type is valid here?

let ret = Type {
    x: unsafe { clang_getPointeeType(self.x) },
};
debug_assert!(ret.kind() != CXType_Invalid);
Some(ret)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do

@emilio
Copy link
Contributor

emilio commented Nov 5, 2016

@bors-servo r+

Thanks!

@bors-servo
Copy link

📌 Commit 45dd2ae has been approved by emilio

@bors-servo
Copy link

⌛ Testing commit 45dd2ae with merge 823abd5...

bors-servo pushed a commit that referenced this pull request Nov 5, 2016
change return value of Type::pointee_type() to Option<Type>

Fixes #137
@bors-servo
Copy link

☀️ Test successful - status-travis

@bors-servo bors-servo merged commit 45dd2ae into rust-lang:master Nov 5, 2016
@ajnirp ajnirp deleted the 137-pointee-type branch November 6, 2016 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants