Skip to content

Commit 0059411

Browse files
authored
Rollup merge of #65237 - KodrAus:fix/map-entry-err, r=sfackler
Move debug_map assertions after check for err Fixes #65231 We have some assertions in `DebugMap` to catch broken implementations of `Debug` that produce malformed entries. These checks don't make sense if formatting fails partway through. This PR moves those assertions to within the `and_then` closures along with the other formatting logic, so they're only checked if the map hasn't failed to format an entry already.
2 parents a16dca3 + 959a6c1 commit 0059411

File tree

2 files changed

+49
-7
lines changed

2 files changed

+49
-7
lines changed

src/libcore/fmt/builders.rs

+9-7
Original file line numberDiff line numberDiff line change
@@ -775,10 +775,10 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> {
775775
reason = "recently added",
776776
issue = "62482")]
777777
pub fn key(&mut self, key: &dyn fmt::Debug) -> &mut DebugMap<'a, 'b> {
778-
assert!(!self.has_key, "attempted to begin a new map entry \
779-
without completing the previous one");
780-
781778
self.result = self.result.and_then(|_| {
779+
assert!(!self.has_key, "attempted to begin a new map entry \
780+
without completing the previous one");
781+
782782
if self.is_pretty() {
783783
if !self.has_fields {
784784
self.fmt.write_str("\n")?;
@@ -839,9 +839,9 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> {
839839
reason = "recently added",
840840
issue = "62482")]
841841
pub fn value(&mut self, value: &dyn fmt::Debug) -> &mut DebugMap<'a, 'b> {
842-
assert!(self.has_key, "attempted to format a map value before its key");
843-
844842
self.result = self.result.and_then(|_| {
843+
assert!(self.has_key, "attempted to format a map value before its key");
844+
845845
if self.is_pretty() {
846846
let mut slot = None;
847847
let mut writer = PadAdapter::wrap(&mut self.fmt, &mut slot, &mut self.state);
@@ -924,9 +924,11 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> {
924924
/// ```
925925
#[stable(feature = "debug_builders", since = "1.2.0")]
926926
pub fn finish(&mut self) -> fmt::Result {
927-
assert!(!self.has_key, "attempted to finish a map with a partial entry");
927+
self.result.and_then(|_| {
928+
assert!(!self.has_key, "attempted to finish a map with a partial entry");
928929

929-
self.result.and_then(|_| self.fmt.write_str("}"))
930+
self.fmt.write_str("}")
931+
})
930932
}
931933

932934
fn is_pretty(&self) -> bool {

src/libcore/tests/fmt/builders.rs

+40
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,46 @@ mod debug_map {
319319
format!("{:#?}", Bar));
320320
}
321321

322+
#[test]
323+
fn test_entry_err() {
324+
// Ensure errors in a map entry don't trigger panics (#65231)
325+
use std::fmt::Write;
326+
327+
struct ErrorFmt;
328+
329+
impl fmt::Debug for ErrorFmt {
330+
fn fmt(&self, _: &mut fmt::Formatter<'_>) -> fmt::Result {
331+
Err(fmt::Error)
332+
}
333+
}
334+
335+
struct KeyValue<K, V>(usize, K, V);
336+
337+
impl<K, V> fmt::Debug for KeyValue<K, V>
338+
where
339+
K: fmt::Debug,
340+
V: fmt::Debug,
341+
{
342+
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
343+
let mut map = fmt.debug_map();
344+
345+
for _ in 0..self.0 {
346+
map.entry(&self.1, &self.2);
347+
}
348+
349+
map.finish()
350+
}
351+
}
352+
353+
let mut buf = String::new();
354+
355+
assert!(write!(&mut buf, "{:?}", KeyValue(1, ErrorFmt, "bar")).is_err());
356+
assert!(write!(&mut buf, "{:?}", KeyValue(1, "foo", ErrorFmt)).is_err());
357+
358+
assert!(write!(&mut buf, "{:?}", KeyValue(2, ErrorFmt, "bar")).is_err());
359+
assert!(write!(&mut buf, "{:?}", KeyValue(2, "foo", ErrorFmt)).is_err());
360+
}
361+
322362
#[test]
323363
#[should_panic]
324364
fn test_invalid_key_when_entry_is_incomplete() {

0 commit comments

Comments
 (0)