whisperity marked an inline comment as done. whisperity added a comment. I'd rather not call them //false positive// because the definition of `false positive` is ugly and mushy with regards to this check. This check attempts to enforce an interface rule, whether you(r project) wants to adhere the rule is a concise decision. A type equivalence (or convertibility in case of the follow-up patch D75041 <https://reviews.llvm.org/D75041> that considers implicit conversions) should be a "true positive" in every case, as an indicator of potentially bad design.
However, there's the question whether or not the similar argument-ness is intentional, i.e. a (from one PoV flawed) but deliberate design decision by the project. We've tested a few projects. The ugliest I've found is //OpenCV// where every variable of a function argument is either an `_InputArray` or an `_OutputArray`... Another question is, how fragmented you want your type system to be... I hope my research into this topic will be at least something fruitful. It's all about compromises, one end if using `any` for all your variables, the other extreme is having a specific type for every single occurrence (stuff like `fabs_return_t`, `create_schema_name_t` and stuff). There's a similar patch D20689 <https://reviews.llvm.org/D20689> but that uses names (and thus requires that arguments and parameters to be "named" in some way). ================ Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AdjacentArgumentsOfSameTypeCheck.cpp:474 +// Default to 3 so the users don't get a warning for every possible thing. +static const unsigned DefaultMinLength = 3; + ---------------- vingeldal wrote: > Am I getting this right, is this the number of consecutive arguments of the > same type which is required for this check to print a diagnostic? If so: why > not set the default value to 2? -I think that's what a user who just read the > C++ Core Guidelines would expect. Well, it is to reduce the number of nagging errors. Setting it to `2` would result in every "min" and "pow" and similar functions to match. It's a compromise. 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
