This is an archive of the discontinued LLVM Phabricator instance.

[YAMLIO] Remove trailing spaces when outputting maps
ClosedPublic

Authored by MaskRay on Jul 11 2019, 6:52 AM.

Details

Summary

llvm::yaml::Output::paddedKey unconditionally outputs spaces, which
are superfluous if the value to be dumped is a sequence or map.
Change bool NeedsNewLine to StringRef Padding so that it can be
overridden to \n if the value is a sequence or map.

An empty map/sequence is special. It is printed as {} or [] without
a newline, while a non-empty map/sequence follows a newline. To handle
this distinction, add another variable PaddingBeforeContainer and does
the special handling in endMapping/endSequence.

Diff Detail

Event Timeline

MaskRay created this revision.Jul 11 2019, 6:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2019, 6:52 AM
jhenderson added inline comments.Jul 11 2019, 7:11 AM
lib/Support/YAMLTraits.cpp
774

Looks like you could move this to before the if?

grimar added inline comments.Jul 11 2019, 7:12 AM
lib/Support/YAMLTraits.cpp
779

This seems can be a bit simpler:

Out << Padding;
Padding = "";
if (Padding != "\n")
  return;

Column = 0;
779

Ah, no. My mistake.

grimar accepted this revision.EditedJul 11 2019, 7:15 AM

Looks fine, but someone else should also confirm (I am not really familar with the code here).

This revision is now accepted and ready to land.Jul 11 2019, 7:15 AM
MaskRay marked 4 inline comments as done.Jul 11 2019, 7:17 AM
MaskRay added inline comments.
lib/Support/YAMLTraits.cpp
779

Looks like you could move this to before the if?

I can't..

grimar added inline comments.Jul 11 2019, 7:18 AM
lib/Support/YAMLTraits.cpp
779

Interesting that James seems tried to suggest the same. Looks there is something in this code pattern that make people think it can be simplified in this way..

jhenderson accepted this revision.Jul 11 2019, 7:31 AM

LGTM, I think too.

lib/Support/YAMLTraits.cpp
779

Yup. My bad too!

MaskRay updated this revision to Diff 209242.Jul 11 2019, 9:08 AM
MaskRay marked an inline comment as done.
MaskRay edited the summary of this revision. (Show Details)

Fix output of empty sequence/map

MaskRay updated this revision to Diff 209413.Jul 11 2019, 9:50 PM
MaskRay edited the summary of this revision. (Show Details)

Update tests

This revision was automatically updated to reflect the committed changes.