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

Add is_unique(), try_unwrap(), get_mut() to alloc::rc #16101

Merged
merged 1 commit into from
Aug 1, 2014

Conversation

lilyball
Copy link
Contributor

Add a few new free functions to alloc::rc for manipulating
uniquely-owned Rc values. is_unique() can be used to test if the Rc is
uniquely-owned, try_unwrap() can remove the value from a uniquely-owned
Rc, and get_mut() can return a &mut for a uniquely-owned Rc.

These are all free functions, because smart pointers should avoid having
methods when possible. They can't be static methods because UFCS will
remove that distinction. I think we should probably change downgrade()
and make_unique() into free functions as well, but that's out of scope.

@emberian
Copy link
Member

I think this is useful and want it. PR looks good, will let someone else r+

/// *Rc.try_get_mut(&mut x).unwrap() = 4u;
/// assert_eq!(*x, 4u);
/// let _y = x.clone();
/// assert!(Rc.try_get_mut(&mut x).is_none());
Copy link
Member

Choose a reason for hiding this comment

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

This and the above example use Rc.foo which probably needs to get re-jiggered to rc::foo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, I guess I never ran the doc tests after changing to free functions.

Copy link
Member

Choose a reason for hiding this comment

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

@aturon Possibly doesn't make a lot of sense out of context:
gfx-rs/gfx#144 (comment)

Essentially you want an affine handle type that, for performance reasons,
can't really have a useful destructor, and the only way to share it is
behind Rc (or another shared container), but in the interest of not
having referring to values that no longer exist, removing the value from
the container requires the handle by-value, but you can't get that without
try_unwrap. It moves the failure into the user who is sharing the handle,
rather than having a handle which refers to a nonexistent value.

On Wed, Jul 30, 2014 at 10:04 AM, Kevin Ballard [email protected]
wrote:

In src/liballoc/rc.rs:

  • }
    +}

+/// Returns a mutable reference to the contained value if the Rc has
+/// unique ownership.
+///
+/// Returns None if the Rc does not have unique ownership.
+///
+/// # Example:
+///
+/// ```
+/// let mut x = Rc::new(3u);
+/// _Rc.try_get_mut(&mut x).unwrap() = 4u;
+/// assert_eq!(_x, 4u);
+/// let _y = x.clone();
+/// assert!(Rc.try_get_mut(&mut x).is_none());

You're right, I guess I never ran the doc tests after changing to free
functions.


Reply to this email directly or view it on GitHub
https://github.com/rust-lang/rust/pull/16101/files#r15597313.

http:https://octayn.net/

Copy link
Member

Choose a reason for hiding this comment

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

(The performance reasons being size, it would make the handle 4x larger, 4
bytes vs 16, https://gist.github.com/cmr/d7dca23f0d888ac5d19d, and these
handles are how users refer to almost every object in the API)

On Wed, Jul 30, 2014 at 11:29 AM, Corey Richardson [email protected] wrote:

@aturon Possibly doesn't make a lot of sense out of context:
gfx-rs/gfx#144 (comment)

Essentially you want an affine handle type that, for performance reasons,
can't really have a useful destructor, and the only way to share it is
behind Rc (or another shared container), but in the interest of not
having referring to values that no longer exist, removing the value from
the container requires the handle by-value, but you can't get that without
try_unwrap. It moves the failure into the user who is sharing the handle,
rather than having a handle which refers to a nonexistent value.

On Wed, Jul 30, 2014 at 10:04 AM, Kevin Ballard [email protected]
wrote:

In src/liballoc/rc.rs:

  • }
    +}

+/// Returns a mutable reference to the contained value if the Rc has
+/// unique ownership.
+///
+/// Returns None if the Rc does not have unique ownership.
+///
+/// # Example:
+///
+/// ```
+/// let mut x = Rc::new(3u);
+/// _Rc.try_get_mut(&mut x).unwrap() = 4u;
+/// assert_eq!(_x, 4u);
+/// let _y = x.clone();
+/// assert!(Rc.try_get_mut(&mut x).is_none());

You're right, I guess I never ran the doc tests after changing to free
functions.


Reply to this email directly or view it on GitHub
https://github.com/rust-lang/rust/pull/16101/files#r15597313.

http:https://octayn.net/

http:https://octayn.net/

@aturon
Copy link
Member

aturon commented Jul 30, 2014

@cmr I'd be interested to hear about the use case you have in mind.

@lilyball
Copy link
Contributor Author

Fixed the doc comment examples.

@aturon
Copy link
Member

aturon commented Jul 30, 2014

@cmr So, the gist seems to be that you have some other mechanism telling you that the Rc should be uniquely owned at some point, and hence try_unwrap should always succeed?

@emberian
Copy link
Member

@aturon Yes, but only dynamically, up to the application...

@emberian
Copy link
Member

Also the comment I left explaining the situation seems to have been eaten by github... looking for it in my sent emails.

@emberian
Copy link
Member

@aturon Possibly doesn't make a lot of sense out of context: gfx-rs/gfx#144 (comment)

Essentially you want an affine handle type that, for performance reasons, can't really have a useful destructor, and the only way to share it is behind Rc (or another shared container), but in the interest of not having referring to values that no longer exist, removing the value from the container requires the handle by-value, but you can't get that without try_unwrap. It moves the failure into the user who is sharing the handle, rather than having a handle which refers to a nonexistent value.

@lilyball
Copy link
Contributor Author

@cmr The comment you left was actually a reply to a diff comment, so it's hidden on an outdated diff.

@emberian
Copy link
Member

Ah, yay github.

@aturon
Copy link
Member

aturon commented Jul 31, 2014

FWIW, I'm fine with adding these as "experimental", which they've been marked.

One nit: the current error guidelines, which represent our current best practices, prefer to avoid try_foo when foo returns an Option/Result. (We use names like Vec::pop instead of Vec::try_pop, for example.)

@lilyball
Copy link
Contributor Author

@aturon try_unwrap() doesn't make any sense as unwrap(), there's no precedent for a method of that name returning an Option/Result (existing precedent implies it would fail if it can't return the T). But perhaps try_get_mut() does warrant changing to just get_mut().

@lilyball lilyball changed the title Add is_unique(), try_unwrap(), try_get_mut() to alloc::rc Add is_unique(), try_unwrap(), get_mut() to alloc::rc Jul 31, 2014
@lilyball
Copy link
Contributor Author

Updated. try_get_mut() is now get_mut().

@aturon
Copy link
Member

aturon commented Jul 31, 2014

OK, I'm fine with try_unwrap for now, though this is likely to change with the resolution of the unwrap conventions.

Add a few new free functions to alloc::rc for manipulating
uniquely-owned Rc values. is_unique() can be used to test if the Rc is
uniquely-owned, try_unwrap() can remove the value from a uniquely-owned
Rc, and get_mut() can return a &mut for a uniquely-owned Rc.

These are all free functions, because smart pointers should avoid having
methods when possible. They can't be static methods because UFCS will
remove that distinction. I think we should probably change downgrade()
and make_unique() into free functions as well, but that's out of scope.
bors added a commit that referenced this pull request Aug 1, 2014
Add a few new free functions to alloc::rc for manipulating
uniquely-owned Rc values. is_unique() can be used to test if the Rc is
uniquely-owned, try_unwrap() can remove the value from a uniquely-owned
Rc, and get_mut() can return a &mut for a uniquely-owned Rc.

These are all free functions, because smart pointers should avoid having
methods when possible. They can't be static methods because UFCS will
remove that distinction. I think we should probably change downgrade()
and make_unique() into free functions as well, but that's out of scope.
@bors bors closed this Aug 1, 2014
@bors bors merged commit 192a8a5 into rust-lang:master Aug 1, 2014
@lilyball lilyball deleted the rc_unique_ownership branch August 1, 2014 17:34
tvallotton pushed a commit to tvallotton/rust that referenced this pull request Dec 15, 2023
…kril

fix: Fix `import_map::search_dependencies` getting confused by assoc and non assoc items with the same name

No test case as creating one is kind of tricky... Ideally the code should be restructured such that this collision wouldn't matter in the first place, its kind of a mess.

Fixes rust-lang/rust-analyzer#16074
Fixes rust-lang/rust-analyzer#16080
Fixes rust-lang/rust-analyzer#15845
tvallotton pushed a commit to tvallotton/rust that referenced this pull request Dec 15, 2023
tvallotton pushed a commit to tvallotton/rust that referenced this pull request Dec 15, 2023
internal: Partially revert rust-lang#16101

rust-lang/rust-analyzer#16101 has severe perf regressions unfortunately, so this reverts the part that fixed the issues for now.
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.

None yet

5 participants