This is an archive of the discontinued LLVM Phabricator instance.

Differential D139966

[clang-tidy] Use Python3 for add_new_check.py and rename_check.py
ClosedPublic

Authored by carlosgalvezp on Dec 13 2022, 1:06 PM.

Details

Reviewers
njames93
MyDeveloperDay
Eugene.Zelenko
aaron.ballman
JonasToth
Commits
rG66bf54abb54e: [clang-tidy] Use Python3 for add_new_check.py and rename_check.py
Summary

Fixes #58782

Diff Detail

Event Timeline

carlosgalvezp created this revision.Dec 13 2022, 1:06 PM
Herald added a reviewer: njames93. · View Herald TranscriptDec 13 2022, 1:06 PM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: xazax.hun. · View Herald Transcript
carlosgalvezp requested review of this revision.Dec 13 2022, 1:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 13 2022, 1:06 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
carlosgalvezp updated this revision to Diff 482609.Dec 13 2022, 1:08 PM

Revert format changes.

carlosgalvezp added reviewers: MyDeveloperDay, Eugene.Zelenko, aaron.ballman, JonasToth.Dec 13 2022, 1:09 PM
carlosgalvezp updated this revision to Diff 482611.Dec 13 2022, 1:11 PM

Mention change in the Release Notes.

Harbormaster completed remote builds in B202937: Diff 482611.Dec 13 2022, 1:51 PM
Eugene.Zelenko added inline comments.Dec 13 2022, 2:21 PM
clang-tools-extra/docs/ReleaseNotes.rst
207

Should be at the beginning of Improvements to clang-tidy section, because it does not relate to checks.

carlosgalvezp updated this revision to Diff 482721.Dec 13 2022, 10:43 PM

Address comments

carlosgalvezp marked an inline comment as done.Dec 13 2022, 10:44 PM
carlosgalvezp updated this revision to Diff 482722.Dec 13 2022, 10:44 PM

Wrap to 80 chars.

Harbormaster completed remote builds in B203024: Diff 482722.Dec 13 2022, 11:04 PM
Eugene.Zelenko accepted this revision.Dec 14 2022, 7:01 AM

Looks OK for me, but will be good idea to wait for couple of days for other eyes.

This revision is now accepted and ready to land.Dec 14 2022, 7:01 AM
njames93 accepted this revision.Dec 15 2022, 1:56 PM

I'm just a little uneasy with the description, this isn't "fixing" the linked issue, its more a workaround. The issue itself is also a bit of a non-issue.

carlosgalvezp added a comment.Dec 15 2022, 10:53 PM
In D139966#3999473, @njames93 wrote:

I'm just a little uneasy with the description, this isn't "fixing" the linked issue, its more a workaround. The issue itself is also a bit of a non-issue.

Thanks for reviewing!

The script has a shebang, whose purpose is to be able to launch it via "./add_new_check.py". Therefore as a user I expect that to work. As such, a supported use case was broken, so IMO the issue is valid, and this is a correct fix, not a workaround. The workaround is invoking the tool as "python3 add_new_check.py".

If the intention is that people should invoke the script as "python3 add_new_check.py" always, instead of "./add_new_check.py", then it should not have any shebang.

Closed by commit rG66bf54abb54e: [clang-tidy] Use Python3 for add_new_check.py and rename_check.py (authored by carlosgalvezp). · Explain WhyDec 15 2022, 10:54 PM
This revision was automatically updated to reflect the committed changes.
carlosgalvezp added a commit: rG66bf54abb54e: [clang-tidy] Use Python3 for add_new_check.py and rename_check.py.

Revision Contents

PathSize
clang-tools-extra/
clang-tidy/
2 lines
2 lines
docs/
3 lines

Diff 483436

clang-tools-extra/clang-tidy/add_new_check.py

#!/usr/bin/env python #!/usr/bin/env python3
# #
#===- add_new_check.py - clang-tidy check generator ---------*- python -*--===# #===- add_new_check.py - clang-tidy check generator ---------*- python -*--===#
# #
# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. # Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
# See https://llvm.org/LICENSE.txt for license information. # See https://llvm.org/LICENSE.txt for license information.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
# #
#===-----------------------------------------------------------------------===# #===-----------------------------------------------------------------------===#
▲ Show 20 Lines • Show All 586 Lines • Show Last 20 Lines

clang-tools-extra/clang-tidy/rename_check.py

#!/usr/bin/env python #!/usr/bin/env python3
# #
#===- rename_check.py - clang-tidy check renamer ------------*- python -*--===# #===- rename_check.py - clang-tidy check renamer ------------*- python -*--===#
# #
# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. # Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
# See https://llvm.org/LICENSE.txt for license information. # See https://llvm.org/LICENSE.txt for license information.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
# #
#===-----------------------------------------------------------------------===# #===-----------------------------------------------------------------------===#
▲ Show 20 Lines • Show All 319 Lines • Show Last 20 Lines

clang-tools-extra/docs/ReleaseNotes.rst

Show First 20 Lines • Show All 90 Lines • ▼ Show 20 Lines
Improvements to clang-rename Improvements to clang-rename
---------------------------- ----------------------------
The improvements are... The improvements are...
Improvements to clang-tidy Improvements to clang-tidy
-------------------------- --------------------------
- Change to Python 3 in the shebang of `add_new_check.py` and `rename_check.py`,
as the existing code is not compatible with Python 2.
New checks New checks
^^^^^^^^^^ ^^^^^^^^^^
- New :doc:`bugprone-suspicious-realloc-usage - New :doc:`bugprone-suspicious-realloc-usage
<clang-tidy/checks/bugprone/suspicious-realloc-usage>` check. <clang-tidy/checks/bugprone/suspicious-realloc-usage>` check.
Finds usages of ``realloc`` where the return value is assigned to the Finds usages of ``realloc`` where the return value is assigned to the
same expression as passed to the first argument. same expression as passed to the first argument.
▲ Show 20 Lines • Show All 89 Lines • ▼ Show 20 Lines- Change the default behavior of :doc:`readability-const-return-type
warn about `const` value parameters of declarations inside macros. warn about `const` value parameters of declarations inside macros.
- Improved :doc:`misc-redundant-expression <clang-tidy/checks/misc/redundant-expression>` - Improved :doc:`misc-redundant-expression <clang-tidy/checks/misc/redundant-expression>`
check. check.
The check now skips concept definitions since redundant expressions still make sense The check now skips concept definitions since redundant expressions still make sense
inside them. inside them.
Removed checks Removed checks
Eugene.ZelenkoUnsubmitted
Done
ReplyInline Actions

Should be at the beginning of Improvements to clang-tidy section, because it does not relate to checks.

Eugene.Zelenko: Should be at the beginning of `Improvements to clang-tidy` section, because it does not relate…
^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^
Improvements to include-fixer Improvements to include-fixer
----------------------------- -----------------------------
The improvements are... The improvements are...
Improvements to clang-include-fixer Improvements to clang-include-fixer
Show All 14 Lines