[PATCH] D75041: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with mixability because of implicit conversions

Whisperity via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 4 07:08:12 PDT 2021


whisperity marked 5 inline comments as done.
whisperity added a comment.

In D75041#2799023 <https://reviews.llvm.org/D75041#2799023>, @martong wrote:

> Perhaps all conversion related logic should go into their own implementation file? Seems like it adds up to roughly 1000 lines.

That's against the usual conventions of the project (1 TU for 1 check implementation). The methods are grouped by namespace, you can fold it up in your editor.



================
Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:550-551
+  if (isUselessSugar(LType.getTypePtr())) {
+    LLVM_DEBUG(llvm::dbgs()
+               << "--- calculateMixability. LHS is useless sugar.\n");
     return calculateMixability(Check, LType.getSingleStepDesugaredType(Ctx),
----------------
martong wrote:
> Is this still WIP or do you use the DEBUG printouts in the tests? If not then could you please create a Diff without the DEBUG printouts, that could increase readability and ease the review.
As discussed earlier (and perhaps in the previous patches), these debug printouts are needed and shall stay here. The output is not used //in automation//, but it's intended for people who might pick up on further developing the check later. The recursive descent is too complex to be handleable in your mind without the debug information.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75041/new/

https://reviews.llvm.org/D75041



More information about the cfe-commits mailing list