Skip to content

overloaded-box protocol changes - #22086

Closed
pnkfelix wants to merge 10 commits into
rust-lang:masterfrom
pnkfelix:fsk-box-placer-take-3
Closed

overloaded-box protocol changes#22086
pnkfelix wants to merge 10 commits into
rust-lang:masterfrom
pnkfelix:fsk-box-placer-take-3

Conversation

@pnkfelix

Copy link
Copy Markdown
Contributor

This is a bootstrapping illustration of the new box prototype. The main point is to illustrate the fallout that occurs with the compiler type inference as it (nearly) stands today. (Note that much of that fallout has now landed in PR #23002.)

This PR is a revised version of PR #22006, building upon PR #22012 to remove much (but certainly not all) of the fallout observed there.

Now most of the fallout in this PR is from where box <expr> was being used in contexts that are implicitly coerced to Box<Trait>, which is not compatible with the new box protocol (at least, not yet), since the use of coercion there subverts the box-protocol's attempt to infer the appropriate boxed-type to create. (This is noted at this comment on the RFC.)

For now, I am usually getting around this by using Box::new(..);

  • it is worth noting that in HEAP { <expr> } can often fix such cases, but not always;
  • Box::new(<expr>) always works, though it may introduce an intermediate copy that could have been otherwise avoided.

see also rust-lang/rfcs#809 (which is now merged as of this (updated) writing).


oh, and

[breaking-change]

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @nikomatsakis

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

@pnkfelix

Copy link
Copy Markdown
ContributorAuthor

(this isn't really up for review, so I removed the assignment to @nikomatsakis , who should nonetheless be cc'ed.)

@pnkfelix
pnkfelixforce-pushed the fsk-box-placer-take-3 branch 6 times, most recently from 23ea741 to 989515cCompareFebruary 8, 2015 13:12
@pnkfelix

Copy link
Copy Markdown
ContributorAuthor

(is not quite right yet, I think I need to accommodate #21988 in some as yet unknown-to-me manner.)

@pnkfelix
pnkfelixforce-pushed the fsk-box-placer-take-3 branch from b50cab7 to 53062c4CompareFebruary 8, 2015 22:09
@pnkfelix

Copy link
Copy Markdown
ContributorAuthor

(okay I figured out how to accommodate #21988)

@pnkfelix

Copy link
Copy Markdown
ContributorAuthor

cc #22181

@pnkfelix
pnkfelixforce-pushed the fsk-box-placer-take-3 branch 3 times, most recently from 3430c4d to 2ed35dfCompareFebruary 18, 2015 20:54
@pnkfelix
pnkfelixforce-pushed the fsk-box-placer-take-3 branch 3 times, most recently from 9bc1982 to c1d4f12CompareMarch 2, 2015 08:01
@bors

bors commented Mar 2, 2015

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #22797) made this pull request unmergeable. Please resolve the merge conflicts.

@pnkfelix
pnkfelixforce-pushed the fsk-box-placer-take-3 branch from c1d4f12 to d0c965dCompareMarch 2, 2015 14:05
@bors

bors commented Mar 2, 2015

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #22510) made this pull request unmergeable. Please resolve the merge conflicts.

@pnkfelix
pnkfelixforce-pushed the fsk-box-placer-take-3 branch from d0c965d to aaf0563CompareMarch 3, 2015 10:14
@bors

bors commented Mar 3, 2015

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #22995) made this pull request unmergeable. Please resolve the merge conflicts.

@bors

bors commented Mar 6, 2015

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #22899) made this pull request unmergeable. Please resolve the merge conflicts.

@pnkfelix
pnkfelixforce-pushed the fsk-box-placer-take-3 branch from 27b06bd to 10f05f9CompareMarch 6, 2015 14:32
@nikomatsakis

Copy link
Copy Markdown
Contributor

This looks good to me. I'd still like those measurements. ;)

UPDATE: r+, in case it wasn't clear

@pnkfelix
pnkfelixforce-pushed the fsk-box-placer-take-3 branch from 10f05f9 to 52f6957CompareMarch 9, 2015 18:56
@pnkfelixpnkfelix mentioned this pull request Mar 11, 2015
@pnkfelix

Copy link
Copy Markdown
ContributorAuthor

Some rough code size data:

configure --enable-optimize --disable-debug:

Baseline: 6048ba8
Post-runway Pre-protocol: b1ed6333100ac0a1bb46a3ed7aa378b3dfe61d92
Post-protocol: e8f64b6c72442d799df86ff35c8657efba1db44b

stage2 rlib nameBaseline sizePost-runway Pre-protocol sizePost-protocol sizePercent change over Baseline
libcore161043501610389816114376(+0.062%)
liblibc286482286482286482(0%)
librand830358830352830414(+0.007%)
liballoc209163620916562132392(+1.949%)
libunicode573894057389105739010(+0.001%)
libcollections611659061158006122776(+0.101%)
libstd152712541527437615353768(+0.540%)
librustc_bitflags130241302413024(0%)
libflate155512155522155580(+0.044%)
libarena163656163672163642(-0.009%)
liblog220018221032222908(+1.314%)
libterm838220838218849644(+1.363%)
libserialize311700231176623120722(+0.119%)
libgetopts658434657510659776(+0.204%)
librbml654998655160655440(+0.067%)
libtest156422415638601579410(+0.971%)
libgraphviz155552155592155226(-0.210%)
libfmt_macros223850223756223768(-0.037%)
libsyntax311977463120432831303020(+0.337%)
librustc_llvm552503545525051855255078(+0.009%)
librustc_back191518019136641920308(+0.268%)
librustc400724184007212840018614(-0.134%)
librustc_typeck114071381140921411413472(+0.056%)
librustc_borrowck222488622255842232206(+0.329%)
librustc_resolve291582629155162889206(-0.913%)
librustc_trans124891921249213212372586(-0.934%)
librustc_privacy688020687596688842(+0.119%)
librustc_lint139805413982961405288(+0.517%)
librustc_driver524675452496985212736(-0.648%)
librustdoc136340961364061013672884(+0.284%)
total232643764232762598(+0.051%)

Numbers gathered via invocation:

for f in $(ls -tr rust-placer/objdir-opt/x86_64-apple-darwin/stage2/lib/rustlib/x86_64-apple-darwin/lib/stamp.* ) ; do \
F=$(basename $f | sed -e s/^stamp./lib/ ) ; \
( echo $F; \
( ls -l rust-plbase/objdir-opt/x86_64-apple-darwin/stage2/lib/rustlib/x86_64-apple-darwin/lib/$F-*.rlib && \
ls -l rust-placer/objdir-opt/x86_64-apple-darwin/stage2/lib/rustlib/x86_64-apple-darwin/lib/$F-*.rlib ) | \
cut -f 8 -d ' ' ) | xargs ; done 

(and then manually feeding the data into some scheme code for calculating the percent-deltas)

@pnkfelix
pnkfelixforce-pushed the fsk-box-placer-take-3 branch from 52f6957 to e8f64b6CompareMarch 11, 2015 10:45
@bors

bors commented Mar 12, 2015

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #23265) made this pull request unmergeable. Please resolve the merge conflicts.

…ed to Box.
Precursor for landing overloaded-`box`, since that will decouple the
`box` syntax from the exchange heap (and in fact will eliminate the
use of the two aforementioned lang items).
Instead, the new demonstration program shows a definition of the
`str_eq` lang item. (We do not have that many procedural lang-items to
choose from, which is a good sign for our efforts to decouple the
compiler from the runtime!)
(This previously used a demo of `panic_bounds_check`, but a `str_eq`
demonstration is both easier to code and arguably a more interesting
aspect of the language to discuss.)
Fix unsafe.md example.
@pnkfelix
pnkfelixforce-pushed the fsk-box-placer-take-3 branch from e8f64b6 to 68b81c1CompareMarch 12, 2015 23:02
See also issue 22405, which tracks going back to `box <expr>` if
possible in the future.
Precursor for overloaded-`box` and placement-`in`; see Issue 22181.
…ck> }`.
Note that `box <expr>` itself remains unchanged.
update test/compile-fail/feature-gate-box-expr.rs to reflect new feature gates.
Part of what lands with Issue 22181.
Namely:
* Update run-pass/new-box-syntax
* Fix doc-embedded test for `alloc::boxed` to reflect new syntax.
* Fix test/debuginfo/box.rs to reflect new syntax.
Part of what lands with Issue 22181.
…t suite.
Precursor for overloaded-`box` and placement-`in`; see Issue 22181.
Precursor for overloaded-`box` and placement-`in`; see Issue 22181.
@pnkfelix
pnkfelixforce-pushed the fsk-box-placer-take-3 branch from 68b81c1 to a0e3c71CompareMarch 12, 2015 23:04
@bors

bors commented Mar 13, 2015

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #23292) made this pull request unmergeable. Please resolve the merge conflicts.

@nikomatsakis

Copy link
Copy Markdown
Contributor

@pnkfelix should we close this PR or keep it open?

@pnkfelix

Copy link
Copy Markdown
ContributorAuthor

@nikomatsakis i guess i'll close it; I know where to find it when I get a chance to revive it (i.e. after I fix the ptr::write codegen issues.

bors added a commit that referenced this pull request Jun 3, 2015
Hack the move_val_init intrinsic to trans directly into the destination address.
This is to remove an intermediate (and unnecessary) alloca on the stack that one otherwise suffers when using this intrinsic.
This is part of the `box` protocol work; in particular, this is meant to address the `ptr::write` codegen issues alluded to at this comment: #22086 (comment)
cc #22181
lnicola pushed a commit to lnicola/rust that referenced this pull request Apr 20, 2026
lnicola pushed a commit to lnicola/rust that referenced this pull request Apr 20, 2026
Sign up for freeto 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.

4 participants

@pnkfelix@rust-highfive@bors@nikomatsakis