Skip to content

Add wave* and some time* functions #107

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
Jun 23, 2015

Conversation

jminer
Copy link
Contributor

@jminer jminer commented Jun 11, 2015

Is this a good way to do unions (MMTIME)? It is similar to how bindgen translated the union.

pub const WAVERR_LASTERROR: MMRESULT = WAVERR_BASE + 3;
#[repr(C)] #[derive(Clone, Copy, Debug)]
pub struct HWAVEIN__;
pub type HWAVEIN = *mut HWAVEIN__;
Copy link
Owner

Choose a reason for hiding this comment

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

Use DECLARE_HANDLE!(HWAVEIN, HWAVEIN__);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed HWAVEIN and HWAVEOUT.

@retep998
Copy link
Owner

I'm not sure how I want to deal with untagged unions. Ideally Rust would get something like rust-lang/rfcs#724 which would makes things way easier. I'm not a fan of those methods returning a *mut instead of a &mut, plus if all you have is an immutable & then you can't access those union members at all.

@jminer jminer force-pushed the winmm-time-and-wave branch from 336205f to a597829 Compare June 11, 2015 06:32
@jminer
Copy link
Contributor Author

jminer commented Jun 11, 2015

Yeah, I wish there were language support for C unions. What do you think about having mut and non-mut versions of each member?

    pub unsafe fn ms<'a>(&'a self) -> &'a ::DWORD {
        ::std::mem::transmute(&self.data)
    }
    pub unsafe fn ms_mut<'a>(&'a mut self) -> &'a mut ::DWORD {
        ::std::mem::transmute(&mut self.data)
    }

The methods should probably have an inline attribute too.

@retep998
Copy link
Owner

There shouldn't be any need to explicitly specify the lifetime in this case as lifetime inference should be enough. Otherwise that approach should work.

@jminer jminer force-pushed the winmm-time-and-wave branch from a597829 to 6679dc0 Compare June 14, 2015 02:41
@jminer
Copy link
Contributor Author

jminer commented Jun 14, 2015

Right, thanks. I was thinking that transmute doesn't preserve the lifetime, but I forgot that's irrelevant as the body of a method doesn't affect lifetime elision. I updated the commit.

@jminer jminer force-pushed the winmm-time-and-wave branch from 6679dc0 to 1c0077c Compare June 14, 2015 09:15
@retep998
Copy link
Owner

Do you think you could update this PR to try to make use of the UNION! macro I wrote?

@jminer jminer force-pushed the winmm-time-and-wave branch from 1c0077c to 4112ad1 Compare June 20, 2015 08:50
@jminer
Copy link
Contributor Author

jminer commented Jun 20, 2015

Sure, it's updated. I think using a macro is a good improvement.

@retep998 retep998 merged commit 4112ad1 into retep998:master Jun 23, 2015
@jminer jminer deleted the winmm-time-and-wave branch June 23, 2015 06:14
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.

2 participants