Make YAML serializer quote forward slashes

[zturner]

Summary

If you have the string /usr/bin our YAML serializer will not quote this, but if you have the string C:\foo\bar it will quote it.

While technically not required by the YAML standard, it's nice if we're consistent on quoting them since / and \ both commonly appear in paths. Otherwise, the same YAML value may be quoted or unquoted depending on what platform a test is run on. This makes for confusing FileCheck statements.

[rnk]

lgtm

Windows paths aside, this is a good change if someone checks out the project inside a directory with spaces, such as "/home/FirstName LastName".

clang/unittests/Tooling/ReplacementsYamlTest.cpp

TEST(ReplacementsYamlTest, serializesReplacements) {
   std::string YamlContent;
   llvm::raw_string_ostream YamlContentStream(YamlContent);
 
   yaml::Output YAML(YamlContentStream);
   YAML << Doc;
 
   // 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"
                "...\n",
                YamlContentStream.str().c_str());
 }
 
 TEST(ReplacementsYamlTest, deserializesReplacements) {

lld/test/mach-o/cstring-sections.yaml

 ...
 
 # CHECK: defined-atoms:
 # 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_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 ]
 # CHECK:     merge:           by-content
 # CHECK:   - scope:           hidden
 # CHECK:     type:            c-string
 # CHECK:     content:         [ 6A, 6B, 6C, 00 ]
 # CHECK:     merge:           by-content

lld/test/mach-o/parse-data-relocs-x86_64.yaml

 # CHECK:    dead-strip:      never
 # CHECK:  - name:            _bar
 # CHECK:    content:         [ C3 ]
 # CHECK:    dead-strip:      never
 # CHECK:  - ref-name:        L003
 # 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
 

lld/test/mach-o/parse-data.yaml

 # CHECK:     type:            zero-fill
 # CHECK:     size:            4
 
 # CHECK:   - name:            _kustom
 # CHECK:     scope:           global
 # 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'
 

lld/test/mach-o/sectcreate.yaml

 # RUN: ld64.lld -r -arch x86_64 -o %t -sectcreate __DATA __data \
 # RUN:   %p/Inputs/hw.raw_bytes -print_atoms | FileCheck %s
 
 # CHECK: --- !native
 # CHECK: path:            '<linker-internal>'
 # CHECK: defined-atoms:
 # CHECK:   - scope:           global
 # 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

llvm/include/llvm/Support/YAMLTraits.h

for (unsigned char C : S) {
     // Alphanum is safe.
     if (isAlnum(C))
       continue;
 
     switch (C) {
     // Safe scalar characters.
     case '_':
     case '-':
-    case '/':
     case '^':
     case '.':
     case ',':
     case ' ':
     // TAB (0x9) is allowed in unquoted strings.
     case 0x9:
       continue;
     // LF(0xA) and CR(0xD) may delimit values and so require at least single
     // quotes.
     case 0xA:
     case 0xD:
       MaxQuotingNeeded = QuotingType::Single;
       continue;
     // 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.
       if (C <= 0x1F)
         return QuotingType::Double;
 
       // Always double quote UTF-8.
       if ((C & 0x80) != 0)

llvm/test/ObjectYAML/MachO/DWARF-BigEndian.yaml

- Code:            0x00000007
       Attributes:      
         - Attribute:       DW_AT_type
           Form:            DW_FORM_ref4       
 ...
 
 #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
 #CHECK:     - __absvdi2
 #CHECK:     - a
 #CHECK:     - N
 #CHECK:     - t
 #CHECK:   debug_abbrev:    

llvm/test/ObjectYAML/MachO/DWARF-LittleEndian.yaml

- Code:            0x00000007
       Attributes:      
         - Attribute:       DW_AT_type
           Form:            DW_FORM_ref4
 ...
 
 #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
 #CHECK:     - __absvdi2
 #CHECK:     - a
 #CHECK:     - N
 #CHECK:     - t
 #CHECK:   debug_abbrev:    

llvm/test/ObjectYAML/MachO/DWARF-debug_str.yaml

debug_str:
     - char
 ...
 
 #CHECK: DWARF:           
 #CHECK:   debug_str:    
 #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
 #CHECK:     - int
 #CHECK:     - char
 

llvm/test/ObjectYAML/MachO/dylib_dylinker_command.yaml

- cmd:             LC_LOAD_DYLIB
     PayloadString:   /usr/lib/libSystem.B.dylib
     ZeroPadBytes:    6
 ...
 
 #CHECK: 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
 #CHECK:     dylib:           
 #CHECK:       name:            24
 #CHECK:       timestamp:       2
 #CHECK:       current_version: 7864576
 #CHECK:       compatibility_version: 65536
 #CHECK:     PayloadString:   '/usr/lib/libc++.1.dylib'
 #CHECK:     ZeroPadBytes:    1
 #CHECK:   - cmd:             LC_LOAD_DYLIB
 #CHECK:     cmdsize:         56
 #CHECK:     dylib:           
 #CHECK:       name:            24
 #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

llvm/test/Other/size-remarks.ll

 ; CGSCC-SAME: [[ORIGFN:[1-9][0-9]*]] to [[FINALFN:[0-9][0-9]*]];
 ; CGSCC-SAME: Delta: [[DELTAFN:-?[1-9][0-9]*]]
 ; CGSCC-NEXT: ---
 ; CGSCC-DAG: !Analysis
 ; CGSCC-NEXT: Pass:            size-info
 ; 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 '
 ; CGSCC-NEXT:  - IRInstrsAfter:   '[[FINAL]]'
 ; CGSCC-NEXT:  - String:          '; Delta: '
 ; CGSCC-NEXT:  - DeltaInstrCount: '[[DELTA]]'
 ; CGSCC-DAG: --- !Analysis
 ; CGSCC-NEXT: Pass:            size-info
 ; 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 '
 ; CGSCC-NEXT:   - IRInstrsBefore:  '[[ORIGFN]]'
 ; CGSCC-NEXT:   - String:          ' to '
 ; CGSCC-NEXT:   - IRInstrsAfter:   '[[FINALFN]]'
 ; CGSCC-NEXT:   - String:          '; Delta: '
 ; CGSCC-NEXT:   - DeltaInstrCount: '[[DELTAFN]]'

llvm/test/Transforms/GVN/opt-remarks.ll

 ; YAML-NEXT:   - Type:            i32
 ; YAML-NEXT:   - String:          ' eliminated'
 ; YAML-NEXT:   - String:          ' in favor of '
 ; YAML-NEXT:   - InfavorOfValue:  load
 ; YAML-NEXT: ...
 ; 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 '
 ; YAML-NEXT:   - Type:            i32
 ; 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) {
 entry:
   store i32 %i, i32* %p
   %load = load i32, i32* %p
   ret i32 %load
 }

llvm/test/Transforms/Inline/optimization-remarks-passed-yaml.ll

 ;  4       return foo();
 ;  5     }
 
 ; CHECK: remark: /tmp/s.c:4:10: foo inlined into bar with (cost={{[0-9\-]+}}, threshold={{[0-9]+}}) (hotness: 30)
 
 ; 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\-]+}}'
 ; YAML-NEXT:   - String: ', threshold='
 ; YAML-NEXT:   - Threshold: '{{[0-9]+}}'
 ; YAML-NEXT:   - String: ')'
 ; YAML-NEXT: ...
 

llvm/test/Transforms/Inline/optimization-remarks-yaml.ll

 ;   6  }
 
 ; CHECK:      remark: /tmp/s.c:5:10: foo will not be inlined into baz because its definition is unavailable (hotness: 30)
 ; CHECK-NEXT: remark: /tmp/s.c:5:18: bar will not be inlined into baz because its definition is unavailable (hotness: 30)
 
 ; 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: ...
 
 ; No remarks should be output, since none meet the threshold.
 ; THRESHOLD-NOT: remark
 
 ; ModuleID = '/tmp/s.c'
 source_filename = "/tmp/s.c"

llvm/unittests/Support/YAMLIOTest.cpp

static void TestEscaped(llvm::StringRef Input, llvm::StringRef Expected) {
   llvm::StringRef Got(out);
   EXPECT_EQ(Expected, Got);
 }
 
 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
   TestEscaped("abc\"fdf", "'abc\"fdf'");
   // Double quote inside double quote
   TestEscaped("\01bc\"fdf", "\"\\x01bc\\\"fdf\"");
   // Single quote inside single quote
   TestEscaped("abc'fdf", "'abc''fdf'");