whisperity marked 2 inline comments as done.
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:
> 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.


================
Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:563-566
+  llvm::Optional<relatedness_heuristic::AppearsInSameExpr> SameExpr;
+  llvm::Optional<relatedness_heuristic::PassedToSameFunction> PassToFun;
+  llvm::Optional<relatedness_heuristic::AccessedSameMemberOf> SameMember;
+  llvm::Optional<relatedness_heuristic::Returned> Returns;
----------------
aaron.ballman wrote:
> I don't think there's a need to make these optional when we're using the 
> `Enabled` flag.
The original idea was that each heuristic could be individually toggled by the 
user... But I think we can agree that would be a bad idea and just 
overengineering.


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