From 9f169afab22b7334739c9de0ea1a1ab1ee0c357e Mon Sep 17 00:00:00 2001 From: Zachary Turner Date: Fri, 12 Oct 2018 16:31:20 +0000 Subject: [PATCH] Make YAML quote forward slashes. If you have the string /usr/bin, prior to this patch it would not be quoted by our YAML serializer. But a string like C:\src would be, due to the presence of a backslash. This makes the quoting rules of basically every single file path different depending on the path syntax (posix vs. Windows). While technically not required by the YAML specification to quote forward slashes, when the behavior of paths is inconsistent it makes it difficult to portably write FileCheck lines that will work with either kind of path. Differential Revision: https://reviews.llvm.org/D53169 llvm-svn: 344359 --- clang/unittests/Tooling/DiagnosticsYamlTest.cpp | 12 ++++++------ clang/unittests/Tooling/ReplacementsYamlTest.cpp | 6 +++--- lld/test/mach-o/cstring-sections.yaml | 8 ++++---- lld/test/mach-o/parse-data-relocs-x86_64.yaml | 2 +- lld/test/mach-o/parse-data.yaml | 2 +- lld/test/mach-o/sectcreate.yaml | 2 +- llvm/include/llvm/Support/YAMLTraits.h | 7 ++++++- llvm/test/CodeGen/AArch64/arm64-spill-remarks.ll | 8 ++++---- llvm/test/ObjectYAML/MachO/DWARF-BigEndian.yaml | 4 ++-- llvm/test/ObjectYAML/MachO/DWARF-LittleEndian.yaml | 4 ++-- llvm/test/ObjectYAML/MachO/DWARF-debug_str.yaml | 2 +- llvm/test/ObjectYAML/MachO/dylib_dylinker_command.yaml | 4 ++-- llvm/test/Other/size-remarks.ll | 4 ++-- llvm/test/Transforms/GVN/opt-remarks.ll | 6 +++--- .../Transforms/Inline/optimization-remarks-passed-yaml.ll | 6 +++--- llvm/test/Transforms/Inline/optimization-remarks-yaml.ll | 8 ++++---- llvm/unittests/Support/YAMLIOTest.cpp | 4 +++- 17 files changed, 48 insertions(+), 41 deletions(-) diff --git a/clang/unittests/Tooling/DiagnosticsYamlTest.cpp b/clang/unittests/Tooling/DiagnosticsYamlTest.cpp index 83e09ea..18284bd 100644 --- a/clang/unittests/Tooling/DiagnosticsYamlTest.cpp +++ b/clang/unittests/Tooling/DiagnosticsYamlTest.cpp @@ -58,30 +58,30 @@ TEST(DiagnosticsYamlTest, serializesDiagnostics) { YAML << TUD; EXPECT_EQ("---\n" - "MainSourceFile: path/to/source.cpp\n" + "MainSourceFile: 'path/to/source.cpp'\n" "Diagnostics: \n" " - DiagnosticName: 'diagnostic#1\'\n" " Message: 'message #1'\n" " FileOffset: 55\n" - " FilePath: path/to/source.cpp\n" + " FilePath: 'path/to/source.cpp'\n" " Replacements: \n" - " - FilePath: path/to/source.cpp\n" + " - FilePath: 'path/to/source.cpp'\n" " Offset: 100\n" " Length: 12\n" " ReplacementText: 'replacement #1'\n" " - DiagnosticName: 'diagnostic#2'\n" " Message: 'message #2'\n" " FileOffset: 60\n" - " FilePath: path/to/header.h\n" + " FilePath: 'path/to/header.h'\n" " Replacements: \n" - " - FilePath: path/to/header.h\n" + " - FilePath: 'path/to/header.h'\n" " Offset: 62\n" " Length: 2\n" " ReplacementText: 'replacement #2'\n" " - DiagnosticName: 'diagnostic#3'\n" " Message: 'message #3'\n" " FileOffset: 72\n" - " FilePath: path/to/source2.cpp\n" + " FilePath: 'path/to/source2.cpp'\n" " Replacements: \n" "...\n", YamlContentStream.str()); diff --git a/clang/unittests/Tooling/ReplacementsYamlTest.cpp b/clang/unittests/Tooling/ReplacementsYamlTest.cpp index 3e4193d..2e5a87a 100644 --- a/clang/unittests/Tooling/ReplacementsYamlTest.cpp +++ b/clang/unittests/Tooling/ReplacementsYamlTest.cpp @@ -33,13 +33,13 @@ TEST(ReplacementsYamlTest, serializesReplacements) { // NOTE: If this test starts to fail for no obvious reason, check whitespace. ASSERT_STREQ("---\n" - "MainSourceFile: /path/to/source.cpp\n" + "MainSourceFile: '/path/to/source.cpp'\n" "Replacements: \n" // Extra whitespace here! - " - FilePath: /path/to/file1.h\n" + " - FilePath: '/path/to/file1.h'\n" " Offset: 232\n" " Length: 56\n" " ReplacementText: 'replacement #1'\n" - " - FilePath: /path/to/file2.h\n" + " - FilePath: '/path/to/file2.h'\n" " Offset: 301\n" " Length: 2\n" " ReplacementText: 'replacement #2'\n" diff --git a/lld/test/mach-o/cstring-sections.yaml b/lld/test/mach-o/cstring-sections.yaml index 2bc7e7c..2936ca9 100644 --- a/lld/test/mach-o/cstring-sections.yaml +++ b/lld/test/mach-o/cstring-sections.yaml @@ -36,25 +36,25 @@ sections: # CHECK: content: [ 61, 62, 63, 00 ] # CHECK: merge: by-content # CHECK: section-choice: custom-required -# CHECK: section-name: __TEXT/__objc_methname +# CHECK: section-name: '__TEXT/__objc_methname' # CHECK: - scope: hidden # CHECK: type: c-string # CHECK: content: [ 64, 65, 66, 00 ] # CHECK: merge: by-content # CHECK: section-choice: custom-required -# CHECK: section-name: __TEXT/__objc_methname +# CHECK: section-name: '__TEXT/__objc_methname' # CHECK: - scope: hidden # CHECK: type: c-string # CHECK: content: [ 61, 62, 63, 00 ] # CHECK: merge: by-content # CHECK: section-choice: custom-required -# CHECK: section-name: __TEXT/__objc_classname +# CHECK: section-name: '__TEXT/__objc_classname' # CHECK: - scope: hidden # CHECK: type: c-string # CHECK: content: [ 67, 68, 69, 00 ] # CHECK: merge: by-content # CHECK: section-choice: custom-required -# CHECK: section-name: __TEXT/__objc_classname +# CHECK: section-name: '__TEXT/__objc_classname' # CHECK: - scope: hidden # CHECK: type: c-string # CHECK: content: [ 61, 62, 63, 00 ] diff --git a/lld/test/mach-o/parse-data-relocs-x86_64.yaml b/lld/test/mach-o/parse-data-relocs-x86_64.yaml index d696aff..994289b 100644 --- a/lld/test/mach-o/parse-data-relocs-x86_64.yaml +++ b/lld/test/mach-o/parse-data-relocs-x86_64.yaml @@ -367,6 +367,6 @@ page-size: 0x00000000 # CHECK: type: unknown # CHECK: content: [ 00, 00, 00, 00, 00, 00, 00, 00 ] # CHECK: section-choice: custom-required -# CHECK: section-name: __DATA/__custom +# CHECK: section-name: '__DATA/__custom' # CHECK: dead-strip: never diff --git a/lld/test/mach-o/parse-data.yaml b/lld/test/mach-o/parse-data.yaml index b1929ef..3b9eb9f 100644 --- a/lld/test/mach-o/parse-data.yaml +++ b/lld/test/mach-o/parse-data.yaml @@ -115,5 +115,5 @@ global-symbols: # CHECK: type: unknown # CHECK: content: [ 01, 02, 03, 04, 05, 06, 07, 08 ] # CHECK: section-choice: custom-required -# CHECK: section-name: __CUST/__custom +# CHECK: section-name: '__CUST/__custom' diff --git a/lld/test/mach-o/sectcreate.yaml b/lld/test/mach-o/sectcreate.yaml index aa32a05..bc17dec 100644 --- a/lld/test/mach-o/sectcreate.yaml +++ b/lld/test/mach-o/sectcreate.yaml @@ -8,5 +8,5 @@ # CHECK: type: sectcreate # CHECK: content: [ 68, 65, 6C, 6C, 6F, 0A ] # CHECK: section-choice: custom-required -# CHECK: section-name: __DATA/__data +# CHECK: section-name: '__DATA/__data' # CHECK: dead-strip: never diff --git a/llvm/include/llvm/Support/YAMLTraits.h b/llvm/include/llvm/Support/YAMLTraits.h index 5d029ad..6219755e 100644 --- a/llvm/include/llvm/Support/YAMLTraits.h +++ b/llvm/include/llvm/Support/YAMLTraits.h @@ -578,7 +578,6 @@ inline QuotingType needsQuotes(StringRef S) { // Safe scalar characters. case '_': case '-': - case '/': case '^': case '.': case ',': @@ -595,6 +594,12 @@ inline QuotingType needsQuotes(StringRef S) { // DEL (0x7F) are excluded from the allowed character range. case 0x7F: return QuotingType::Double; + // Forward slash is allowed to be unquoted, but we quote it anyway. We have + // many tests that use FileCheck against YAML output, and this output often + // contains paths. If we quote backslashes but not forward slashes then + // paths will come out either quoted or unquoted depending on which platform + // the test is run on, making FileCheck comparisons difficult. + case '/': default: { // C0 control block (0x0 - 0x1F) is excluded from the allowed character // range. diff --git a/llvm/test/CodeGen/AArch64/arm64-spill-remarks.ll b/llvm/test/CodeGen/AArch64/arm64-spill-remarks.ll index 53a16ed..2d187a7 100644 --- a/llvm/test/CodeGen/AArch64/arm64-spill-remarks.ll +++ b/llvm/test/CodeGen/AArch64/arm64-spill-remarks.ll @@ -38,7 +38,7 @@ ; YAML: --- !Missed ; YAML: Pass: regalloc ; YAML: Name: LoopSpillReload -; YAML: DebugLoc: { File: /tmp/kk.c, Line: 3, Column: 20 } +; YAML: DebugLoc: { File: '/tmp/kk.c', Line: 3, Column: 20 } ; YAML: Function: fpr128 ; YAML: Hotness: 300 ; YAML: Args: @@ -51,7 +51,7 @@ ; YAML: --- !Missed ; YAML: Pass: regalloc ; YAML: Name: LoopSpillReload -; YAML: DebugLoc: { File: /tmp/kk.c, Line: 2, Column: 20 } +; YAML: DebugLoc: { File: '/tmp/kk.c', Line: 2, Column: 20 } ; YAML: Function: fpr128 ; YAML: Hotness: 30000 ; YAML: Args: @@ -64,7 +64,7 @@ ; YAML: --- !Missed ; YAML: Pass: regalloc ; YAML: Name: LoopSpillReload -; YAML: DebugLoc: { File: /tmp/kk.c, Line: 1, Column: 20 } +; YAML: DebugLoc: { File: '/tmp/kk.c', Line: 1, Column: 20 } ; YAML: Function: fpr128 ; YAML: Hotness: 300 ; YAML: Args: @@ -79,7 +79,7 @@ ; THRESHOLD_YAML: --- !Missed ; THRESHOLD_YAML: Pass: regalloc ; THRESHOLD_YAML: Name: LoopSpillReload -; THRESHOLD_YAML: DebugLoc: { File: /tmp/kk.c, Line: 2, Column: 20 } +; THRESHOLD_YAML: DebugLoc: { File: '/tmp/kk.c', Line: 2, Column: 20 } ; THRESHOLD_YAML: Function: fpr128 ; THRESHOLD_YAML: Hotness: 30000 ; THRESHOLD_YAML: Args: diff --git a/llvm/test/ObjectYAML/MachO/DWARF-BigEndian.yaml b/llvm/test/ObjectYAML/MachO/DWARF-BigEndian.yaml index adc95b9..c6a45cd 100644 --- a/llvm/test/ObjectYAML/MachO/DWARF-BigEndian.yaml +++ b/llvm/test/ObjectYAML/MachO/DWARF-BigEndian.yaml @@ -376,8 +376,8 @@ DWARF: #CHECK: DWARF: #CHECK: debug_str: #CHECK: - 'clang version 4.0.0 (trunk 290181) (llvm/trunk 290209)' -#CHECK: - ../compiler-rt/lib/builtins/absvdi2.c -#CHECK: - /Users/cbieneman/dev/open-source/llvm-build-rel +#CHECK: - '../compiler-rt/lib/builtins/absvdi2.c' +#CHECK: - '/Users/cbieneman/dev/open-source/llvm-build-rel' #CHECK: - int #CHECK: - di_int #CHECK: - long long int diff --git a/llvm/test/ObjectYAML/MachO/DWARF-LittleEndian.yaml b/llvm/test/ObjectYAML/MachO/DWARF-LittleEndian.yaml index 1d6da66..1e136e6 100644 --- a/llvm/test/ObjectYAML/MachO/DWARF-LittleEndian.yaml +++ b/llvm/test/ObjectYAML/MachO/DWARF-LittleEndian.yaml @@ -365,8 +365,8 @@ DWARF: #CHECK: DWARF: #CHECK: debug_str: #CHECK: - 'clang version 4.0.0 (trunk 290181) (llvm/trunk 290209)' -#CHECK: - ../compiler-rt/lib/builtins/absvdi2.c -#CHECK: - /Users/cbieneman/dev/open-source/llvm-build-rel +#CHECK: - '../compiler-rt/lib/builtins/absvdi2.c' +#CHECK: - '/Users/cbieneman/dev/open-source/llvm-build-rel' #CHECK: - int #CHECK: - di_int #CHECK: - long long int diff --git a/llvm/test/ObjectYAML/MachO/DWARF-debug_str.yaml b/llvm/test/ObjectYAML/MachO/DWARF-debug_str.yaml index 417a755..84c5e22 100644 --- a/llvm/test/ObjectYAML/MachO/DWARF-debug_str.yaml +++ b/llvm/test/ObjectYAML/MachO/DWARF-debug_str.yaml @@ -257,7 +257,7 @@ DWARF: #CHECK: - '' #CHECK: - 'clang version 4.0.0 (trunk 288677) (llvm/trunk 288676)' #CHECK: - hello_world.c -#CHECK: - /Users/cbieneman/dev/open-source/llvm-build-rel +#CHECK: - '/Users/cbieneman/dev/open-source/llvm-build-rel' #CHECK: - main #CHECK: - argc #CHECK: - argv diff --git a/llvm/test/ObjectYAML/MachO/dylib_dylinker_command.yaml b/llvm/test/ObjectYAML/MachO/dylib_dylinker_command.yaml index 9184e3c..5fc6afa53 100644 --- a/llvm/test/ObjectYAML/MachO/dylib_dylinker_command.yaml +++ b/llvm/test/ObjectYAML/MachO/dylib_dylinker_command.yaml @@ -40,7 +40,7 @@ LoadCommands: #CHECK: - cmd: LC_LOAD_DYLINKER #CHECK: cmdsize: 32 #CHECK: name: 12 -#CHECK: PayloadString: /usr/lib/dyld +#CHECK: PayloadString: '/usr/lib/dyld' #CHECK: ZeroPadBytes: 7 #CHECK: - cmd: LC_LOAD_DYLIB #CHECK: cmdsize: 48 @@ -58,5 +58,5 @@ LoadCommands: #CHECK: timestamp: 2 #CHECK: current_version: 80349697 #CHECK: compatibility_version: 65536 -#CHECK: PayloadString: /usr/lib/libSystem.B.dylib +#CHECK: PayloadString: '/usr/lib/libSystem.B.dylib' #CHECK: ZeroPadBytes: 6 diff --git a/llvm/test/Other/size-remarks.ll b/llvm/test/Other/size-remarks.ll index 34cb120..1e96dd0 100644 --- a/llvm/test/Other/size-remarks.ll +++ b/llvm/test/Other/size-remarks.ll @@ -32,7 +32,7 @@ ; CGSCC-NEXT: Name: IRSizeChange ; CGSCC-NEXT: Function: ; CGSCC-NEXT: Args: -; CGSCC-NEXT: - Pass: Function Integration/Inlining +; CGSCC-NEXT: - Pass: 'Function Integration/Inlining' ; CGSCC-NEXT: - String: ': IR instruction count changed from ' ; CGSCC-NEXT: - IRInstrsBefore: '[[ORIG]]' ; CGSCC-NEXT: - String: ' to ' @@ -44,7 +44,7 @@ ; CGSCC-NEXT: Name: FunctionIRSizeChange ; CGSCC-NEXT: Function: ; CGSCC-NEXT: Args: -; CGSCC-NEXT: - Pass: Function Integration/Inlining +; CGSCC-NEXT: - Pass: 'Function Integration/Inlining' ; CGSCC-NEXT: - String: ': Function: ' ; CGSCC-NEXT: - Function: bar ; CGSCC-NEXT: - String: ': IR instruction count changed from ' diff --git a/llvm/test/Transforms/GVN/opt-remarks.ll b/llvm/test/Transforms/GVN/opt-remarks.ll index 6919528..120ff36 100644 --- a/llvm/test/Transforms/GVN/opt-remarks.ll +++ b/llvm/test/Transforms/GVN/opt-remarks.ll @@ -49,7 +49,7 @@ ; YAML-NEXT: --- !Missed ; YAML-NEXT: Pass: gvn ; YAML-NEXT: Name: LoadClobbered -; YAML-NEXT: DebugLoc: { File: /tmp/s.c, Line: 3, Column: 3 } +; YAML-NEXT: DebugLoc: { File: '/tmp/s.c', Line: 3, Column: 3 } ; YAML-NEXT: Function: may_alias ; YAML-NEXT: Args: ; YAML-NEXT: - String: 'load of type ' @@ -57,10 +57,10 @@ ; YAML-NEXT: - String: ' not eliminated' ; YAML-NEXT: - String: ' in favor of ' ; YAML-NEXT: - OtherAccess: load -; YAML-NEXT: DebugLoc: { File: /tmp/s.c, Line: 1, Column: 13 } +; YAML-NEXT: DebugLoc: { File: '/tmp/s.c', Line: 1, Column: 13 } ; YAML-NEXT: - String: ' because it is clobbered by ' ; YAML-NEXT: - ClobberedBy: store -; YAML-NEXT: DebugLoc: { File: /tmp/s.c, Line: 2, Column: 10 } +; YAML-NEXT: DebugLoc: { File: '/tmp/s.c', Line: 2, Column: 10 } ; YAML-NEXT: ... define i32 @arg(i32* %p, i32 %i) { diff --git a/llvm/test/Transforms/Inline/optimization-remarks-passed-yaml.ll b/llvm/test/Transforms/Inline/optimization-remarks-passed-yaml.ll index 0ac7635..8692abf 100644 --- a/llvm/test/Transforms/Inline/optimization-remarks-passed-yaml.ll +++ b/llvm/test/Transforms/Inline/optimization-remarks-passed-yaml.ll @@ -22,15 +22,15 @@ ; YAML: --- !Passed ; YAML-NEXT: Pass: inline ; YAML-NEXT: Name: Inlined -; YAML-NEXT: DebugLoc: { File: /tmp/s.c, Line: 4, Column: 10 } +; YAML-NEXT: DebugLoc: { File: '/tmp/s.c', Line: 4, Column: 10 } ; YAML-NEXT: Function: bar ; YAML-NEXT: Hotness: 30 ; YAML-NEXT: Args: ; YAML-NEXT: - Callee: foo -; YAML-NEXT: DebugLoc: { File: /tmp/s.c, Line: 1, Column: 0 } +; YAML-NEXT: DebugLoc: { File: '/tmp/s.c', Line: 1, Column: 0 } ; YAML-NEXT: - String: ' inlined into ' ; YAML-NEXT: - Caller: bar -; YAML-NEXT: DebugLoc: { File: /tmp/s.c, Line: 3, Column: 0 } +; YAML-NEXT: DebugLoc: { File: '/tmp/s.c', Line: 3, Column: 0 } ; YAML-NEXT: - String: ' with ' ; YAML-NEXT: - String: '(cost=' ; YAML-NEXT: - Cost: '{{[0-9\-]+}}' diff --git a/llvm/test/Transforms/Inline/optimization-remarks-yaml.ll b/llvm/test/Transforms/Inline/optimization-remarks-yaml.ll index cb366db..10a93f5 100644 --- a/llvm/test/Transforms/Inline/optimization-remarks-yaml.ll +++ b/llvm/test/Transforms/Inline/optimization-remarks-yaml.ll @@ -52,27 +52,27 @@ ; YAML: --- !Missed ; YAML-NEXT: Pass: inline ; YAML-NEXT: Name: NoDefinition -; YAML-NEXT: DebugLoc: { File: /tmp/s.c, Line: 5, Column: 10 } +; YAML-NEXT: DebugLoc: { File: '/tmp/s.c', Line: 5, Column: 10 } ; YAML-NEXT: Function: baz ; YAML-NEXT: Hotness: 30 ; YAML-NEXT: Args: ; YAML-NEXT: - Callee: foo ; YAML-NEXT: - String: ' will not be inlined into ' ; YAML-NEXT: - Caller: baz -; YAML-NEXT: DebugLoc: { File: /tmp/s.c, Line: 4, Column: 0 } +; YAML-NEXT: DebugLoc: { File: '/tmp/s.c', Line: 4, Column: 0 } ; YAML-NEXT: - String: ' because its definition is unavailable' ; YAML-NEXT: ... ; YAML-NEXT: --- !Missed ; YAML-NEXT: Pass: inline ; YAML-NEXT: Name: NoDefinition -; YAML-NEXT: DebugLoc: { File: /tmp/s.c, Line: 5, Column: 18 } +; YAML-NEXT: DebugLoc: { File: '/tmp/s.c', Line: 5, Column: 18 } ; YAML-NEXT: Function: baz ; YAML-NEXT: Hotness: 30 ; YAML-NEXT: Args: ; YAML-NEXT: - Callee: bar ; YAML-NEXT: - String: ' will not be inlined into ' ; YAML-NEXT: - Caller: baz -; YAML-NEXT: DebugLoc: { File: /tmp/s.c, Line: 4, Column: 0 } +; YAML-NEXT: DebugLoc: { File: '/tmp/s.c', Line: 4, Column: 0 } ; YAML-NEXT: - String: ' because its definition is unavailable' ; YAML-NEXT: ... diff --git a/llvm/unittests/Support/YAMLIOTest.cpp b/llvm/unittests/Support/YAMLIOTest.cpp index 4530482..94e9874 100644 --- a/llvm/unittests/Support/YAMLIOTest.cpp +++ b/llvm/unittests/Support/YAMLIOTest.cpp @@ -2543,7 +2543,9 @@ TEST(YAMLIO, TestEscaped) { // Single quote TestEscaped("@abc@", "'@abc@'"); // No quote - TestEscaped("abc/", "abc/"); + TestEscaped("abc", "abc"); + // Forward slash quoted + TestEscaped("abc/", "'abc/'"); // Double quote non-printable TestEscaped("\01@abc@", "\"\\x01@abc@\""); // Double quote inside single quote -- 2.7.4