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

desugaring-based placement-in (just in, no box); take-5 branch #27215

Merged
merged 27 commits into from
Jul 24, 2015

Conversation

pnkfelix
Copy link
Member

Macro desugaring of in PLACE { BLOCK } into "simpler" expressions following the in-development "Placer" protocol.

Includes Placer API that one can override to integrate support for in into one's own type. (See RFC 809.)

Part of #22181

Replaced PR #26180.

Turns on the in PLACE { BLOCK } syntax, while leaving in support for the old box (PLACE) EXPR syntax (since we need to support that at least until we have a snapshot with support for in PLACE { BLOCK }.

(Note that we are not 100% committed to the in PLACE { BLOCK } syntax. In particular I still want to play around with some other alternatives. Still, I want to get the fundamental framework for the protocol landed so we can play with implementing it for non Box types.)


Also, this PR leaves out support for desugaring-based box EXPR. We will hopefully land that in the future, but for the short term there are type-inference issues injected by that change that we want to resolve separately.

Even after expansion, the generated expressions still track depth of
such pushes (i.e. how often you have "pushed" without a corresponding
"pop"), and we add a rule that in a context with a positive
`push_unsafe!` depth, it is effectively an `unsafe` block context.

(This way, we can inject code that uses `unsafe` features, but still
contains within it a sub-expression that should inherit the outer
safety checking setting, outside of the injected code.)

This is a total hack; it not only needs a feature-gate, but probably
should be feature-gated forever (if possible).

ignore-pretty in test/run-pass/pushpop-unsafe-okay.rs
update test/compile-fail/feature-gate-box-expr.rs to reflect new feature gates.

Part of what lands with Issue 22181.
…dest`.

rebase update to typeck/check/mod.rs
(Over time the stability checking has gotten more finicky; in
particular one must attach the (whole) span of the original `in PLACE
BLOCK` expression to the injected references to unstable paths, as
noted in the comments.)

call `push_compiler_expansion` during the placement-`in` expansion.
…unstable`.

It is all `debug!` instrumentation so it should not impose a cost on
non-debug builds.
The two tests are separate since the current implementation performs
the feature gate checks at distinct phases in the compilation, with an
`abort_if_errors` calls separating them.
…ntax.rs`.

The original test program exercised the (garbage-collected heap)
allocation form `box (GC) ...`, feeding an instance of `Structure` in
as the expression. This obviously went away when `GC` went away, but
there's no reason for us not to include an appropriate unit test here
for the same form, just for heap-allocated instances of `Structure`.
@rust-highfive
Copy link
Collaborator

r? @pcwalton

(rust_highfive has picked a reviewer for you, use r? to override)

@pnkfelix
Copy link
Member Author

r? @nikomatsakis

@pnkfelix
Copy link
Member Author

(fyi a local run a make check shows that I broke the help suggestion on test/parse-fail/parenthesized-box-expr-message.rs .. i'll be fixing that in parallel with addressing the review feedback...)

/// to carry the explicit alignment; that is just a work-around for
/// the fact that the `align_of` intrinsic currently requires the
/// input type to be Sized (which I do not think is strictly
/// necessary).
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you expect align_of to return for a trait?

Copy link
Member

Choose a reason for hiding this comment

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

Presumably align_of would be bounded by an Aligned trait, a supertrait of Sized, implemented by all types except traits.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a non-issue because you can't allocate a box for a trait object (or at least, you first allocate a box for the concrete type). I think the use case we were primarily interested in was something like box [v; n] where n is NOT a compile-time constant. It's not clear how best to handle this (if we should at all), but it might be neat if it worked. (It would allocate enough space for an n-length array and initialize to 0.) Evaluation order is kind of weird, since you must evaluate n before v -- well, I guess that's not true, since we're going to be copying v into each spot, so we could still evaluate the v first. Anyway, I think that's the only case where it makes sense to allocate a box for an unsized type without having an actual value in hand.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I've been wanting this for ages, but it was blocked on having a placement API at all.
That is correct, you evaluate v first, then n, then allocate n * sizeof(v) bytes and only afterwards you can copy v into n different slots.
You can also do it for trait objects from Sized types: you allocate the size of the type, write the typed value in, then save the vtable pointer in the fat pointer without any unsizing step.

Direct allocation of unsized types is necessary to construct types that do not allow type-param-based unsizing, such as:

struct str([u8]);
struct Entity {
    id: u32,
    kind: Kind,
    name: &str,
    value: Any
}

Wouldn't &str(*b"foo") be a nice desugaring of "foo"? Admittedly, it couldn't be exposed everywhere without allowing invalid UTF-8. Unsafe fields/constructors, anyone?

@nikomatsakis
Copy link
Contributor

r+ modulo comments and nits. very nicely factored, what a joy to read.

@pnkfelix
Copy link
Member Author

@bors r=nikomatsakis d066a7b

bors added a commit that referenced this pull request Jul 24, 2015
…akis

Macro desugaring of `in PLACE { BLOCK }` into "simpler" expressions following the in-development "Placer" protocol.

Includes Placer API that one can override to integrate support for `in` into one's own type.  (See [RFC 809].)

[RFC 809]: https://github.com/rust-lang/rfcs/blob/master/text/0809-box-and-in-for-stdlib.md

Part of #22181

Replaced PR #26180.

Turns on the `in PLACE { BLOCK }` syntax, while leaving in support for the old `box (PLACE) EXPR` syntax (since we need to support that at least until we have a snapshot with support for `in PLACE { BLOCK }`.

(Note that we are not 100% committed to the `in PLACE { BLOCK }` syntax.  In particular I still want to play around with some other alternatives.  Still, I want to get the fundamental framework for the protocol landed so we can play with implementing it for non `Box` types.)

----

Also, this PR leaves out support for desugaring-based `box EXPR`.  We will hopefully land that in the future, but for the short term there are type-inference issues injected by that change that we want to resolve separately.
@bors
Copy link
Contributor

bors commented Jul 24, 2015

⌛ Testing commit d066a7b with merge 9413a92...

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.

7 participants