[llvm] r344359 - Make YAML quote forward slashes.
Zachary Turner via llvm-commits
llvm-commits at lists.llvm.org
Sun Oct 14 19:25:27 PDT 2018
That’s unfortunate, but I’m not sure what the best thing to do here is. Is
there a reason why it’s undesirable for timer names to have quotes? In
theory we should never care, and the only time it should matter is for the
purposes of writing FileCheck lines. If there’s something that depends on a
particular thing not having quotes that seems like a problem.
I’m hesitant to undo this change because the issue of writing
cross-platform FileCheck statements that work with Windows and Posix paths
is definitely going to come up again and I think having consistency in how
we quote paths is important
On Sun, Oct 14, 2018 at 7:03 PM Bob Wilson <bob.wilson at apple.com> wrote:
> Swift uses an LLVM Timer with a name that includes a slash ("Type checking
> / Semantic analysis”). That seems like it ought to be OK, but it now causes
> an assertion failure in the following code in lib/Support/Timer.cpp:
>
> void TimerGroup::printJSONValue(raw_ostream &OS, const PrintRecord &R,
> const char *suffix, double Value) {
> assert(yaml::needsQuotes(Name) == yaml::QuotingType::None &&
> "TimerGroup name should not need quotes");
> assert(yaml::needsQuotes(R.Name) == yaml::QuotingType::None &&
> "Timer name should not need quotes");
> constexpr auto max_digits10 = std::numeric_limits<double>::max_digits10;
> OS << "\t\"time." << Name << '.' << R.Name << suffix
> << "\": " << format("%.*e", max_digits10 - 1, Value);
> }
>
> > On Oct 12, 2018, at 9:31 AM, Zachary Turner via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
> >
> > Author: zturner
> > Date: Fri Oct 12 09:31:20 2018
> > New Revision: 344359
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=344359&view=rev
> > Log:
> > 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
> >
> > Modified:
> > llvm/trunk/include/llvm/Support/YAMLTraits.h
> > llvm/trunk/test/CodeGen/AArch64/arm64-spill-remarks.ll
> > llvm/trunk/test/ObjectYAML/MachO/DWARF-BigEndian.yaml
> > llvm/trunk/test/ObjectYAML/MachO/DWARF-LittleEndian.yaml
> > llvm/trunk/test/ObjectYAML/MachO/DWARF-debug_str.yaml
> > llvm/trunk/test/ObjectYAML/MachO/dylib_dylinker_command.yaml
> > llvm/trunk/test/Other/size-remarks.ll
> > llvm/trunk/test/Transforms/GVN/opt-remarks.ll
> > llvm/trunk/test/Transforms/Inline/optimization-remarks-passed-yaml.ll
> > llvm/trunk/test/Transforms/Inline/optimization-remarks-yaml.ll
> > llvm/trunk/unittests/Support/YAMLIOTest.cpp
> >
> > Modified: llvm/trunk/include/llvm/Support/YAMLTraits.h
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/YAMLTraits.h?rev=344359&r1=344358&r2=344359&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/include/llvm/Support/YAMLTraits.h (original)
> > +++ llvm/trunk/include/llvm/Support/YAMLTraits.h Fri Oct 12 09:31:20 2018
> > @@ -578,7 +578,6 @@ inline QuotingType needsQuotes(StringRef
> > // Safe scalar characters.
> > case '_':
> > case '-':
> > - case '/':
> > case '^':
> > case '.':
> > case ',':
> > @@ -595,6 +594,12 @@ inline QuotingType needsQuotes(StringRef
> > // 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.
> >
> > Modified: llvm/trunk/test/CodeGen/AArch64/arm64-spill-remarks.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/arm64-spill-remarks.ll?rev=344359&r1=344358&r2=344359&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/CodeGen/AArch64/arm64-spill-remarks.ll (original)
> > +++ llvm/trunk/test/CodeGen/AArch64/arm64-spill-remarks.ll Fri Oct 12
> 09:31:20 2018
> > @@ -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:
> >
> > Modified: llvm/trunk/test/ObjectYAML/MachO/DWARF-BigEndian.yaml
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ObjectYAML/MachO/DWARF-BigEndian.yaml?rev=344359&r1=344358&r2=344359&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/ObjectYAML/MachO/DWARF-BigEndian.yaml (original)
> > +++ llvm/trunk/test/ObjectYAML/MachO/DWARF-BigEndian.yaml Fri Oct 12
> 09:31:20 2018
> > @@ -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
> >
> > Modified: llvm/trunk/test/ObjectYAML/MachO/DWARF-LittleEndian.yaml
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ObjectYAML/MachO/DWARF-LittleEndian.yaml?rev=344359&r1=344358&r2=344359&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/ObjectYAML/MachO/DWARF-LittleEndian.yaml (original)
> > +++ llvm/trunk/test/ObjectYAML/MachO/DWARF-LittleEndian.yaml Fri Oct 12
> 09:31:20 2018
> > @@ -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
> >
> > Modified: llvm/trunk/test/ObjectYAML/MachO/DWARF-debug_str.yaml
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ObjectYAML/MachO/DWARF-debug_str.yaml?rev=344359&r1=344358&r2=344359&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/ObjectYAML/MachO/DWARF-debug_str.yaml (original)
> > +++ llvm/trunk/test/ObjectYAML/MachO/DWARF-debug_str.yaml Fri Oct 12
> 09:31:20 2018
> > @@ -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
> >
> > Modified: llvm/trunk/test/ObjectYAML/MachO/dylib_dylinker_command.yaml
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ObjectYAML/MachO/dylib_dylinker_command.yaml?rev=344359&r1=344358&r2=344359&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/ObjectYAML/MachO/dylib_dylinker_command.yaml
> (original)
> > +++ llvm/trunk/test/ObjectYAML/MachO/dylib_dylinker_command.yaml Fri Oct
> 12 09:31:20 2018
> > @@ -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
> >
> > Modified: llvm/trunk/test/Other/size-remarks.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/size-remarks.ll?rev=344359&r1=344358&r2=344359&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/Other/size-remarks.ll (original)
> > +++ llvm/trunk/test/Other/size-remarks.ll Fri Oct 12 09:31:20 2018
> > @@ -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 '
> >
> > Modified: llvm/trunk/test/Transforms/GVN/opt-remarks.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/opt-remarks.ll?rev=344359&r1=344358&r2=344359&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/Transforms/GVN/opt-remarks.ll (original)
> > +++ llvm/trunk/test/Transforms/GVN/opt-remarks.ll Fri Oct 12 09:31:20
> 2018
> > @@ -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) {
> >
> > Modified:
> llvm/trunk/test/Transforms/Inline/optimization-remarks-passed-yaml.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/optimization-remarks-passed-yaml.ll?rev=344359&r1=344358&r2=344359&view=diff
> >
> ==============================================================================
> > ---
> llvm/trunk/test/Transforms/Inline/optimization-remarks-passed-yaml.ll
> (original)
> > +++
> llvm/trunk/test/Transforms/Inline/optimization-remarks-passed-yaml.ll Fri
> Oct 12 09:31:20 2018
> > @@ -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\-]+}}'
> >
> > Modified: llvm/trunk/test/Transforms/Inline/optimization-remarks-yaml.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/optimization-remarks-yaml.ll?rev=344359&r1=344358&r2=344359&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/Transforms/Inline/optimization-remarks-yaml.ll
> (original)
> > +++ llvm/trunk/test/Transforms/Inline/optimization-remarks-yaml.ll Fri
> Oct 12 09:31:20 2018
> > @@ -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: ...
> >
> >
> > Modified: llvm/trunk/unittests/Support/YAMLIOTest.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/YAMLIOTest.cpp?rev=344359&r1=344358&r2=344359&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/unittests/Support/YAMLIOTest.cpp (original)
> > +++ llvm/trunk/unittests/Support/YAMLIOTest.cpp Fri Oct 12 09:31:20 2018
> > @@ -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 at abc@", "\"\\x01 at abc@\"");
> > // Double quote inside single quote
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181014/8623fc3e/attachment.html>
More information about the llvm-commits
mailing list