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

Mark std::mem::drop as unstable until negative bounds are implemented #536

Closed
wants to merge 1 commit into from
Closed

Mark std::mem::drop as unstable until negative bounds are implemented #536

wants to merge 1 commit into from

Conversation

bstrie
Copy link
Contributor

@bstrie bstrie commented Dec 19, 2014

@bstrie bstrie mentioned this pull request Dec 19, 2014
@bstrie
Copy link
Contributor Author

bstrie commented Dec 19, 2014

On IRC it has been mentioned that an alternative would be to have a lint specifically for this function, that would give a compilation error if the function is called with an implicitly copyable type. I'm fine with this solution, as long as the lint cannot be overridden. My only concern is that this function should not be callable with an implicitly copyable type, regardless of how that is achieved.

@ftxqxd
Copy link
Contributor

ftxqxd commented Dec 19, 2014

This would be problematic when used with generic functions. Something like this:

fn frob<T>(x: T) {
    // …do something…
    drop(x);
    // …do some more things…
}

is perfectly valid today, but will not compile under this change. A lint would probably be much better because it could ignore cases like this where the type might be Copy, but might not be.

Another alternative is to put a Drop bound on the T in drop. This retains the problem with generics above, but would prevent things like drop(&mut foo), which I’ve seen people try a few times but does nothing.

@sfackler
Copy link
Member

A Drop bound won't work since something like struct Foo { v: Vec<u8> } doesn't implement Drop but still uses move semantics.

@bstrie
Copy link
Contributor Author

bstrie commented Dec 19, 2014

@P1start I contend that that generic code is wrong. If you expect drop to do something to your value and it doesn't do anything, then the fact that the compiler doesn't stop you is an error on Rust's part.

@Gankra
Copy link
Contributor

Gankra commented Dec 20, 2014

bstrie: the code might not be intending to "do" anything from its own perspective, but simply trying to be a good citizen and release the object as soon as it's not needed. However such generic code strikes me as a bit contrived (taking something by-value that you only need temporarily, but otherwise know nothing about).

I would personally be interested in "upgrading" drop so that it effectively kills the variable of interest. In this way it could be used to "unborrow" a reference in the middle of a scope, without having to scope-juggle variables.

@bstrie
Copy link
Contributor Author

bstrie commented Dec 20, 2014

@gankro would your scheme require drop to become a lang item?

@Gankra
Copy link
Contributor

Gankra commented Dec 20, 2014

It would probably have to be an intrinsic of some kind. Same as mem::forget.

@bstrie
Copy link
Contributor Author

bstrie commented Dec 20, 2014

If drop were to gain such an oft-requested feature, then I'd like to note that #535 becomes even more important. :)

@Gankra
Copy link
Contributor

Gankra commented Dec 20, 2014

Yes, if drop was upgraded I'd absolutely like it to become something like discard.

@ftxqxd
Copy link
Contributor

ftxqxd commented Dec 20, 2014

drop wouldn’t need to be upgraded at all if we had proper non-lexical lifetimes. I think that’s a much better solution than forcing the problem of shortening lifetimes onto the user of the language.

@Gankra
Copy link
Contributor

Gankra commented Dec 20, 2014

@P1start I'm not sure if "variable is not used after this point" is part of the scope of non-lexical lifetimes (I think that would imply eager drop?). In particular note that if you drop() an &T it only gets copied. You can only end a borrow based on an &T by having the variable of interest go out of scope. If drop/discard actual killed the variable itself (semantically), then this would be accomplished (might still need non-lexical for that fact to register.

@ftxqxd
Copy link
Contributor

ftxqxd commented Dec 20, 2014

The SEME regions RFC mentions shortening borrows within the same scope as the first point in the Motivation section.

@Gankra
Copy link
Contributor

Gankra commented Dec 20, 2014

@P1start Ah, you're correct (been a long while since I read that). Point retracted, then!

@petrochenkov
Copy link
Contributor

@bstrie

I contend that that generic code is wrong. If you expect drop to do something to your value and it doesn't do anything, then the fact that the compiler doesn't stop you is an error on Rust's part.

See Vec::truncate for example, it should drop the elements regardless of their Copyness (it just can't use the drop function for it).
C++ (sorry for mentioning it again) even have a language feature for it - pseudo-destructors, the sole purpose of which is to do nothing when destructor syntax turns out to be used on primitive types in generics.
So, -1 to the idea, drop should be able to be used in generics.

@arielb1
Copy link
Contributor

arielb1 commented Dec 21, 2014

If we want this, it should probably go in a lint, not as a generic.

@Thiez
Copy link

Thiez commented Dec 23, 2014

This seems really pointless to me, unless we get types that implement both Drop and Copy, in which case drop should probably be replaced by a lang item that takes ownership without copying, or we need the move keyword back that used to be in the language a long time ago. Is that going to be a thing? I sure hope not.

Any confusion that is currently caused by droping Copy types is because implicitly copyable types are not linear, and perhaps this isn't explained as well as it should be?

@aturon
Copy link
Member

aturon commented Dec 30, 2014

@bstrie Thanks for the RFC. After the discussion here and on IRC, it seems preferable to create a lint for this instead (which could likely be landed directly as a PR) -- it could probably give a clearer error message than negative bounds (which do not yet have a fully fleshed out design). I'm going to close this RFC in favor of a lint.

@aturon aturon closed this Dec 30, 2014
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.

8 participants