whisperity added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:529 +/// ReturnStmts return them from the function. +class Returned { + llvm::SmallVector<const ParmVarDecl *, SmallDataStructureSize> ReturnedParams; ---------------- aaron.ballman wrote: > whisperity wrote: > > whisperity wrote: > > > aaron.ballman wrote: > > > > Question: would it make sense to (someday, not now) consider output > > > > parameters similar to return statements? e.g., > > > > ``` > > > > void int_swap(int &foo, int &bar) { > > > > int temp = foo; > > > > foo = bar; > > > > bar = temp; > > > > } > > > > ``` > > > > As a question for today: should `co_return` should be handled similarly > > > > as `return`? > > > 1. Maybe. Unfortunately, one needs to be careful with that. Originally I > > > did implement a "5th" heuristic that dealt with ignoring parameters that > > > had the same builtin operator used on them. However, while it silenced a > > > few false positives, it started creating **massive** amounts of false > > > negatives. > > > (In the concrete example, I think `foo = bar` will silence them because > > > //"appears in the same expression"//.) > > > > > > 2. I don't know. It would require a deeper understanding of Coroutines > > > themselves, and how people use them. > > If you don't mind me posting images, I can show a direct example. Things > > where the inability to track data flow really bites us in the backside. > > > > Examples from Apache Xerces: > > > > Here's a false positive that would be silenced by the logic of //"using the > > same operateur on the two params"//. > > {F15891567} > > > > And here is a false negative from the exact same logic. > > {F15891568} > > > > Maybe it could be //some// solace that we're restricting to > > non-const-lvalue-references (and pointers-to-non-const ??) and only > > assignment (**only** assignment?!))... > Ah, point #1 is an excellent point. As for #2, I think we can handle it in a > follow-up, but I believe `co_return` follows the same usage patterns as > `return`, generally. Added a TODO for `co_return`, so we don't forget. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78652/new/ https://reviews.llvm.org/D78652 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits