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

const-stabilize HashMap/HashSet::with_hasher (+ required compiller changes) #118427

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,9 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
// should be fixed later.
let callee_is_unstable_unmarked = tcx.lookup_const_stability(callee).is_none()
&& tcx.lookup_stability(callee).is_some_and(|s| s.is_unstable());
if callee_is_unstable_unmarked {
let can_call_unmarked_function =
super::rustc_allow_const_fn_unstable(tcx, caller, sym::any_unmarked);
if callee_is_unstable_unmarked && !can_call_unmarked_function {
trace!("callee_is_unstable_unmarked");
// We do not use `const` modifiers for intrinsic "functions", as intrinsics are
// `extern` functions, and these have no way to get marked `const`. So instead we
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ symbols! {
anon,
anonymous_lifetime_in_impl_trait,
any,
any_unmarked,
append_const_msg,
arbitrary_enum_discriminant,
arbitrary_self_types,
Expand Down
13 changes: 12 additions & 1 deletion library/std/src/collections/hash/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,18 @@ impl<K, V, S> HashMap<K, V, S> {
/// ```
#[inline]
#[stable(feature = "hashmap_build_hasher", since = "1.7.0")]
#[rustc_const_unstable(feature = "const_collections_with_hasher", issue = "102575")]
#[cfg_attr(
bootstrap,
rustc_const_unstable(feature = "const_collections_with_hasher", issue = "102575")
)]
#[cfg_attr(
not(bootstrap),
rustc_const_stable(
feature = "const_collections_with_hasher",
since = "CURRENT_RUSTC_VERSION"
)
)]
#[rustc_allow_const_fn_unstable(any_unmarked)]
pub const fn with_hasher(hash_builder: S) -> HashMap<K, V, S> {
HashMap { base: base::HashMap::with_hasher(hash_builder) }
}
Expand Down
13 changes: 12 additions & 1 deletion library/std/src/collections/hash/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,18 @@ impl<T, S> HashSet<T, S> {
/// ```
#[inline]
#[stable(feature = "hashmap_build_hasher", since = "1.7.0")]
#[rustc_const_unstable(feature = "const_collections_with_hasher", issue = "102575")]
#[cfg_attr(
bootstrap,
rustc_const_unstable(feature = "const_collections_with_hasher", issue = "102575")
)]
#[cfg_attr(
not(bootstrap),
rustc_const_stable(
feature = "const_collections_with_hasher",
since = "CURRENT_RUSTC_VERSION"
)
)]
#[rustc_allow_const_fn_unstable(any_unmarked)]
pub const fn with_hasher(hasher: S) -> HashSet<T, S> {
HashSet { base: base::HashSet::with_hasher(hasher) }
}
Expand Down
3 changes: 2 additions & 1 deletion library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@
#![feature(no_sanitize)]
#![feature(platform_intrinsics)]
#![feature(prelude_import)]
#![feature(rustc_allow_const_fn_unstable)]
#![feature(rustc_attrs)]
#![feature(rustdoc_internals)]
#![feature(staged_api)]
Expand Down Expand Up @@ -388,7 +389,7 @@
//
// Only for const-ness:
// tidy-alphabetical-start
#![feature(const_collections_with_hasher)]
#![cfg_attr(bootstrap, feature(const_collections_with_hasher))]
#![feature(const_hash)]
#![feature(const_io_structs)]
#![feature(const_ip)]
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/stability-attribute/auxiliary/const-unstable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#![feature(staged_api)]
#![stable(feature = "rust1", since = "1.0.0")]

#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_identity", issue = "1")]
pub const fn identity(x: i32) -> i32 {
x
}
6 changes: 6 additions & 0 deletions tests/ui/stability-attribute/auxiliary/normal-const-fn.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// compile-flags: -Zforce-unstable-if-unmarked

// This emulates a dep-of-std (eg hashbrown), that has const functions it
// cannot mark as stable, and is build with force-unstable-if-unmarked.

pub const fn do_something_else() {}
22 changes: 22 additions & 0 deletions tests/ui/stability-attribute/const-stable-cross-crate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// aux-build:normal-const-fn.rs
// check-pass
#![crate_type = "lib"]
#![feature(staged_api)]
#![feature(rustc_attrs)]
#![feature(rustc_private)]
#![allow(internal_features)]
#![feature(rustc_allow_const_fn_unstable)]
#![stable(feature = "stable_feature", since = "1.0.0")]

extern crate normal_const_fn;

// This ensures std can call const functions in it's deps that don't have
// access to rustc_const_stable annotations (and hense don't have a feature)
// gate.

#[rustc_const_stable(feature = "stable_feature", since = "1.0.0")]
#[stable(feature = "stable_feature", since = "1.0.0")]
#[rustc_allow_const_fn_unstable(any_unmarked)]
pub const fn do_something() {
normal_const_fn::do_something_else()
}
16 changes: 16 additions & 0 deletions tests/ui/stability-attribute/const-unstable-from-unmarked.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// aux-build: const-unstable.rs
// compile-flags: -Zforce-unstable-if-unmarked
#![crate_type = "lib"]
extern crate const_unstable;

// Check that crates build with `-Zforce-unstable-if-unmarked` can't call
// const-unstable functions, despite their functions sometimes being considerd
// unstable.
//
// See https://github.com/rust-lang/rust/pull/118427#discussion_r1409914941 for
// more context.

pub const fn identity(x: i32) -> i32 {
const_unstable::identity(x)
//~^ ERROR `const_unstable::identity` is not yet stable as a const fn
Copy link
Member

@RalfJung RalfJung Dec 1, 2023

Choose a reason for hiding this comment

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

What I was asking is that this should error even if the crate has the feature flag enabled. So please add feature(const_identity). I think then the error will disappear but we still want it to error (and require rustc_allow_const_fn_unstable to pass).

That's how const-stable functions behave, and if we are going to allow calling unmarked functions from const-stable functions we should also apply this check to unmarked functions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changing this breaks a load of stuff in std, as it has a load of these unmarked-unstable functions, and relies on them being able to call further const-unstable functions.

Copy link
Member

@RalfJung RalfJung Dec 2, 2023

Choose a reason for hiding this comment

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

How bad would it be to mark those as #[rustc_const_unstable("const_internals")] or so?

I'd really prefer us to guard against accidental stabilization of const features. It's way too easy to screw that up. I am not very concerned about specifically HashMap::with_hasher as that function does basically nothing, I am concerned about all the other const fn we will have in the future that call into hashbrown (or into other crates). We have to get this right.

Copy link
Member Author

Choose a reason for hiding this comment

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

How bad would it be to mark those as #[rustc_const_unstable("const_internals")] or so?

It's 43 errors in core, can't see how many in std/alloc, so deffinatly doable if T-libs is fine with it.

I'll open a separate PR to do this (+ compiler changes), probably in a couple of weeks (exams).

}
10 changes: 10 additions & 0 deletions tests/ui/stability-attribute/const-unstable-from-unmarked.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: `const_unstable::identity` is not yet stable as a const fn
--> $DIR/const-unstable-from-unmarked.rs:14:5
|
LL | const_unstable::identity(x)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: add `#![feature(const_identity)]` to the crate attributes to enable

error: aborting due to 1 previous error

Loading