aaron.ballman added a comment. In D69560#1889483 <https://reviews.llvm.org/D69560#1889483>, @whisperity wrote:
> 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. The problem is that there *are* false positives, logically. The rule is about "unrelated" parameters, so the false positives come from not having any way to distinguish between related and unrelated arguments. My intuition is that the simple enforcement suggested by the rule is unlikely to be acceptable, but that's why I'm asking for data over large, real-world projects. > 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). Agreed -- if we can find a heuristic that makes this work, the check has a lot of value. >> In D74463#1888270 <https://reviews.llvm.org/D74463#1888270>, @aaron.ballman >> wrote: >> There is no obvious way to silence any of the diagnostics generated as false >> positives short of requiring a naming convention for users to follow, which >> is not a particularly clean solution. > > There is, you decide not to enable the checker? That is not a useful level of granularity given that this check is likely to be incredibly noisy as part of our other C++ Core Guideline enforcement. We typically have `NOLINT` comments that we can fall back on for silencing individual instances of a false positive, but that's not likely to be a workable solution by itself for this check in particular because of system and third-party headers which are outside of the user's control (and unlikely to be willing or able to modify). 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
