-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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 const fn
type_name
#63084
Comments
Can the output change between compiling a library crate, and a binary crate using that library? Or only when switching rustc versions? |
No, we're using global paths now.
Yes, there needs to be a change in rustc that causes a change in output. |
I might take a look at this. (I implemented the support for using the actual I think we definitely know it works at compile time though, don't we? That's what this test I added at the time checks for. |
Sorry it took me a bit to get to this. Just opened a PR. |
…, r=oli-obk `const fn`-ify `std::any::type_name` as laid out in rust-lang#63084 A test, based on the one I added when I implemented support for the underlying `core::intrinsics::type_name` being allowed in `const fn` contexts, is included.
…, r=oli-obk `const fn`-ify `std::any::type_name` as laid out in rust-lang#63084 A test, based on the one I added when I implemented support for the underlying `core::intrinsics::type_name` being allowed in `const fn` contexts, is included.
Rollup of 7 pull requests Successful merges: - #62293 (Unsupport the `await!(future)` macro) - #62469 (Add doc links to liballoc crate page) - #63095 (Turn `INCOMPLETE_FEATURES` into lint) - #63117 (Use global variable 'environ' to pass environments to rtpSpawn) - #63123 (`const fn`-ify `std::any::type_name` as laid out in #63084) - #63129 (Subslice patterns: Test passing static & dynamic semantics.) - #63147 (Updated RELEASES.md for 1.37.0) Failed merges: r? @ghost
How does this break referential transparency? I'd like to see that elaborated upon. |
I might be wrong, but I think the first "checkbox" can be checked since my PR was merged? Personally, I disagree, but some people seem to think that despite the fact Rust has fully reified generics, it's somehow a "dangerous" concept for your "average Joe/Jane" to be able to access the (100%-guaranteed-correct) name of a generic parameter. Something about "Java programmers I used to work with did Some Bad Thing and I don't want Rust programmers to also do Some Bad Thing". I don't know anything about Java though so I'm probably not overly qualified to get too much into that. |
Hi, is there a stabilization issue for this |
This is the tracking issue, so it will get closed on stabilization. I don't know what the next steps for stabilizing it are though. The discussion around referential transparency needs to be had (although with specialization it'd become obsolete, because then we'd get this feature anyway, but in a less convenient way). I think posting use cases would be very helpful for the discussion. Since this works with feature gates, you can always show a use case with nightly. |
Hi, certainly I can write about a use case that I am working on! I'm part of the Embedded WG where I am currently working on an instrumentation/logging tool for embedded targets. Here, as you probably know, we have strict requirements on code size as the micro controllers have very little memory. To this end I place formating strings and (I want) type strings in an // test1 is some variable
mylog::log!("Look what I got: {}", &test1);
//
// Expands to:
//
const fn get_type_str<T>(_: &T) -> &'static str {
// type_name needs to be a const fn for this to work
core::any::type_name::<T>()
}
// ugly hack to tranform `&'static str` -> `[u8; str.len()]`
union Transmute<T: Copy, U: Copy> {
from: T,
to: U,
}
const FMT: &'static str = "Look what I got: {}";
const TYP: &'static str = get_type_str(&test1);
// Transform string into byte arrays at compile time so
// they can be placed in a specific memory region
// Formating string placed in a specific linker section
#[link_section = ".some_info_section"]
static F: [u8; FMT.as_bytes().len()] = unsafe {
*Transmute::<*const [u8; FMT.len()], &[u8; FMT.as_bytes().len()]> {
from: FMT.as_ptr() as *const [u8; FMT.as_bytes().len()],
}
.to
};
// Type string placed in a specific linker section
#[link_section = ".some_info_section"]
static T: [u8; TYP.as_bytes().len()] = unsafe {
*Transmute::<*const [u8; TYP.len()], &[u8; TYP.as_bytes().len()]> {
from: TYP.as_ptr() as *const [u8; TYP.as_bytes().len()],
}
.to
};
// Here we send where the strings are stored in the ELF + the data to the host
write_frame_to_host(&F, &T, &test1) Link script:
By doing this the strings are available in the generated ELF but not loaded onto the target, and the type can be reconstructed through DWARF with help of the type string. Currently the string is placed in This is why I am asking about stabilization here. |
I would also like to get this stabilized and can present a use-case. I'm working on firestorm: a low overhead intrusive flame graph profiler. I created Part of keeping the overhead of TLDR: I need this to be |
To elaborate here's code I would like to have be able to compile: #[macro_export]
macro_rules! profile_method {
($($t:tt)*) => {
let _firestorm_method_guard = {
const FIRESTORM_STRUCT_NAME: &'static str = ::std::any::type_name::<Self>();
let event_data: &'static _ = &$crate::internal::EventData::Start(
$crate::internal::Start::Method {
signature: stringify!($($t)*),
typ: FIRESTORM_STRUCT_NAME,
}
);
$crate::internal::start(event_data);
$crate::internal::SpanGuard
};
};
} It seems there are at least 2 problems here though. First, is that |
@That3Percent I don't think the |
@oli-obk Yes, you are right. Both issues would need to be resolved to support my use-case in the way that I would like. |
I found a bug with If you comment out the first panic, you get the correct type name. |
This seems to be fixed now? |
Has there been any progress on the stabilization of |
Ping again on the status of stabilization? |
|
Would you believe I clicked the wrong link in Rustdoc? Sorry. |
Is there any outlook on stabilization/progress towards it? |
There's a rendering bug and a soundness bug that we need to fix before we can stabilize. Both are linked from the main post |
The soundness bug seems to have been fixed by now, and all other linked issues also seem to be fixed, is there any particular roadblock remaining? |
Reproduction: `cargo test -p apollo-federation -- -q` ## Background #5157 introduced a new kind of snapshot test for the Rust query planner. Test schemas are specified as sets of subgraph schemas, so composing them is needed. Since we don’t have composition in Rust yet, the tests rely on JS composition through Rover. To avoid a dependency on Rover on CI and for most contributors, the composed supergraph are cached in the repository. Rover use is opt-in and only required when a cached file is missing or out of date. (Composition inputs are hashed.) The file name is derived (through a macro) from the test function name. To detect name conflicts, a static `OnceLock<Mutex<HashSet<&'static str>>>` is used to ensure no name is used more than once. ## The problem The static hash set relies on all snapshot tests running in the same process. This is the case with `cargo test`, but `cargo nextest run` as used on CI isolates every test in its own process. This breaks conflict detection of cache file names for composed supergraph schemas, since each test only "sees" itself in the static hash set. #5240 introduced a name conflict: composition is used in a function called twice with different arguments. Because nextest was used both locally and on CI, the conflict went undectected. As a result, running `cargo test` on dev fails because the conflict is detected. ## This PR This PR fixes this case of cache file name conflict, but conflict detection is still broken with nextest. As a result it’s possible that this kind of conflict could happen again and be merged undectected. ## Non-solutions tried * Nextest has a notion of [test groups](https://nexte.st/book/test-groups), but they don’t appear to let multiple tests run in the same process * Instead of relying on the runtime side effect of tests, could conflict detection rely on enumerating tests at compile time? The [`linkme` crate](https://crates.io/crates/linkme) is a building block Router used to register Rust plugins. It could be used here in all composition inputs can be made const, but `std::any::type_name` used in a macro to extract the current function name is not `const fn` [yet](rust-lang/rust#63084) ## Potential solutions * Remove the cache and accept the dependency on Rover for testing. This impacts CI and all contributors. * Require every `planner!` macro invocation to specify an explicit cache file name instead of relying on the function name. Then conflict detection can use `linkme`. * Move conflict detection to a separate test that something something parses Rust source files of other tests something
Reproduction: `cargo test -p apollo-federation -- -q` ## Background #5157 introduced a new kind of snapshot test for the Rust query planner. Test schemas are specified as sets of subgraph schemas, so composing them is needed. Since we don’t have composition in Rust yet, the tests rely on JS composition through Rover. To avoid a dependency on Rover on CI and for most contributors, the composed supergraph are cached in the repository. Rover use is opt-in and only required when a cached file is missing or out of date. (Composition inputs are hashed.) The file name is derived (through a macro) from the test function name. To detect name conflicts, a static `OnceLock<Mutex<HashSet<&'static str>>>` is used to ensure no name is used more than once. ## The problem The static hash set relies on all snapshot tests running in the same process. This is the case with `cargo test`, but `cargo nextest run` as used on CI isolates every test in its own process. This breaks conflict detection of cache file names for composed supergraph schemas, since each test only "sees" itself in the static hash set. #5240 introduced a name conflict: composition is used in a function called twice with different arguments. Because nextest was used both locally and on CI, the conflict went undectected. As a result, running `cargo test` on dev fails because the conflict is detected. ## This PR This PR fixes this case of cache file name conflict, but conflict detection is still broken with nextest. As a result it’s possible that this kind of conflict could happen again and be merged undectected. ## Non-solutions tried * Nextest has a notion of [test groups](https://nexte.st/book/test-groups), but they don’t appear to let multiple tests run in the same process * Instead of relying on the runtime side effect of tests, could conflict detection rely on enumerating tests at compile time? The [`linkme` crate](https://crates.io/crates/linkme) is a building block Router used to register Rust plugins. It could be used here in all composition inputs can be made const, but `std::any::type_name` used in a macro to extract the current function name is not `const fn` [yet](rust-lang/rust#63084) ## Potential solutions * Remove the cache and accept the dependency on Rover for testing. This impacts CI and all contributors. * Require every `planner!` macro invocation to specify an explicit cache file name instead of relying on the function name. Then conflict detection can use `linkme`. * Move conflict detection to a separate test that something something parses Rust source files of other tests something
Reproduction: `cargo test -p apollo-federation -- -q` ## Background #5157 introduced a new kind of snapshot test for the Rust query planner. Test schemas are specified as sets of subgraph schemas, so composing them is needed. Since we don’t have composition in Rust yet, the tests rely on JS composition through Rover. To avoid a dependency on Rover on CI and for most contributors, the composed supergraph are cached in the repository. Rover use is opt-in and only required when a cached file is missing or out of date. (Composition inputs are hashed.) The file name is derived (through a macro) from the test function name. To detect name conflicts, a static `OnceLock<Mutex<HashSet<&'static str>>>` is used to ensure no name is used more than once. ## The problem The static hash set relies on all snapshot tests running in the same process. This is the case with `cargo test`, but `cargo nextest run` as used on CI isolates every test in its own process. This breaks conflict detection of cache file names for composed supergraph schemas, since each test only "sees" itself in the static hash set. #5240 introduced a name conflict: composition is used in a function called twice with different arguments. Because nextest was used both locally and on CI, the conflict went undectected. As a result, running `cargo test` on dev fails because the conflict is detected. ## This PR This PR fixes this case of cache file name conflict, but conflict detection is still broken with nextest. As a result it’s possible that this kind of conflict could happen again and be merged undectected. ## Non-solutions tried * Nextest has a notion of [test groups](https://nexte.st/book/test-groups), but they don’t appear to let multiple tests run in the same process * Instead of relying on the runtime side effect of tests, could conflict detection rely on enumerating tests at compile time? The [`linkme` crate](https://crates.io/crates/linkme) is a building block Router used to register Rust plugins. It could be used here in all composition inputs can be made const, but `std::any::type_name` used in a macro to extract the current function name is not `const fn` [yet](rust-lang/rust#63084) ## Potential solutions * Remove the cache and accept the dependency on Rover for testing. This impacts CI and all contributors. * Require every `planner!` macro invocation to specify an explicit cache file name instead of relying on the function name. Then conflict detection can use `linkme`. * Move conflict detection to a separate test that something something parses Rust source files of other tests something
Reproduction: `cargo test -p apollo-federation -- -q` #5157 introduced a new kind of snapshot test for the Rust query planner. Test schemas are specified as sets of subgraph schemas, so composing them is needed. Since we don’t have composition in Rust yet, the tests rely on JS composition through Rover. To avoid a dependency on Rover on CI and for most contributors, the composed supergraph are cached in the repository. Rover use is opt-in and only required when a cached file is missing or out of date. (Composition inputs are hashed.) The file name is derived (through a macro) from the test function name. To detect name conflicts, a static `OnceLock<Mutex<HashSet<&'static str>>>` is used to ensure no name is used more than once. The static hash set relies on all snapshot tests running in the same process. This is the case with `cargo test`, but `cargo nextest run` as used on CI isolates every test in its own process. This breaks conflict detection of cache file names for composed supergraph schemas, since each test only "sees" itself in the static hash set. #5240 introduced a name conflict: composition is used in a function called twice with different arguments. Because nextest was used both locally and on CI, the conflict went undectected. As a result, running `cargo test` on dev fails because the conflict is detected. This PR fixes this case of cache file name conflict, but conflict detection is still broken with nextest. As a result it’s possible that this kind of conflict could happen again and be merged undectected. * Nextest has a notion of [test groups](https://nexte.st/book/test-groups), but they don’t appear to let multiple tests run in the same process * Instead of relying on the runtime side effect of tests, could conflict detection rely on enumerating tests at compile time? The [`linkme` crate](https://crates.io/crates/linkme) is a building block Router used to register Rust plugins. It could be used here in all composition inputs can be made const, but `std::any::type_name` used in a macro to extract the current function name is not `const fn` [yet](rust-lang/rust#63084) * Remove the cache and accept the dependency on Rover for testing. This impacts CI and all contributors. * Require every `planner!` macro invocation to specify an explicit cache file name instead of relying on the function name. Then conflict detection can use `linkme`. * Move conflict detection to a separate test that something something parses Rust source files of other tests something
Reproduction: `cargo test -p apollo-federation -- -q` ## Background apollographql#5157 introduced a new kind of snapshot test for the Rust query planner. Test schemas are specified as sets of subgraph schemas, so composing them is needed. Since we don’t have composition in Rust yet, the tests rely on JS composition through Rover. To avoid a dependency on Rover on CI and for most contributors, the composed supergraph are cached in the repository. Rover use is opt-in and only required when a cached file is missing or out of date. (Composition inputs are hashed.) The file name is derived (through a macro) from the test function name. To detect name conflicts, a static `OnceLock<Mutex<HashSet<&'static str>>>` is used to ensure no name is used more than once. ## The problem The static hash set relies on all snapshot tests running in the same process. This is the case with `cargo test`, but `cargo nextest run` as used on CI isolates every test in its own process. This breaks conflict detection of cache file names for composed supergraph schemas, since each test only "sees" itself in the static hash set. apollographql#5240 introduced a name conflict: composition is used in a function called twice with different arguments. Because nextest was used both locally and on CI, the conflict went undectected. As a result, running `cargo test` on dev fails because the conflict is detected. ## This PR This PR fixes this case of cache file name conflict, but conflict detection is still broken with nextest. As a result it’s possible that this kind of conflict could happen again and be merged undectected. ## Non-solutions tried * Nextest has a notion of [test groups](https://nexte.st/book/test-groups), but they don’t appear to let multiple tests run in the same process * Instead of relying on the runtime side effect of tests, could conflict detection rely on enumerating tests at compile time? The [`linkme` crate](https://crates.io/crates/linkme) is a building block Router used to register Rust plugins. It could be used here in all composition inputs can be made const, but `std::any::type_name` used in a macro to extract the current function name is not `const fn` [yet](rust-lang/rust#63084) ## Potential solutions * Remove the cache and accept the dependency on Rover for testing. This impacts CI and all contributors. * Require every `planner!` macro invocation to specify an explicit cache file name instead of relying on the function name. Then conflict detection can use `linkme`. * Move conflict detection to a separate test that something something parses Rust source files of other tests something
With the recent Are there more known roadblocks to |
What were the fruits of that discussion? What blockers, if any, were named? Can this tracking issue be updated with any outstanding concerns |
The main concern that got raised is that if we give people type_name but not type_id, they will start using type_name comparison to check if two types are equal. The docs explicitly say not to do that, but it'll happen anyway when the proper alternative (type_id comparison) is simply not available.
Do you remember what this is about? In which sense does this function break referential transparency? We do have to ensure that the result is consistent across all crates in a crate graph but I assume that is already the case? Or are there |
|
Hm, that seems like a potential soundness concern then, doesn't it? The same const could produce different results when it is sometimes evaluated with that flag and sometimes without. Is there any more systematic way we can avoid accidentally having const-eval depend on such flags? This is a really easy mistake to make and it'll hardly ever be detected... but it is a soundness issue. |
If the |
This is a tracking issue for making and stabilizing
type_name
asconst fn
. It is not clear whether this is sound. Needs some T-lang discussion probably, too.Steps needed:
const
and#[rustc_const_unstable(feature = "const_type_name")
to the function and add tests showing that it works at compile-time.type_name
depends on-Zverbose
flag #94187TypeId
exposes equality-by-subtyping vs normal-form-syntactic-equality unsoundness. #97156 replicates fortype_name
, tooThe text was updated successfully, but these errors were encountered: