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.

Sign up for GitHub

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

alloc: make vec! unavailable under no_global_oom_handling #96089

Merged
merged 1 commit into from
Apr 19, 2022
Merged

alloc: make vec! unavailable under no_global_oom_handling #96089

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions library/alloc/src/macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)))]
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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... :)

Copy link
Member

Choose a reason for hiding this comment

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

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).

#[macro_export]
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_diagnostic_item = "vec_macro"]
Expand All @@ -55,7 +55,7 @@ macro_rules! vec {
// required for this macro definition, is not available. Instead use the
// `slice::into_vec` function which is only available with cfg(test)
// NB see the slice::hack module in slice.rs for more information
#[cfg(test)]
#[cfg(all(not(no_global_oom_handling), test))]
macro_rules! vec {
() => (
$crate::vec::Vec::new()
Expand Down