Use box syntax in vec! macro

[apasel422]

Closes #28950.

r? @eddyb

[eddyb]

r? @alexcrichton There might be a reason this wasn't done yet.

[alexcrichton]

Hm, I think that there's no reason to not do this. I don't think there's anything visible that can be relied upon by using box instead of Box::new. That being said this is a super common macro, so we may wish to tread carefully.

cc @rust-lang/libs

[sfackler]

I'm on board - this seems like exactly the kind of thing we added #[allow_internal_unstable] for.

[eddyb]

Mind adding a test that would overflow the default stack with the old implementation, and force it to be compiled with no optimizations?

[apasel422]

@eddyb Would something like this suffice?

#![feature(box_syntax)]

const LEN: usize = 1 << 15;

use std::thread::Builder;

fn main() {
    assert!(Builder::new().stack_size(LEN / 2).spawn(|| {
        let vec = <[_]>::into_vec(box ([0; LEN]);
        assert_eq!(vec.len(), LEN);
    }).unwrap().join().is_ok());
}

[eddyb]

@apasel422 the point is to use vec![...] so that the test will pass as long as box [...] is being used in vec!.
(But yes, that test looks good otherwise)

EDIT: Oops, I forgot we don't use the array for for repeats, so it would be hard to trigger without passing a lot of things to vec![] or using a large type - vec![[0; LEN]] might just work (check on playpen or on older build that it overflows the stack).

[apasel422]

@eddyb Right. Do you have an example of how I could use the vec![a, b, c] syntax with enough elements to overflow the stack?

[eddyb]

This crashes on playpen:

fn main() {
    let _v = vec![[0; 1 << 23]];
}

[apasel422]

Is there a way to specify the optimization level in a run-pass test, or do I have to use a run-make test?

Edit: Never mind; found compile-flags.

Edit 2: Actually, because -O is already specified, using // compile-flags: -C opt-level=0 conflicts with it. I think a run-make test is necessary.

[apasel422]

Updated with test.

[alexcrichton]

Could this be added as a run-pass test instead? We've got builders for unoptimized tests which should cover this I believe.

[apasel422]

Sure. I guess I don't have to do anything special to configure the test?

[apasel422]

Done.

[alexcrichton]

To be extra super sure, I'm gonna run crater on this.

[alexcrichton]

Crater reports zero regressions (as expected)

[apasel422]

@alexcrichton Does this need anything else?

[alexcrichton]

Ah I was just hoping to discuss this with the libs team briefly, but I suspect we'll just merge

[bors]

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

[alexcrichton]

We talked about this in triage yesterday and everything seemed all good, thanks again @apasel422!

@bors: r+ 34aed8f

[bors]

⌛ Testing commit 34aed8f with merge 809b14a...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-mac-ios-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt