Add key and value methods to DebugMap #2696

KodrAus · 2019-05-01T18:23:57Z

Add two new methods to std::fmt::DebugMap for writing the key and value part of a map entry separately:

impl<'a, 'b: 'a> DebugMap<'a, 'b> {
    pub fn key(&mut self, key: &dyn Debug) -> &mut Self;
    pub fn value(&mut self, value: &dyn Debug) -> &mut Self;
}

Centril · 2019-05-01T20:26:58Z

text/0000-debug-map-key-value.md

+# Drawbacks
+[drawbacks]: #drawbacks
+
+The proposed `key` and `value` methods are't immediately useful for `Debug` implementors that are able to call `entry` instead. This creates a decision point where there wasn't one before. The proposed implementation is also going to be less efficient than the one that exists now because it introduces a few conditionals.


Regarding efficiency, I would suggest testing this out with a PR and then doing a timer build to see how much perf regresses (that just affects compile times but it's better than nothing). With that data we can evaluate the drawback better.

I opened up a PR (rust-lang/rust#60458) that we can use to check correctness and get some timings 👍

I don't expect the difference to be significant, since it amounts to a few conditionals but it'll be good to verify!

Alrighty, I posted a quick benchmark in the PR, but also inlining the results here:

Before (current .entry()) After (.key().value())

36 ns/iter ( /- 7) 44 ns/iter ( /- 2)

Centril · 2019-05-01T20:27:59Z

text/0000-debug-map-key-value.md

+
+- Write an alternative implementation of the format builders. The output from this alternative implementation would need to be kept reasonably in-sync with the one in the standard library. It doesn't change very frequently, but does from time to time. It would also have to take the same care as the standard library implementation to retain formatting flags when working with entries.
+- Buffer keys and format them together with values when the whole entry is available. Unless the key is guaranteed to live until the value is supplied (meaning it probably needs to be `'static`) then the key will need to be formatted into a string first. This means allocating (though the cost could be amortized over the whole map) and potentially losing formatting flags when buffering.
+


A third option here is to simply not error when keys and values come alone. That might actually be useful in some weird semi-map semi-array cases. That would also be more efficient?

Hmm, that's an interesting thought 🤔 I think that might just mask incorrect implementations though. I imagine we'd be tracking the same additional state too so that we knew when a key or value called 'out of order' should be formatted in a set-like way rather than a map-like way.

Yeah quite possibly.
Would be good to record this in the text of the RFC in any case. :)

text/0000-debug-map-key-value.md

ollie27 · 2019-05-19T14:41:31Z

text/0000-debug-map-key-value.md

+    pub fn finish(&mut self) -> fmt::Result {
+        // Make sure there isn't a partial entry
+        if self.has_key {
+            return Err(fmt::Error);


Creating a fmt::Error like this isn't allowed. From https://doc.rust-lang.org/nightly/std/fmt/index.html#formatting-traits:

a formatting implementation must and may only return an error if the passed-in Formatter returns an error.

As this indicates a programming error I think an explicit panic here would be correct.

Sure! A panic seems reasonable.

KodrAus · 2019-05-26T23:15:33Z

CC @japaric in case you're interested, given your work on an alternative formatting stack.

KodrAus · 2019-05-28T02:01:52Z

Nominating for discussion at the next triage to get some more input from the wider libs team.

alexcrichton · 2019-05-28T14:50:12Z

I'll go ahead an propose to merge, seems like a reasonable feature to me!

@rfcbot fcp merge

rfcbot · 2019-05-28T14:50:15Z

Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

rfcbot · 2019-06-27T15:15:24Z

🔔 This is now entering its final comment period, as per the review above. 🔔

rfcbot · 2019-07-07T15:16:27Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

…alue.md

…hton Add key and value methods to DebugMap Implementation PR for an active (not approved) RFC: rust-lang/rfcs#2696. Add two new methods to `std::fmt::DebugMap` for writing the key and value part of a map entry separately: ```rust impl<'a, 'b: 'a> DebugMap<'a, 'b> { pub fn key(&mut self, key: &dyn Debug) -> &mut Self; pub fn value(&mut self, value: &dyn Debug) -> &mut Self; } ``` I want to do this so that I can write a `serde::Serializer` that forwards to our format builders, so that any `T: Serialize` can also be treated like a `T: Debug`.

…ue, r=alexcrichton Stabilize the debug_map_key_value feature RFC: rust-lang/rfcs#2696 Tracking issue: rust-lang#62482 Stabilizes the `debug_map_key_value` feature, which covers: ```rust impl<'a, 'b> DebugMap<'a, 'b> { pub fn key(&mut self, key: &dyn fmt::Debug) -> &mut DebugMap<'a, 'b> {} pub fn value(&mut self, value: &dyn fmt::Debug) -> &mut DebugMap<'a, 'b> {} } ``` These methods are small and self-contained, and are used as the basis for the existing `DebugMap::entry` method, so have been used in the wild for the last 6 months or so.

KodrAus added 2 commits May 2, 2019 04:10

add key and value methods to DebugMap

8d63666

reword alternative for buffering

7b8549c

KodrAus added A-fmt Proposals relating to std::fmt and formatting macros. T-libs-api Relevant to the library API team, which will review and decide on the RFC. labels May 1, 2019

Centril reviewed May 1, 2019

View reviewed changes

KodrAus mentioned this pull request May 1, 2019

Add key and value methods to DebugMap rust-lang/rust#60458

Merged

KodrAus commented May 2, 2019

View reviewed changes

text/0000-debug-map-key-value.md Outdated Show resolved Hide resolved

update alteratives and reference impl

16db5ae

ollie27 reviewed May 19, 2019

View reviewed changes

panic instead of poisoning

7a29e3f

KodrAus added the I-nominated label May 28, 2019

rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. labels May 28, 2019

KodrAus removed the I-nominated label May 30, 2019

rfcbot added the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label Jun 27, 2019

rfcbot removed the proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. label Jun 27, 2019

KodrAus mentioned this pull request Jul 1, 2019

Tracking issue for kv_unstable (structured logging) rust-lang/log#328

Closed

29 tasks

rfcbot added finished-final-comment-period The final comment period is finished for this RFC. and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels Jul 7, 2019

KodrAus mentioned this pull request Jul 7, 2019

Tracking issue for debug_map_key_value rust-lang/rust#62482

Closed

3 tasks

Update and rename 0000-debug-map-key-value.md to 2696-debug-map-key-v…

d5fea4e

…alue.md

KodrAus merged commit 158eaea into rust-lang:master Jul 7, 2019

KodrAus deleted the debug_map_entry branch July 7, 2019 23:49

KodrAus mentioned this pull request Jan 14, 2020

Stabilize the debug_map_key_value feature rust-lang/rust#68200

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add key and value methods to DebugMap #2696

Add key and value methods to DebugMap #2696

KodrAus commented May 1, 2019 •

edited

Loading

Centril May 1, 2019

KodrAus May 1, 2019

KodrAus May 13, 2019 •

edited

Loading

Centril May 1, 2019

KodrAus May 1, 2019

Centril May 1, 2019

ollie27 May 19, 2019

KodrAus May 19, 2019

KodrAus commented May 26, 2019

KodrAus commented May 28, 2019

alexcrichton commented May 28, 2019

rfcbot commented May 28, 2019 •

edited by withoutboats

Loading

rfcbot commented Jun 27, 2019

rfcbot commented Jul 7, 2019


		- Write an alternative implementation of the format builders. The output from this alternative implementation would need to be kept reasonably in-sync with the one in the standard library. It doesn't change very frequently, but does from time to time. It would also have to take the same care as the standard library implementation to retain formatting flags when working with entries.
		- Buffer keys and format them together with values when the whole entry is available. Unless the key is guaranteed to live until the value is supplied (meaning it probably needs to be `'static`) then the key will need to be formatted into a string first. This means allocating (though the cost could be amortized over the whole map) and potentially losing formatting flags when buffering.

Add key and value methods to DebugMap #2696

Add key and value methods to DebugMap #2696

Conversation

KodrAus commented May 1, 2019 • edited Loading

Centril May 1, 2019

Choose a reason for hiding this comment

KodrAus May 1, 2019

Choose a reason for hiding this comment

KodrAus May 13, 2019 • edited Loading

Choose a reason for hiding this comment

Centril May 1, 2019

Choose a reason for hiding this comment

KodrAus May 1, 2019

Choose a reason for hiding this comment

Centril May 1, 2019

Choose a reason for hiding this comment

ollie27 May 19, 2019

Choose a reason for hiding this comment

KodrAus May 19, 2019

Choose a reason for hiding this comment

KodrAus commented May 26, 2019

KodrAus commented May 28, 2019

alexcrichton commented May 28, 2019

rfcbot commented May 28, 2019 • edited by withoutboats Loading

rfcbot commented Jun 27, 2019

rfcbot commented Jul 7, 2019

KodrAus commented May 1, 2019 •

edited

Loading

KodrAus May 13, 2019 •

edited

Loading

rfcbot commented May 28, 2019 •

edited by withoutboats

Loading