This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] run-clang-tidy fails using python 3.7
ClosedPublic

Authored by Abpostelnicu on Aug 24 2018, 8:42 AM.

Details

Summary

Because the data types like str and byte changed in python 3.x we get this error when running run-clang-tidy:

Exception in thread Thread-1:
Traceback (most recent call last):

File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/threading.py", line 917, in _bootstrap_inner
  self.run()
File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/threading.py", line 865, in run
  self._target(*self._args, **self._kwargs)
File "run-clang-tidy.py", line 178, in run_tidy
  sys.stdout.write(' '.join(invocation) + '\n' + output + '\n')

TypeError: can only concatenate str (not "bytes") to str

Diff Detail

Event Timeline

Abpostelnicu created this revision.Aug 24 2018, 8:42 AM
Abpostelnicu edited the summary of this revision. (Show Details)Aug 24 2018, 8:43 AM
Abpostelnicu added a reviewer: JonasToth.
Abpostelnicu added a reviewer: alexfh.
JonasToth set the repository for this revision to rCTE Clang Tools Extra.

I don't know a lot about how to write portable code!

But wouldn't this be out usecase? http://python-future.org/compatible_idioms.html#file-io-with-open
Using the decode is supposed to work in both pythons

I don't know a lot about how to write portable code!

But wouldn't this be out usecase? http://python-future.org/compatible_idioms.html#file-io-with-open
Using the decode is supposed to work in both pythons

Yes, indeed, this is more elegant solution!

Could you please verify it is actually working?

Am 24.08.2018 um 18:05 schrieb Andi via Phabricator:

Abpostelnicu added a comment.

In https://reviews.llvm.org/D51220#1212463, @JonasToth wrote:

I don't know a lot about how to write portable code!

But wouldn't this be out usecase? http://python-future.org/compatible_idioms.html#file-io-with-open
Using the decode is supposed to work in both pythons

Yes, indeed, this is more elegant solution!

https://reviews.llvm.org/D51220

I can confirm tested on:
2.7.15
3.7.0

On both it worked.

LG from my side, but please let @aaron.ballman or @alexfh approve first

Am 24.08.2018 um 18:08 schrieb Andi via Phabricator:

Abpostelnicu added a comment.

I can confirm tested on:
2.7.15
3.7.0

On both it worked.

https://reviews.llvm.org/D51220

Have you seen D36624 / D38289 ?
Please test how it behaves with both the python2 and python3 when the clang-tidy output contains non-ASCII symbols.

Unicode works both with python3 and python2 (but seemed to work before too).

Otherwise LG

clang-tidy/tool/run-clang-tidy.py
169

The patch does not apply clean to master because of this part. Could you please recheck that?

171

please use len(err) here instead, python3 does not work.

Abpostelnicu marked 2 inline comments as done.
JonasToth accepted this revision.Sep 19 2018, 4:49 AM
This revision is now accepted and ready to land.Sep 19 2018, 4:49 AM
This revision was automatically updated to reflect the committed changes.