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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
