[PATCH] D37946: [lit] Fix some Python 3 compatibility issues.

[David L. Jones via Phabricator via llvm-commits]

dlj accepted this revision.
dlj added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/utils/lit/lit/TestRunner.py:286
+
+        arg = lit.util.to_bytes(arg)
+        codec = 'string_escape' if sys.version_info < (3,0) else 'unicode_escape'
----------------
zturner wrote:
> dlj wrote:
> > So I have to admit that here, I'm not sure exactly what corner cases are problematic... but would lit.util.to_string give you the correct result?
> No, because `to_string` literally just converts from bytes to string.  `string_escape` is kind of a non-standard codec.  It literally just adds or removes backslashes as necessary in order to convert between a python string literal and the actual value.  For example, if you encode `a\nb` using `string_escape`, then you're saying "I have the actual string literal `a\nb`, give me something that I could use to write that string literal in Python.  So it returns `a\\nb`.
> 
> On the other hand, if you //decode// `a\nb` using the `string_escape` codec, you're telling it that the string is already escaped, and you want to know what it would look like if printed.  So it returns 
> 
> ```
> a
> b
> ```
> 
> We need this logic here because we're literally parsing a command line, and we're mimicing the shell's escaping rules.  This way a person can write `echo "foo\tbar"` and it will actually print `foo<tab>bar`
> 
> (Hopefully I got that explanation right)
OK, that makes sense.

The to_string conversion doesn't //quite// do a straightforward conversion, but IIRC returns some sort of a repr-like syntax if the input isn't otherwise-valid UTF-8. So your choice here seems better in this case.

I also seem to recall that string_escape only escapes double quotes, not single quotes (since its purpose is to generate legal strings for C). But I don't think that's an issue here.


https://reviews.llvm.org/D37946