whisperity added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:284 +/// The default value for the MinimumLength check option. +static constexpr unsigned DefaultMinimumLength = 2; + ---------------- aaron.ballman wrote: > It might be good to move this to a more prominent location since it's a > default value. Moved it to the top. ================ Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:302 + "FowardIt", "bidirit", "BidirIt", "constiterator", + "const_iterator", "Const_Iterator", "Constiterator", + "ConstIterator"}); ---------------- aaron.ballman wrote: > `reverse_iterator` and `reverse_const_iterator` too? > > How about ranges? > How about ranges? I would like to say that having an `f(RangeTy, RangeTy)` is //exactly// as problematic (especially if both are non-const) as having `f(int, int)` or `f(void*, void*)`, and should be taken care of the same way (relatedness heuristic, name heuristic). The idea behind ignoring iterator-ly named things was that //"Iterators more often than not comes in pairs and you can't(*) do anything about it"//. (*): Use ranges. Similarly how `draw(int x, int y)` is `draw(Point2D)` if we are enhancing type safety. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69560/new/ https://reviews.llvm.org/D69560 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
