zturner added inline comments.

================
Comment at: packages/Python/lldbsuite/test/lldbtest.py:2230-2233
+        # In Python 2, communicate sends byte strings. In Python 3, 
communicate sends bytes.
+        # If we got a string (and not a byte string), encode it before sending.
+        if isinstance(output, str) and not isinstance(output, bytes):
+            output = output.encode()
----------------
Ok I had to stare at this for a long time to figure out what was going on.  I 
think you need to update the comment here, because it makes it sounds like 
`output` is the result of a `Popen.communicate`.  But `output` is the result of 
a `debugger.HandleCommand()`.  

I think we should actually just leave this the way it was and fix it in the 
call to `Popen` (see below)


================
Comment at: packages/Python/lldbsuite/test/lldbtest.py:2247-2249
         subproc = Popen(filecheck_args, stdin=PIPE, stdout=PIPE, stderr=PIPE)
         cmd_stdout, cmd_stderr = subproc.communicate(input=output)
         cmd_status = subproc.returncode
----------------
If I'm not mistaken, Python 3 can accept `stdin` as `bytes` or `string`, and 
either will work, it depends on the value of `universal_newlines`, and `stdout` 
will be in whatever format `stdin` was in.  If we pass 
`universal_newlines=True`, then it will expect a `string` and output a 
`string`, otherwise it expects a `bytes` and output a `bytes`.  Given that 
`FileCheck` is supposed to operate on textual data and not binary data, perhaps 
we should be doing that here?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53166



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to