[PATCH] D47697: [llvm-mca] Make sure not to end the test files with an empty line.

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

@gbedwell thank you for taking a look.
I forgot to check `git shortlog` of the script itself, thus did not add you as a reviewer :)



================
Comment at: utils/update_mca_test_checks.py:445
+  # The file should not end with two newlines. It creates unnecessary churn.
+  while len(output_lines) > 0 and output_lines[-1] == '':
+    output_lines.pop()
----------------
gbedwell wrote:
> can be simplified to 
> ```while output_lines and output_lines[-1] == '':```
> I think.
Elsewhere in this script, `len(list) > 0` is used.
I can change it if you insist, but i think this is inconsistent, and less readable.


================
Comment at: utils/update_mca_test_checks.py:458
   with open(test_path, 'wb') as f:
-    for line in output_lines:
-      f.write('{}\n'.format(line.rstrip()).encode())
-
+    f.writelines([l + '\n' for l in output_lines])
 
----------------
gbedwell wrote:
> I've been trying to keep this file compatible with Python 2 and 3, ready for inevitably switching the default at some point hence jumping through a few weird hoops with this sort of stuff.  Python 3 now produces:
> 
> ```
> e:\work\upstream-llvm\llvm\utils>py -3 update_mca_test_checks.py ..\test\tools\llvm-mca\*\*.s
> Test: ..\test\tools\llvm-mca\ARM\simple-test-cortex-a9.s
>       [38 lines total]
> Traceback (most recent call last):
>   File "update_mca_test_checks.py", line 499, in <module>
>     sys.exit(main())
>   File "update_mca_test_checks.py", line 491, in main
>     prefix_pad)
>   File "update_mca_test_checks.py", line 458, in _write_output
>     f.writelines([l + '\n' for l in output_lines])
> TypeError: a bytes-like object is required, not 'str'
> ```
Works fine here in linux with
```
$ python3 --version
Python 3.6.5
```

This change shouldn't be **needed**, but that is what **every** single other update script does **already**.
So are they all broken? Or maybe something on your side?


Repository:
  rL LLVM

https://reviews.llvm.org/D47697