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

Reply via email to