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

Tracking issue for box_syntax #49733

Closed
aidanhs opened this issue Apr 6, 2018 · 31 comments · Fixed by #108471
Closed

Tracking issue for box_syntax #49733

aidanhs opened this issue Apr 6, 2018 · 31 comments · Fixed by #108471
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC S-tracking-perma-unstable Status: The feature will stay unstable indefinitely. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@aidanhs
Copy link
Member

aidanhs commented Apr 6, 2018

This is a tracking issue for the box_syntax feature, the special syntax for creating boxes, e.g. box 1usize (note: box patterns is separate and is tracked at at #29641)

This was previously under the same tracking issues as placement (#22181/#27779) but I didn't want to remove it (yet) due to widespread usage in the compiler (Box::new is implemented in terms of it) and in the wider ecosystem (unlike <- etc which only had a handful of crates using it).

Things to decide:

  • how should box syntax be phased out?
    • how easy is it to remove box syntax as a language feature? Can we just describe it as "will never be stabilised in its current form as it's an internal implementation detail, may be stabilised under a new design of placement")?
      • if it were possible to remove it as a language feature, is it reasonable to do so without a replacement in the form of working placement?
      • if we want to keep box syntax internally, should we rename (but keep reserved) the keyword to something else to deliberately break users and indicate it shouldn't be used for placement?
    • is there any reason for users to use box syntax (apart from it looking nice) given Box::new is inline always, or does that not work?
  • if we decide to keep this, there are a number of unresolved questions in box and in expressions (tracking issue for RFC 809) #22181 about the syntax behaviour

Note: when removing this, be careful not to remove the syntax entirely in order to not cause another case of #50832

@pietroalbini pietroalbini added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Apr 6, 2018
@spacekookie
Copy link
Member

Do I understand it correctly that the box syntax is dependent on placement-new (which has now been unapproved as an RFC)?

Personally I like the idea of having some syntactic sugar for creating something as common as a box (it does look nice). But obviously it would need to be built on something that works.

@steveklabnik
Copy link
Member

@spacekookie yes, the big deal for box syntax is placement new.

@oyvindln
Copy link
Contributor

oyvindln commented Jun 27, 2018

s there any reason for users to use box syntax (apart from it looking nice) given Box::new is inline always, or does that not work?

It doesn't optimize well, and even if it did, you would need some other way around it to avoid blowing up the stack in debug mode.
#41160

@mark-i-m
Copy link
Member

is there any reason for users to use box syntax (apart from it looking nice) given Box::new is inline always, or does that not work?

So this is perhaps a bit of a weird usecase, but it is the only reason I have used box at all: I do a lot of no_std projects where the stack doesn't grow automatically. In these cases, using Box::new(MyLargeStruct) will overrun the limited amount of stack space by allocating MyLargeStruct on the stack and copying it to the allocated space. Allocating and initializing in place is critical.

The obvious alternative is to use the allocator directly and unsafely construct a Box from it, but that's annoying and ugly.

See example here: https://github.com/mark-i-m/os2/blob/2ab9d589a4463bd3133157b5c1a2f9f6735a3432/kernel/process/sched.rs#L76-L83

@ice1000
Copy link
Contributor

ice1000 commented Mar 10, 2019

In most cases people just want to simplify the Box::news everywhere. I really want a shorter version and box syntax sounds like a great idea.
Is there a recent plan of stabilizing box syntax?

@vadixidav
Copy link
Contributor

I think that there really isn't a convenient alternative for destructuring boxes without adding a lot of extra nesting. Personally, in very box-heavy code I would prefer that box destructuring be stabilized separately from box placement syntax. For instance, in this file I have 14 uses of the box destructuring syntax and only 1 of the box placement syntax. The code would become significantly less readable from the removal of box destructuring, whereas converting box <thing> with Box::new(thing) certainly doesn't bother me at all because the code is just as readable.

My thought then is that we should separate the tracking issue out for box destructuring so we can get that stabilized. The only reason we might not want to do this is if we want to create a general RFC for pattern matching items into Deref/DerefMut as described here. I don't intend to create said RFC at this time, but if someone else wants to do that then I think that is a viable alternative.

@mark-i-m
Copy link
Member

mark-i-m commented May 1, 2019

This could possible alleviate my usecase: https://crates.io/crates/copyless

@aidanhs
Copy link
Member Author

aidanhs commented Nov 1, 2019

@vadixidav box destructuring(/patterns) is already tracked at #29641 (see mention of 'box patterns' in the issue description)

@jonas-schievink jonas-schievink added B-unstable Blocker: Implemented in the nightly compiler and unstable. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Nov 26, 2019
@kellerkindt
Copy link
Contributor

kellerkindt commented Feb 27, 2020

I was looking at Box::pin when I noticed, that box_syntax is using the prefix syntax. But for the very reason (box x).into() is ugly to type and chain, the await operator(?) is using postfix syntax, So, maybe, for the same reasons, box should use postfix syntax as well?

(box x).into()

vs

x.box.into()

At least, that is what came to my mind. Sorry if this is the wrong ticket or off topic, I just want to see rust getting continuously better and better 😃

@tech6hutch
Copy link

x.box.into()

At least, that is what came to my mind. Sorry if this is the wrong ticket or off topic, I just want to see rust getting continuously better and better 😃

You could easily make a trait that adds a method and call it into_box or boxed:

x.boxed().into()

Implemented like:

trait Boxed {
    fn boxed(self) -> Box<Self>;
}

impl<T> Boxed for T {
    fn boxed(self) -> Box<Self> {
        Box::new(self)
    }
}

(adding type restrictions as needed)

@josephlr
Copy link
Contributor

josephlr commented Apr 30, 2020

For my use case, I'm less concerned with syntactic sugar (Box::new is fine) but with blowing up the stack (in both debug and release mode) as @oyvindln and @mark-i-m suggested.

If I have a struct like:

const SIZE: usize = 4096*4096;
#[derive(Copy, Clone)]
#[repr(C, align(4096))]
pub struct Pages([u8; SIZE]);

impl Default for Pages {
    fn default() -> Self {
        Pages([0u8; SIZE])
    }
}

Creating a Box<Pages> without overflowing the stack is quite hard. Consider the following example implementations:

pub fn with_new() -> Box<Pages> {
    Box::new(Pages([0u8; SIZE]))
}

pub fn with_box_syntax() -> Box<Pages> {
    box Pages([0u8; SIZE])
}

pub fn with_default() -> Box<Pages> {
    Default::default()
}

pub fn with_raw_alloc() -> Box<Pages> {
    use std::alloc::{alloc_zeroed, Layout, handle_alloc_error};
    let layout = Layout::new::<Pages>();
    let ptr = unsafe { alloc_zeroed(layout) };
    if ptr.is_null() {
        handle_alloc_error(layout);
    }
    unsafe { Box::from_raw(ptr as *mut Pages) }
}

with_new blows the stack in debug and release, while with_box_syntax and with_default blow the stack only in debug mode. Godbolt Link

So even something like @mark-i-m's example isn't guaranteed to work. Should the box syntax in with_box_syntax be guaranteed to not blow the stack (even in debug mode)?

@fghzxm
Copy link

fghzxm commented Nov 12, 2020

@josephlr https://doc.rust-lang.org/std/boxed/struct.Box.html#method.new_zeroed_slice

@TanakaDev
Copy link

I'm sort of confused on the state of this feature. Will it ever be stabilized? What are the reasons it cannot be stabilized in its current form if so?

@mark-i-m
Copy link
Member

mark-i-m commented Dec 1, 2020

I believe there is a bunch of design work left. In particular, it's not clear how adjacent features like a hypothetical "placement new" syntax would work, and how they would interact with box syntax.

@notgull
Copy link

notgull commented Dec 24, 2020

Here's a small, maybe naive, idea. Considering how #73394 was used to "pseudo-stablize" #64490, what if we had a macro that wrapped around box syntax? For instance:

#[allow_internal_unstable(box_syntax)]
macro_rules! boxed {
    ($item: expr) => {{
        box ($item)
    }}
}

@chorman0773
Copy link
Contributor

chorman0773 commented Dec 24, 2020

I'm working on a competing compiler to rustc, and I am particularily aiming to make Box not magic (the current impl is not even a lang item. It uses an unstable DerefMove trait and an internal attribute for moving out of boxes). I would support the proposed macro, certainly over specialized syntax (and I have written an implementation of it to that effect).
I'm curious about the extent of the mandatory move-elision with box syntax, and thus the proposed macro. In particular, for expressions involving non-trivial control flow, would eliding the move and constructing the value in-place still be necessary?

@fghzxm
Copy link

fghzxm commented Jan 1, 2021

If we can have typeof, we can simulate box syntax with the help of new_uninit (#63291):

macro_rules! boxed {
    ($e:expr) => {
        {
            let mut ws = Box::<typeof($e)>::new_uninit();
            let contents = $e;
            unsafe {
                // type inference is forbidden for raw ptrs, hence the need for typeof
                ws.as_mut_ptr().write(contents);
                ws.assume_init()
            }
        }
    }
}

See https://rust.godbolt.org/z/c5eE6T for a hacky demo; new_uninit generates the exact same assembly as box syntax (except on the error path), down to register selection.

Never mind, with maybe_uninit_extra it gets there without typeof (https://rust.godbolt.org/z/Kf1Kdh):

#![feature(maybe_uninit_extra, new_uninit)]

macro_rules! boxed {
    ($e:expr) => {
        {
            let mut ws = Box::new_uninit();
            ws.write($e);
            unsafe { ws.assume_init() }
        }
    }
}

@est31
Copy link
Member

est31 commented May 22, 2022

FTR, deref patterns (#87121) don't help with removal of box_syntax, only with removal of box_patterns (#29641).

box_syntax is still needed inside std facade crates, to implement Box::new, but also to implement vec![]. There are box syntax free alternatives for both, but in #87781 no alternative could be found to remove box syntax from those two places. See #87781 (comment) for one of the perf run results. That being said, back when I filed the PR, both the latest and the minimum LLVM was older. Now it's 12 (#90175), so maybe the regressions are smaller now. Maybe it's time for a second attempt? If box syntax is only used in library crates, it might also be possible to give it a less prominent API, something that discourages nightly uses of it. If that is added though, it should be phased in gradually.

But yeah, going via the deprecation lint route might be useful because I suppose a lot of people are using box syntax. Unconditional warnings are equivalent to unconditional errors, because you can't turn them off. But a deprecation lint might work, idk. Given that it's a nightly feature, it could even start with deny by default.

@est31
Copy link
Member

est31 commented May 22, 2022

I've opened #97293 to figure out a replacement syntax. In there, I introduce #[rustc_box] Box:new(x) which gets lowered to the internal representation of box x very early on during compilation. Then at a later point in time, box x can be removed from the early parts like AST. And then it's properly become an implementation detail.

Note that I have edited this comment, it didn't say at the start what I intended. Sorry for the trouble!

@est31
Copy link
Member

est31 commented Aug 5, 2022

I've been wondering about #[rustc_box] Box:new(x): the issue with it is that it requires two feature gates to be enabled, both stmt_expr_attributes, and rustc_attrs. Furthermore, both are general features and enabling them exposes users to an unnecessarily large range of unstable features.

The alternative that I could think of would be a lang-item function like Box::new_in_place or a free function alloc::boxed::box_in_place with its own feature gate. What do people think?

@estebank
Copy link
Contributor

estebank commented Oct 3, 2022

The alternative that I could think of would be a lang-item function like Box::new_in_place or a free function alloc::boxed::box_in_place with its own feature gate. What do people think?

@est31 Yet another alternative would be to have a rustc_box!() macro or similar, but I don't know if that buys us much beyond a lang-item (beyond making it slightly clear that the macro intrinsic might be doing something beyond a pure function call).

@chorman0773
Copy link
Contributor

chorman0773 commented Oct 3, 2022 via email

@est31
Copy link
Member

est31 commented Oct 3, 2022

@estebank Personally, I see the attribute on the function more as a guaranteed optimization. But admittedly you can see macros in the same way, as one of the things they provide is guaranteed inlining.

I've recently been drawn towards builtin#box(). There have been questions on how to pretty print format! invocations in the MCP for making format an AST token. builtin# syntax might also help with the implementation of a builtin offset_of macro, at least if that macro gets capabilities that can't be expressed soundly in current Rust.

So maybe the builtin# syntax could be used in multiple places. It might be the go-to syntax if something isn't quite a function, or a macro, but a custom syntax is too complicated for it. I've been thinking about doing an MCP for it. What do you think?

@ShadowJonathan
Copy link

Apologies for the ping on this question for anyone else, but I haven't heard of builtin# before, where can I follow that development?

@tgross35
Copy link
Contributor

tgross35 commented Jan 22, 2023

It took me an awful long time to hunt down the status of this issue, and the history is kind of confusing. So to save anyone time who's not familiar with this issue, here are the answers to the questions I had:

  • box_syntax will not be stabalized. Work is being done to prune it out so this issue can be closed (est31's comment outlined some of the blockers, many of those issues have been closed Tracking issue for box_syntax #49733 (comment))
  • It is also unrelated to placement new, which is being worked on separately. Placement new will likely be an optimization for Box, Vec, etc, and they may get new methods to specify the address
  • box_syntax isn't blocking box_patterns, which will also eventually be removed (something better is coming, see my comment here Tracking issue for box_patterns feature #29641 (comment))

bors added a commit to rust-lang-ci/rust that referenced this issue Jan 23, 2023
Do not use box syntax in `std`

See rust-lang#94970 and rust-lang#49733. About half of the `box` instances in `std` do not even need to allocate, the other half can simply be replaced with `Box::new`.

`@rustbot` label +T-libs
r? rust-lang/libs
@bors bors closed this as completed in f1b1ed7 Mar 13, 2023
flip1995 pushed a commit to flip1995/rust that referenced this issue Mar 24, 2023
…ieb,est31

Remove `box_syntax`

r? `@Nilstrieb`

This removes the feature `box_syntax`, which allows the use of `box <expr>` to create a Box, and finalises removing use of the feature from the compiler. `box_patterns` (allowing the use of `box <pat>` in a pattern) is unaffected.
It also removes `ast::ExprKind::Box` - the only way to create a 'box' expression now is with the rustc-internal `#[rustc_box]` attribute.
As a temporary measure to help users move away, `box <expr>` now parses the inner expression, and emits a `MachineApplicable` lint to replace it with `Box::new`

Closes rust-lang#49733
sourcefrog added a commit to sourcefrog/cargo-mutants that referenced this issue Apr 30, 2023
It was dropped in rust-lang/rust#49733
and isn't supported by new Syn.

box unpacking looks like a good substitute
sourcefrog added a commit to sourcefrog/cargo-mutants that referenced this issue Apr 30, 2023
It was dropped in rust-lang/rust#49733
and isn't supported by new Syn.

box unpacking looks like a good substitute
sourcefrog added a commit to sourcefrog/cargo-mutants that referenced this issue Apr 30, 2023
It was dropped in rust-lang/rust#49733
and isn't supported by new Syn.

box unpacking looks like a good substitute
parno added a commit to verus-lang/verus that referenced this issue Dec 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC S-tracking-perma-unstable Status: The feature will stay unstable indefinitely. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.