Skip to content
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

Some Questions #62

Closed
Phaiax opened this issue Mar 11, 2016 · 2 comments
Closed

Some Questions #62

Phaiax opened this issue Mar 11, 2016 · 2 comments

Comments

@Phaiax
Copy link
Contributor

Phaiax commented Mar 11, 2016

Hey

I looked a little bit around at the source code and thought about all that unsafe pointer usage.

Why is that and why not using slices that also carry pointer and length information? Is this a port of some C library? Performance reasons?

Secondly, I tried to understand how divmod works, but no chance by just looking at it. Can you give me a hint how the algorithm is called or where to search for more information?

I'm more a of rust beginner, but I maybe could contribute some things. I thought of three things:

  1. Converting the /* */ comments into proper /// comments, so that they get compiled to the docs and the gh-pages.
  2. [feature request] Convert from/to bytes #6 From byte conversion
  3. Change use of i32 to u32 where appropriate #22 (and using usize for all sizes)

@Aatch Do you approve? Do you have time to review PRs?

@Aatch
Copy link
Owner

Aatch commented Mar 11, 2016

I looked a little bit around at the source code and thought about all that unsafe pointer usage.

Why is that and why not using slices that also carry pointer and length information? Is this a port of some C library? Performance reasons?

Performance is the main reason, but it's due to aliasing, rather than anything else. Basically, many algorithms work perfectly well with the input and output being the same place, or overlapping as long as they overlap in the right way. That doesn't work for regular borrowed pointers, I can't have a &[u32] and a &mut [u32] pointing to the same place or overlapping. This is actually quite important for performance to avoid having to copy data unecessarily.

Secondly, I tried to understand how divmod works, but no chance by just looking at it. Can you give me a hint how the algorithm is called or where to search for more information?

I'll admit the details are fuzzy, it's a complicated algorithm that I struggled to get working correctly. Well, there's actually several algorithms, since it handles certain cases differently, but I digress. I'll look over it and see if I can write some more documentation on the actual function of the algorithm.

The comments are mostly /** */-comments, which I prefer to ///. They're also doc comments, so they end up in the generated docs. If there are some doc-comments that are /* */-style, then they should be converted to /** */ ones.

For #6, that's just something I haven't got around to yet. If you want to take a stab at it, go for it. I lay out roughly what needs to be done there, but fell free to ask questions on that issue if you need to.

@Phaiax
Copy link
Contributor Author

Phaiax commented Mar 12, 2016

Note: During testing if I am doing unsafe the correct way, i figured:

        let mut raw = vec![14u8, 252, 125, 13,  15, 26,  26, 231,
                   121,  63,  53,  255, 24, 152, 199, 1];
        unsafe {
            // create slice as some kind of u16 view into the vec.
            // It is safe, since as_u16 is only a slice and has no
            // destructor and will not free any memory (except for
            // its stack fat pointer)
            // Safe as long as raw is not used here.
            let as_u16 : &[u16] = std::slice::from_raw_parts(
                                                raw.as_ptr() as *const u16,
                                                raw.len() / 2);
            assert_eq!(as_u16[0], 64526);
            assert_eq!(as_u16.len(), 8);
            raw[0] = 12;
            assert_eq!(as_u16[0], 64526); // fails. concurrent changes
        }

Edited: Okay. Anything with mem::transmute is probably the way to hell. (The len will not be updated)

@rozbb rozbb closed this as completed Jun 18, 2018
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

No branches or pull requests

3 participants