From: Fangrui Song Date: Fri, 12 Jul 2019 04:51:31 +0000 (+0000) Subject: [YAMLIO] Remove trailing spaces when outputting maps X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d6a4ded922547d20fbdcce77138b70b87d867513;p=llvm [YAMLIO] Remove trailing spaces when outputting maps 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. Reviewed By: grimar, jhenderson Differential Revision: https://reviews.llvm.org/D64566 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@365869 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/Support/YAMLTraits.h b/include/llvm/Support/YAMLTraits.h index 2185cd75ae4..5181dc56d81 100644 --- a/include/llvm/Support/YAMLTraits.h +++ b/include/llvm/Support/YAMLTraits.h @@ -1620,8 +1620,9 @@ private: bool NeedBitValueComma = false; bool NeedFlowSequenceComma = false; bool EnumerationMatchFound = false; - bool NeedsNewLine = false; bool WriteDefaultValues = false; + StringRef Padding; + StringRef PaddingBeforeContainer; }; /// YAML I/O does conversion based on types. But often native data types diff --git a/lib/Support/YAMLTraits.cpp b/lib/Support/YAMLTraits.cpp index 183da5b3900..09eb36943de 100644 --- a/lib/Support/YAMLTraits.cpp +++ b/lib/Support/YAMLTraits.cpp @@ -446,7 +446,8 @@ bool Output::outputting() { void Output::beginMapping() { StateStack.push_back(inMapFirstKey); - NeedsNewLine = true; + PaddingBeforeContainer = Padding; + Padding = "\n"; } bool Output::mapTag(StringRef Tag, bool Use) { @@ -474,7 +475,7 @@ bool Output::mapTag(StringRef Tag, bool Use) { } // Tags inside maps in sequences should act as keys in the map from a // formatting perspective, so we always want a newline in a sequence. - NeedsNewLine = true; + Padding = "\n"; } } return Use; @@ -482,8 +483,12 @@ bool Output::mapTag(StringRef Tag, bool Use) { void Output::endMapping() { // If we did not map anything, we should explicitly emit an empty map - if (StateStack.back() == inMapFirstKey) + if (StateStack.back() == inMapFirstKey) { + Padding = PaddingBeforeContainer; + newLineCheck(); output("{}"); + Padding = "\n"; + } StateStack.pop_back(); } @@ -548,14 +553,19 @@ void Output::endDocuments() { unsigned Output::beginSequence() { StateStack.push_back(inSeqFirstElement); - NeedsNewLine = true; + PaddingBeforeContainer = Padding; + Padding = "\n"; return 0; } void Output::endSequence() { // If we did not emit anything, we should explicitly emit an empty sequence - if (StateStack.back() == inSeqFirstElement) + if (StateStack.back() == inSeqFirstElement) { + Padding = PaddingBeforeContainer; + newLineCheck(); output("[]"); + Padding = "\n"; + } StateStack.pop_back(); } @@ -746,7 +756,7 @@ void Output::outputUpToEndOfLine(StringRef s) { output(s); if (StateStack.empty() || (!inFlowSeqAnyElement(StateStack.back()) && !inFlowMapAnyKey(StateStack.back()))) - NeedsNewLine = true; + Padding = "\n"; } void Output::outputNewLine() { @@ -759,11 +769,13 @@ void Output::outputNewLine() { // void Output::newLineCheck() { - if (!NeedsNewLine) + if (Padding != "\n") { + output(Padding); + Padding = {}; return; - NeedsNewLine = false; - + } outputNewLine(); + Padding = {}; if (StateStack.size() == 0) return; @@ -797,9 +809,9 @@ void Output::paddedKey(StringRef key) { output(":"); const char *spaces = " "; if (key.size() < strlen(spaces)) - output(&spaces[key.size()]); + Padding = &spaces[key.size()]; else - output(" "); + Padding = " "; } void Output::flowKey(StringRef Key) { diff --git a/unittests/BinaryFormat/MsgPackDocumentTest.cpp b/unittests/BinaryFormat/MsgPackDocumentTest.cpp index 77b7e29d059..00b157963f6 100644 --- a/unittests/BinaryFormat/MsgPackDocumentTest.cpp +++ b/unittests/BinaryFormat/MsgPackDocumentTest.cpp @@ -127,7 +127,7 @@ TEST(MsgPackDocument, TestOutputYAMLMap) { ASSERT_EQ(OStream.str(), "---\n" "bar: 2\n" "foo: 1\n" - "qux: \n" + "qux:\n" " baz: true\n" "...\n"); } @@ -147,7 +147,7 @@ TEST(MsgPackDocument, TestOutputYAMLMapHex) { ASSERT_EQ(OStream.str(), "---\n" "bar: 0x2\n" "foo: 1\n" - "qux: \n" + "qux:\n" " baz: true\n" "...\n"); } diff --git a/unittests/Support/YAMLIOTest.cpp b/unittests/Support/YAMLIOTest.cpp index ef14352628a..a757040c4e8 100644 --- a/unittests/Support/YAMLIOTest.cpp +++ b/unittests/Support/YAMLIOTest.cpp @@ -2528,7 +2528,7 @@ TEST(YAMLIO, TestMapWithContext) { ostr.flush(); EXPECT_EQ(1, Context.A); EXPECT_EQ("---\n" - "Simple: \n" + "Simple:\n" " B: 0\n" " C: 0\n" " Context: 1\n" @@ -2543,7 +2543,7 @@ TEST(YAMLIO, TestMapWithContext) { ostr.flush(); EXPECT_EQ(2, Context.A); EXPECT_EQ("---\n" - "Simple: \n" + "Simple:\n" " B: 2\n" " C: 3\n" " Context: 2\n" @@ -2556,13 +2556,22 @@ LLVM_YAML_IS_STRING_MAP(int) TEST(YAMLIO, TestCustomMapping) { std::map x; - x["foo"] = 1; - x["bar"] = 2; std::string out; llvm::raw_string_ostream ostr(out); Output xout(ostr, nullptr, 0); + xout << x; + ostr.flush(); + EXPECT_EQ("---\n" + "{}\n" + "...\n", + out); + + x["foo"] = 1; + x["bar"] = 2; + + out.clear(); xout << x; ostr.flush(); EXPECT_EQ("---\n" @@ -2595,10 +2604,10 @@ TEST(YAMLIO, TestCustomMappingStruct) { xout << x; ostr.flush(); EXPECT_EQ("---\n" - "bar: \n" + "bar:\n" " foo: 3\n" " bar: 4\n" - "foo: \n" + "foo:\n" " foo: 1\n" " bar: 2\n" "...\n", @@ -2614,6 +2623,34 @@ TEST(YAMLIO, TestCustomMappingStruct) { EXPECT_EQ(4, y["bar"].bar); } +struct FooBarMapMap { + std::map fbm; +}; + +template <> struct MappingTraits { + static void mapping(IO &io, FooBarMapMap &x) { + io.mapRequired("fbm", x.fbm); + } +}; + +TEST(YAMLIO, TestEmptyMapWrite) { + FooBarMapMap cont; + std::string str; + llvm::raw_string_ostream OS(str); + Output yout(OS); + yout << cont; + EXPECT_EQ(OS.str(), "---\nfbm: {}\n...\n"); +} + +TEST(YAMLIO, TestEmptySequenceWrite) { + FooBarContainer cont; + std::string str; + llvm::raw_string_ostream OS(str); + Output yout(OS); + yout << cont; + EXPECT_EQ(OS.str(), "---\nfbs: []\n...\n"); +} + static void TestEscaped(llvm::StringRef Input, llvm::StringRef Expected) { std::string out; llvm::raw_string_ostream ostr(out); diff --git a/unittests/TextAPI/ELFYAMLTest.cpp b/unittests/TextAPI/ELFYAMLTest.cpp index 9c50a6bee50..8217507b5a5 100644 --- a/unittests/TextAPI/ELFYAMLTest.cpp +++ b/unittests/TextAPI/ELFYAMLTest.cpp @@ -149,7 +149,7 @@ TEST(ElfYamlTextAPI, YAMLWritesTBESymbols) { "--- !tapi-tbe\n" "TbeVersion: 1.0\n" "Arch: AArch64\n" - "Symbols: \n" + "Symbols:\n" " bar: { Type: Func, Weak: true }\n" " foo: { Type: NoType, Size: 99, Warning: Does nothing }\n" " nor: { Type: Func, Undefined: true }\n" @@ -205,7 +205,7 @@ TEST(ElfYamlTextAPI, YAMLWritesNoTBESyms) { "TbeVersion: 1.0\n" "SoName: nosyms.so\n" "Arch: x86_64\n" - "NeededLibs: \n" + "NeededLibs:\n" " - libc.so\n" " - libfoo.so\n" " - libbar.so\n" diff --git a/unittests/TextAPI/TextStubV1Tests.cpp b/unittests/TextAPI/TextStubV1Tests.cpp index 5ff15b9fb8d..165e58f09d5 100644 --- a/unittests/TextAPI/TextStubV1Tests.cpp +++ b/unittests/TextAPI/TextStubV1Tests.cpp @@ -159,7 +159,7 @@ TEST(TBDv1, WriteFile) { "compatibility-version: 0\n" "swift-version: 5\n" "objc-constraint: retain_release\n" - "exports: \n" + "exports:\n" " - archs: [ i386 ]\n" " symbols: [ _sym1 ]\n" " weak-def-symbols: [ _sym2 ]\n" diff --git a/unittests/TextAPI/TextStubV2Tests.cpp b/unittests/TextAPI/TextStubV2Tests.cpp index f380f7f362d..b4b8ca9b91e 100644 --- a/unittests/TextAPI/TextStubV2Tests.cpp +++ b/unittests/TextAPI/TextStubV2Tests.cpp @@ -182,7 +182,7 @@ TEST(TBDv2, WriteFile) { "current-version: 1.2.3\n" "compatibility-version: 0\n" "swift-version: 5\n" - "exports: \n" + "exports:\n" " - archs: [ i386 ]\n" " symbols: [ _sym1 ]\n" " weak-def-symbols: [ _sym2 ]\n"