Update clang-tidy rules#2814
Update clang-tidy rules#2814guwirth merged 19 commits intoSonarOpenCommunity:masterfrom cmorve-te:llvm_191
Conversation
Was failing with "TypeError: cannot use a string pattern on a bytes-like object"
To match the existing clangtidy.xml.
To make it deterministic and match the existing clangtidy.xml. It's swapping the order of "modernize-use-default" and "modernize-use-default-member-init", making "modernize-use-default" go first.
To match the existing clangtidy.xml.
This handles at least the ReportMoreUnsafeFunctions link in bugprone/unsafe-functions.
…erity/type To match the existing clangtidy.xml.
To match the existing clangtidy.xml.
To match the existing clangtidy.xml.
To avoid having to check them in the diff when updating. I suspect clang-analyzer-core.DynamicTypePropagation has never been a real one, but let's keep it for the time being. linuxkernel-must-use-errs is not included because it was always incorrect in the documentation, the code always used linuxkernel-must-check-errs.
They were already sorted until the 19.1.0 update.
In the order the script generates them. Before the 19.1.0 update that was already the case.
In the order the script generates them when using the llvmorg-19.1.0 tag.
Using the latest version of clangtidy_createrules.py.
To make it deterministic and make it easier to know where to place old ones.
To avoid having to check the diff when updating.
|
@cmorve-te CI failed because of timeout. I re-run the job ... |
|
@cmorve-te Normally, the sensors support all the rules of the current version and also those of the previous versions. I would not deviate from this in 2.1.x. With 2.2.x we can think about cleaning up and removing outdated rules (this PR is for 2.2). For me, the decisive factor is always which version of the tool is included in larger Linux distributions. These rules should at least be supported. |
Sorry, not sure I understand what you mean here. Is this about linuxkernel-must-use-errs? If it was not clear from the commit message, that has never been a real rule. Not in 19.1, not in any previous version. The "linuxkernel-must-check-errs" warning was added in llvm 10 (llvm/llvm-project@fc8c65b), but wrongly documented as "linuxkernel-must-use-errs". sonar-cxx has had a "linuxkernel-must-use-errs" rule because it was parsing the (wrong) documentation to obtain the list of rules. Do you want me to modify this PR to keep "linuxkernel-must-use-errs", despite clang-tidy having never generated such a message? |
@cmorve-te No I like to verify which clang version is part of current CI/CD systems
In case it's an older one we should discuss if it makes sense to support also these old rules? |
14.0 -> https://packages.ubuntu.com/jammy/clang
11.0 -> https://packages.debian.org/bullseye/clang
I don't have a preference here. |
|
@cmorve-te do you know which rules are in clang 11 and no more in clang 14?
What do you think? |
In principle, fine with me. But see below.
Kind of. You can mostly see the versions from 7294a24. So you can see https://releases.llvm.org/13.0.0/tools/clang/docs/DiagnosticsReference.html#wrequires-expression was last available in clang 13, the rest were still there in clang 14. But you will notice I didn't change the links to some "old" warnings. And that's because the
There are quite a few of these warnings that, according to the documentation, were real warnings at some point and become "for GCC compatibility" later. I find this quite strange, you can probably treat them as "it was remove in clang 15", but not 100% sure. |
|
Hi @cmorve-te, my proposal is to "merge the two PRs as is" and compare the XML afterwards with the old ones. I would add then the missing rules at the beginning of the XML as deprecated. What do you think? |
Not sure I got this. The main thing from this specific PR is that it makes updating the rules easier, since the script directly generates the existing XML. Part of this is because old rules are already added by the add_old_clangtidy_rules()/add_old_diagnostics_rules() functions. If you prefer to have them only in the XML, but in a specific section at the top, that's also fine with me, yes. |
|
@cmorve-te I compared old/new again. My old comparison was wrong (why ever?). There is only one difference: |
There was a problem hiding this comment.
@cmorve-te in the old file were additional keys which are missing now:
<key>linuxkernel-must-use-errs</key>
There was a problem hiding this comment.
Yes, this was done on purpose. I tried to document it in d419293
linuxkernel-must-use-errs is not included because it was always incorrect in the documentation, the code always used linuxkernel-must-check-errs.
You can see the details in llvm/llvm-project@a269195
| def create_template_rules(rules): | ||
| rule_key = "CustomRuleTemplate" | ||
| rule_name = "Template for custom Custom rules" | ||
| rule_name = "Rule template for Clang-Tidy custom rules" |
There was a problem hiding this comment.
CustomRuleTemplate is correct now
|
|
||
| RulesDefinition.Repository repo = context.repository(CxxClangTidyRuleRepository.KEY); | ||
| assertThat(repo.rules()).hasSize(1573); | ||
| assertThat(repo.rules()).hasSize(1572); |
There was a problem hiding this comment.
@cmorve-te could you add a list with added/removed rules please.
|
@cmorve-te thx for providing this PR |
I did this before #2782, but it got stuck in my employer's review process. Even if it's not adding any new rule any more, I still think the rest is worth merging.
The main thing is that it makes updating easier. After this,
python clangtidy_createrules.py rules_fixurls <src_dir>clang-tools-extra/docs/clang-tidy/checks(which now works again) can be basically copy&pasted as it is into clangtidy.xml.python clangtidy_createrules.py diagnostics output.jsoncan be basically copy&pasted as it is into clangtidy.xml.Doing it this way avoids missing things like the fact the cppcoreguidelines-special-member-functions clang-tidy check gained an "AllowImplicitlyDeletedCopyOrMove" option in llvm 19, which is now included.
This change is