Skip to content

Vec::as_ptr for empty vectors is undocumented #39625

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

Closed
nbaksalyar opened this issue Feb 7, 2017 · 11 comments
Closed

Vec::as_ptr for empty vectors is undocumented #39625

nbaksalyar opened this issue Feb 7, 2017 · 11 comments
Assignees
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools P-medium Medium priority

Comments

@nbaksalyar
Copy link
Contributor

Vec::as_ptr returns a pointer to address 0x1 for empty vectors. As it seems, this is an internal implementation detail for RawVec and allocation.

This behavior is unexpected and not documented. In certain scenarios it might cause a segmentation fault (e.g. due to failed checks for a null pointer when dealing with FFI).

@steveklabnik
Copy link
Member

Yes, we should document this for sure.

@Stebalien
Copy link
Contributor

due to failed checks for a null pointer when dealing with FFI

FYI, this is precisely why rust uses uses 0x1 for zero-sized things instead of 0x0 (null); it does this to distinguish between "nothing" and "something empty/zero sized".

@frewsxcv
Copy link
Member

frewsxcv commented Feb 8, 2017

Would it be accurate to say that it returning 0x1 is an implementation detail?

@steveklabnik
Copy link
Member

Yes, I would say something like "the pointer value of an empty vector is meaningless", but uh, worded better. Not after midnight 😉

@Stebalien
Copy link
Contributor

Stebalien commented Feb 8, 2017 via email

@Cobrand
Copy link
Contributor

Cobrand commented Feb 8, 2017

This not only happen for Vec, but for other things of 0 size allocated on the heap as well, (namely Box<()>). While you might say it's pointless to do Box::new(()), it can still happen and it should be documented.

I suspect it works the same way for Rc<()> and other things like that, so a chapter in the nomicon would be great I think. At the very least, say something in Box::into_raw_ptr() about 0 size structs, and add a link from Vec::as_ptr() to this part.

@frewsxcv
Copy link
Member

frewsxcv commented Feb 12, 2017

Opened a pull request here: #39757.

In the pull request, I used the term 'meaningless' @steveklabnik mentioned above. I don't think it's a great term, but I can't think of anything better right now.

Please please share better ideas!

frewsxcv added a commit to frewsxcv/rust that referenced this issue Feb 12, 2017
@nagisa
Copy link
Member

nagisa commented Feb 13, 2017

There are multiple aspects to this issue:

  1. You are not supposed to dereference pointer of 0-capacity vector anyway. It makes no difference whether the pointer is 0, 1 or 0xFRESHVEGGIES.
  2. 0-sized pointer dereference (including pointer returned by Vec<ZeroSized>::as_ptr) is no-op and dereferencing 0, 1 or 0xFRESHVEGGIES are all equally valid (i.e. no-op) in current implementation, although not specified.
  3. These pointers are not meaningless. You need to pass the same pointers back into from_raw and friends to insure the correct behaviour of data structures and smart pointers afterwards.

In that sense the only real change that’s possible in documentation is

This function may return non-null non-dereferenceable pointers.

@steveklabnik steveklabnik added A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools and removed A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools labels Mar 10, 2017
@steveklabnik
Copy link
Member

docs team triage: p-medium, still need to sort out the exact details, thanks for the above comment @nagisa

@steveklabnik steveklabnik added the P-medium Medium priority label Mar 15, 2017
@steveklabnik
Copy link
Member

I am going to send in a PR for this based on @nagisa 's wording.

@steveklabnik
Copy link
Member

I have completely turned 180 on this. There are too many pitfalls, as @nagisa says. I don't think that sentence really adds enough to actually understand what's going on here. Sadly, I think this is the best we can do. Giving this a close now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools P-medium Medium priority
Projects
None yet
Development

No branches or pull requests

6 participants