alloc: make vec! unavailable under no_global_oom_handling
#96089
Conversation
rustbot
added
the
T-libs
|
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
|
(rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
A fine lines below there is a #[cfg(test)]
macro_rules! vec {too. I don't think we've bothered to run tests with oom panicking disabled, but for sake of a future where we do, can we modify that cfg too? |
|
If I understand correctly, you want to prevent that somebody writes |
ojeda
force-pushed
the
no-vec-no_global_oom_handling
branch
from
22701aa
to
81406b1
Compare
The `vec!` macro has 3 rules, but two are not usable under `no_global_oom_handling` builds of the standard library (even with a zero size): ```rust let _ = vec![42]; // Error: requires `exchange_malloc` lang_item. let _ = vec![42; 0]; // Error: cannot find function `from_elem`. ``` Thus those two rules should not be available to begin with. The remaining one, with an empty matcher, is just a shorthand for `new()` and may not make as much sense to have alone, since the idea behind `vec!` is to enable `Vec`s to be defined with the same syntax as array expressions. Furthermore, the documentation can be confusing since it shows the other rules. Thus perhaps it is better and simpler to disable `vec!` entirely under `no_global_oom_handling` environments, and let users call `new()` instead: ```rust let _: Vec<i32> = vec![]; let _: Vec<i32> = Vec::new(); ``` Notwithstanding this, a `try_vec!` macro would be useful, such as the one introduced in rust-lang#95051. If the shorthand for `new()` is deemed worth keeping on its own, then it may be interesting to have a separate `vec!` macro with a single rule and different, simpler documentation. Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda
force-pushed
the
no-vec-no_global_oom_handling
branch
from
81406b1
to
8cec88b
Compare
|
Reworded commit message a bit better -- no changes. |
| @@ -34,7 +34,7 @@ | |||
| /// be mindful of side effects. | |||
| /// | |||
| /// [`Vec`]: crate::vec::Vec | |||
| #[cfg(not(test))] | |||
| #[cfg(all(not(no_global_oom_handling), not(test)))] | |||
There was a problem hiding this comment.
Small nit: I might prefer not(any(...)) here, which I think reads a little nicer?
I generally find it faster to digest or'd lists myself, but it probably varies from person to person, so feel free to resolve if you disagree.
There was a problem hiding this comment.
I used this one since not(no_global_oom_handling) is the one normally found elsewhere on its own (since it is used to disable things) and also makes it a prefix of the other (so perhaps it is easier to see it is the same thing but for test/!test):
#[cfg(all(not(no_global_oom_handling), not(test)))]
#[cfg(all(not(no_global_oom_handling), test))]From a quick look, there are currently 2 not(any(...)), and 4 all(not(...)) (for no_global_oom_handling/test).
I am fine with either, so please let me know which one you prefer given the above.
(Personally, I think multiple cfgs would be more readable and easier for diff/VCS purposes; and I hope for &&/and operators, but... :)
There was a problem hiding this comment.
Hm, yeah, not sure. Let's leave it as-is for now -- I can see either direction being beneficial. Multiple cfgs would also be fine I think (they don't work when you want an or but for and I think should work fine).
|
r=me, modulo the nit if you agree with it. |
|
@bors r+ |
|
📌 Commit 8cec88b has been approved by |
bors
added
S-waiting-on-bors
…, r=Mark-Simulacrum `alloc`: make `vec!` unavailable under `no_global_oom_handling` `alloc`: make `vec!` unavailable under `no_global_oom_handling` The `vec!` macro has 3 rules, but two are not usable under `no_global_oom_handling` builds of the standard library (even with a zero size): ```rust let _ = vec![42]; // Error: requires `exchange_malloc` lang_item. let _ = vec![42; 0]; // Error: cannot find function `from_elem`. ``` Thus those two rules should not be available to begin with. The remaining one, with an empty matcher, is just a shorthand for `new()` and may not make as much sense to have alone, since the idea behind `vec!` is to enable `Vec`s to be defined with the same syntax as array expressions. Furthermore, the documentation can be confusing since it shows the other rules. Thus perhaps it is better and simpler to disable `vec!` entirely under `no_global_oom_handling` environments, and let users call `new()` instead: ```rust let _: Vec<i32> = vec![]; let _: Vec<i32> = Vec::new(); ``` Notwithstanding this, a `try_vec!` macro would be useful, such as the one introduced in rust-lang#95051. If the shorthand for `new()` is deemed worth keeping on its own, then it may be interesting to have a separate `vec!` macro with a single rule and different, simpler documentation. Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
…, r=Mark-Simulacrum `alloc`: make `vec!` unavailable under `no_global_oom_handling` `alloc`: make `vec!` unavailable under `no_global_oom_handling` The `vec!` macro has 3 rules, but two are not usable under `no_global_oom_handling` builds of the standard library (even with a zero size): ```rust let _ = vec![42]; // Error: requires `exchange_malloc` lang_item. let _ = vec![42; 0]; // Error: cannot find function `from_elem`. ``` Thus those two rules should not be available to begin with. The remaining one, with an empty matcher, is just a shorthand for `new()` and may not make as much sense to have alone, since the idea behind `vec!` is to enable `Vec`s to be defined with the same syntax as array expressions. Furthermore, the documentation can be confusing since it shows the other rules. Thus perhaps it is better and simpler to disable `vec!` entirely under `no_global_oom_handling` environments, and let users call `new()` instead: ```rust let _: Vec<i32> = vec![]; let _: Vec<i32> = Vec::new(); ``` Notwithstanding this, a `try_vec!` macro would be useful, such as the one introduced in rust-lang#95051. If the shorthand for `new()` is deemed worth keeping on its own, then it may be interesting to have a separate `vec!` macro with a single rule and different, simpler documentation. Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Rollup of 6 pull requests Successful merges: - rust-lang#94493 (Improved diagnostic on failure to meet send bound on future in a foreign crate) - rust-lang#95809 (Fix typo in bootstrap.py) - rust-lang#96086 (Remove `--extern-location` and all associated code) - rust-lang#96089 (`alloc`: make `vec!` unavailable under `no_global_oom_handling`) - rust-lang#96122 (Fix an invalid error for a suggestion to add a slice in pattern-matching) - rust-lang#96142 (Stop using CRATE_DEF_INDEX outside of metadata encoding.) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
alloc: makevec!unavailable underno_global_oom_handlingThe
vec!macro has 3 rules, but two are not usable underno_global_oom_handlingbuilds of the standard library(even with a zero size):
Thus those two rules should not be available to begin with.
The remaining one, with an empty matcher, is just a shorthand for
new()and may not make as much sense to have alone, since theidea behind
vec!is to enableVecs to be defined with the samesyntax as array expressions. Furthermore, the documentation can be
confusing since it shows the other rules.
Thus perhaps it is better and simpler to disable
vec!entirelyunder
no_global_oom_handlingenvironments, and let users callnew()instead:Notwithstanding this, a
try_vec!macro would be useful, such asthe one introduced in #95051.
If the shorthand for
new()is deemed worth keeping on its own,then it may be interesting to have a separate
vec!macro witha single rule and different, simpler documentation.
Signed-off-by: Miguel Ojeda ojeda@kernel.org