Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upoverloaded-`box` and placement-`in` #809
Conversation
pnkfelix
referenced this pull request
Feb 4, 2015
Closed
RFC for placement box with Placer trait for overloading. #470
This comment has been minimized.
This comment has been minimized.
|
|
alexcrichton
reviewed
Feb 4, 2015
| the `<place-expr>` to determine what placement code to run. | ||
|
|
||
| * The only stablized implementation for the `box <expr>` operator | ||
| proposed by this RFC is `Box<T>`. The question of which other types |
This comment has been minimized.
This comment has been minimized.
alexcrichton
Feb 4, 2015
Member
To clarify, does this affect inference in an adverse way if we add later implementations to support box expressions creating Rc<T> and Arc<T>? For example, would this compile successfully today with only one implementation, but fail with many implementations?
drop(box 3);If we only have one implementation of boxing, does this mean that we'll successfully resolve this to Box<i32> vs Rc<i32>?
This comment has been minimized.
This comment has been minimized.
pnkfelix
Feb 4, 2015
Author
Member
That's a good question; I had assumed that the coherence rules would ensure that drop(box 3) would be rejected, but its possible I misremember/misunderstand.
This comment has been minimized.
This comment has been minimized.
alexcrichton
Feb 4, 2015
Member
I think it will too, I just haven't kept up with all the coherence rules and trait selection recently! FWIW this does not compile:
pub trait Foo {
fn make() -> Self;
}
impl Foo for i32 {
fn make() -> i32 { 2 }
}
fn main() {
drop(Foo::make());
}foo.rs:10:5: 10:9 error: unable to infer enough type information about `_`; type annotations required [E0282]
foo.rs:10 drop(Foo::make());
^~~~
error: aborting due to previous error
This comment has been minimized.
This comment has been minimized.
pnkfelix
Feb 4, 2015
Author
Member
(in the prototype from the Appendix, if you add
drop(box_!( 7 ));
it fails to compile, with the message
<anon>:36:13: 36:22 error: unable to infer enough type information about `_`; type annotations required [E0282]
<anon>:36 let mut place = ::protocol::BoxPlace::make_place();
^~~~~~~~~
So, a terrible error message, but that does jibe with my hypothesis that such code would be rejected.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
nikomatsakis
Feb 5, 2015
Contributor
@alexcrichton I am assuming we will add implementations for Rc and Arc once the protocol exists, so this concern is somewhat moot.
This comment has been minimized.
This comment has been minimized.
|
Will this land behind a feature gate? |
eddyb
reviewed
Feb 5, 2015
| use std::rc::Rc; | ||
| let mut v = vec![1,2]; | ||
| in (v.emplace_back()) 3; // has return type `()` |
This comment has been minimized.
This comment has been minimized.
eddyb
Feb 5, 2015
Member
I prefer in(v.back()) 3, fwiw, as "emplace" is just a longer way of conveying the same thing in does.
Also it could return &mut T, but I may be mistaken.
This is straying a bit from the topic at hand, and I'm not sure what will come of it, but I have the start of a design that makes *v.back() = 3 possible.
This comment has been minimized.
This comment has been minimized.
pnkfelix
Feb 5, 2015
Author
Member
Regarding names and return types: The beauty of the approach of this RFC is to try to avoid spending time right now quibbling over these kinds of details, which are the topic of library design and not language design. (Having said that, I am not attached to any particular names here.)
The reason I said "has return type () was mostly to make it clear that the API can be made flexible enough so that the in (<place-expr>) <expr> form is usable even in scenarios where it is solely used for imperative update and not for its return value.
Regarding making *v.back() = 3 possible: Okay, that's good, but I do not think such a feature will make the need for placement-in obsolete. (It would, I guess, obsolete its use in this example of Vec::emplace_back.)
This comment has been minimized.
This comment has been minimized.
|
@brson asked:
Hmm. So my gut reaction is "yes; shouldn't everything land behind a feature gate?" But another (more important) way to interpret your question is as follows:
(Where "this", I believe, refers to the I would like to try to ensure we have a beta cycle with the syntax unfeature-gated. Otherwise, its not clear why we would attempt to put it into the 1.0 release at all. |
This comment has been minimized.
This comment has been minimized.
netvl
commented
Feb 5, 2015
|
Has anyone considered using |
This comment has been minimized.
This comment has been minimized.
|
@netvl a large host of syntaxes have been considered. I guess I should have referenced those discussions in the RFC. See e.g. https://github.com/pnkfelix/rfcs/blob/fsk-placement-box-rfc/text/0000-placement-box.md#same-semantics-but-different-surface-syntax which does list One particular constraint I am interested in maintaining is: The evaluation order should reflect the order of expressions in the syntax. Also the author of #405 points out another drawback: the (Meanwhile, the other alternative of |
This comment has been minimized.
This comment has been minimized.
|
@netvl another problem with syntaxes that have an optional suffix is that they tend to induce ambiguity. For example, |
This comment has been minimized.
This comment has been minimized.
|
While in (v.emplace_back()) 3;
// feels like, what? "In... emplace to v's back, 3. ...?"
// looks like a strange version of `v.emplace_back(3);`?
// then why can't we use `v.emplace_back(3);` etc. etc.
let b6 = in (HEAP) 6;
// feels like, check if 6 is in the HEAP?Just some alternatives I feel more natural, if the order is required to be placer → expression: // use `box in` phrase. that is, `box expression in[to] place`.
box in (v.end()) 1 + 2 + 3;
let b6 = box in (HEAP) 4 + 5 + 6;
// more rearrangements. already mentioned by pnkfelix.
// the `box` keyword is *required* for human to get the correct context.
in v.end() box 1 + 2 + 3;
let b6 = in HEAP box 4 + 5 + 6;
// reuse `move` keyword. should have no conflict with `move |x| y + x`.
// but sounds like going to move the placer instead of the expression.
// misnomer since nothing is actually moved.
move (v.end()) 1 + 2 + 3;
let b6 = move (HEAP) 4 + 5 + 6;
// use `move in` phrase. a bit too verbose, but make it clear that the placer
// is the target. rearranged from `move expression in[to] place`.
move in (v.end()) 1 + 2 + 3;
let b6 = move in (HEAP) 4 + 5 + 6;
// use a new operator. free to define whatever it means, but hard to learn about it.
v.end() <| 1 + 2 + 3;
let b6 = HEAP <| 4 + 5 + 6;
v.end() <~ 1 + 2 + 3;
let b6 = HEAP <~ 4 + 5 + 6;
// just create another built-in syntax extension?
emplace!(v.end(), 1 + 2 + 3);
let b6 = emplace!(HEAP, 4 + 5 + 6);
|
This comment has been minimized.
This comment has been minimized.
|
+1! Better than this rfc.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
that said, while I agree that |
This comment has been minimized.
This comment has been minimized.
|
One more thought regarding stabilization -- even if we want to land this feature-gated, I think it'd be better to take the RFC and get this work in tree. I feel pretty sure that we want this sort of "emplacement" feature, even if we're not ready to commit to the details in particular. I'd rather have it in tree so we can experiment and gain experience with it. (I'd feel differently if no work had been done -- but @pnkfelix has most of the pieces implemented by now.) |
This comment has been minimized.
This comment has been minimized.
|
I agree that inference-based overloaded And I may be misunderstanding something, but why add the |
This comment has been minimized.
This comment has been minimized.
I believe it's no different from how this works: let v: Vec<_> = (0u32 .. 10).collect();
I agree, that's why I suggested
I also agree that |
This comment has been minimized.
This comment has been minimized.
|
@nikomatsakis I agree that |
pnkfelix
self-assigned this
Feb 5, 2015
This comment has been minimized.
This comment has been minimized.
|
A note about |
This comment has been minimized.
This comment has been minimized.
|
It'd be very nice if chained NRVO, URVO and placement-new were guaranteed to apply in scenarios like
and elide all the unnecessary copies. |
This comment has been minimized.
This comment has been minimized.
|
@petrochenkov regarding return-value-optimization: the intention is that the desugaring described in the RFC hopefully presents the code in a manner where LLVM can optimize it accordingly. If we observe that LLVM fails to optimize the code as desired, then we can instead switch from a macro-based implementation to more integrated support within the whole compiler pipeline; but that will hopefully be unnecessary. (i.e., the focus of this RFC is on the changes that are visible to the programmer, not on the quality of implementation, though of course we do care about quality of implementation.) |
pnkfelix
referenced this pull request
Feb 6, 2015
Closed
No checkin: overloaded-`box` protocol changes #22006
This comment has been minimized.
This comment has been minimized.
|
I just wanted to jot a few cases where the current desugaring still fails to do as well as I would like, even after augmenting
(Both of the above cases compile fine if you use It would be good to resolve that, or at least incorporate it into the drawbacks section if I cannot fix it quickly. Some of the errors I'm seeing from the desugaring on the above cases is arising because the traits as written are not as general as they could be, with respect to the It may be possible to generalize the trick I used in rust-lang/rust#22006, instead of making it a special case just for the |
This comment has been minimized.
This comment has been minimized.
|
nice!
|
pnkfelix
referenced this pull request
Feb 7, 2015
Open
closure/FnMut type mismatch using `box (HEAP) ...` that does not occur with `box ...` #22043
This comment has been minimized.
This comment has been minimized.
|
The core team has decided to accept this RFC. However, the The reason for this feature gate is not only the usual matter of not wanting to expose partially implemented features, but also some questions as to the desirability of the overall design (as raised in this discussion thread):
Ordinarily these uncertainties might lead to the RFC being postponed. However, given that there is an existing |
nikomatsakis
referenced this pull request
Feb 11, 2015
Closed
`box` and `in` expressions (tracking issue for RFC 809) #22181
This comment has been minimized.
This comment has been minimized.
|
Tracking issue is rust-lang/rust#22181 |
nikomatsakis
merged commit 2880173
into
rust-lang:master
Feb 11, 2015
This comment has been minimized.
This comment has been minimized.
|
Up to date final text will be available here: https://github.com/rust-lang/rfcs/blob/master/text/0809-box-and-in-for-stdlib.md |
pnkfelix
referenced this pull request
Feb 16, 2015
Open
Could type inference insert coercions to accommodate box desugaring? #22405
This comment has been minimized.
This comment has been minimized.
nwin
commented
Feb 23, 2015
|
My favorite would be the latter variant because it does not use this strange inversion. |
This comment has been minimized.
This comment has been minimized.
diwic
commented
Feb 23, 2015
|
Do you think there'll be confusion if the name "box" can refer both to the "simple heap pointer" and "any type of pointer"? I e, if you write:
A newcomer would probably expect something like this:
Rather than:
In this case there is a distinction between |
This comment has been minimized.
This comment has been minimized.
nwin
commented
Feb 23, 2015
|
I don’t think this will lead to confusion, especially when talking about the language. |
This comment has been minimized.
This comment has been minimized.
diwic
commented
Feb 23, 2015
|
Regardless of whether |
This comment has been minimized.
This comment has been minimized.
comex
commented
Feb 23, 2015
|
I agree that since emplacing is semantically almost always preferable (possibly faster, never slower, changed ordering of allocation and construction usually not significant), it should be as close to syntactically preferable as possible, or at least not much worse. |
This comment has been minimized.
This comment has been minimized.
|
cc RFC #98 |
This comment has been minimized.
This comment has been minimized.
|
someone said that (of course one can say "but what about expressions like |
This comment has been minimized.
This comment has been minimized.
m13253
commented
Mar 6, 2015
What about |
This comment has been minimized.
This comment has been minimized.
|
@m13253 that's the less-than-or-equal operator... |
This comment has been minimized.
This comment has been minimized.
erkinalp
commented
Mar 16, 2015
|
What about |
pnkfelix commentedFeb 4, 2015
Summary:
box (<place-expr>) <expr>instead to:in <place-expr> { <block> }(Note: waspreviously)in (<place-expr>) <expr>box <expr>to an overloaded operator that chooses its implementation based on the expected type.core::opsfor both operators, so that libstd can provide support for the overloaded operators; the traits are unstable so that the language designers are free to revise the underlying protocol in the future post 1.0.(rendered
text/)(rendered draft)
Tracking issue: rust-lang/rust#22181