-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Add T to PhantomData impl Debug #99099
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
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
A counter argument to this change is that we don't include |
We never include generic type in any implementation of Debug however A use case of #[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct First;
#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct Last;
#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct OneContext<Behavior, Context> {
behavior: PhantomData<Behavior>,
context: Context,
}
impl<Context> BitOr for OneContext<First, Context> {
type Output = Self;
fn bitor(self, _rhs: Self) -> Self::Output {
self
}
}
impl<Context> BitOr for OneContext<Last, Context> {
type Output = Self;
fn bitor(self, rhs: Self) -> Self::Output {
rhs
}
} Here This kind of use of |
Hm, I have somewhat mixed feelings on a change like this. I think it's true that this information can be useful, but it's also somewhat of an unusual thing for us to do -- most generic types don't include generic information in their print out. I think overall I'm net-positive on this change, and it seems like something that we could revert in the future, but I'd like to get another set of eyes on it. Let's r? @joshtriplett |
This seems useful to me, to the extent that it's useful for PhantomData to show up in Debug at all. On the other hand, I suspect that PhantomData is also a type that many structures would want to skip in Debug given an attribute allowing the |
@bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (1f1defc): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Footnotes |
…sion of Rust due to PhantomData debug now including type info
This add debug information for
PhantomData
, I believe it's make sense to add this to debug impl ofPhantomData
sinceT
is what define what is thePhantomData
just write"PhantomData"
is not very useful for debugging.Alternative:
PhantomData::<{}>
PhantomData { t: "str_type" }
@rustbot label +T-libs-api -T-libs