whisperity marked 12 inline comments as done.
whisperity added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:657-660
+ } else {
+ ParamTypes.push_back(QualType());
+ ParamNames.push_back(StringRef());
+ }
----------------
whisperity wrote:
> aaron.ballman wrote:
> > Can this case happen?
> Oops... It seems I posted the updated patch right where you were writing more
> comments and we got into a data race. Which case are you referring to? It's
> now affixed to a `diag(` call for me...
Oh, nevermind, there is a button that shows me the older diff where it's
aligned properly.
And yeah, it seems it can't, `getParamDecl` always returns a `ParmVarDecl`.
Weird issues might arise when the vectors that are built here get out of sync
(such as the issue we had with `operator()` calls before I fixed it!), so I
understood the reason behind keeping the two functions parallel with each other
in terms of pure visuals, even.
================
Comment at:
clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:743-744
+
+ if (AreNamesSimilar)
+ break;
+
----------------
aaron.ballman wrote:
> Any reason not to move this below the `switch` and use `=` instead of `|=`
> within the cases? (Or return from the cases directly?)
Returning from the cases directly is a bad idea because we want to try all
heuristics and only say `false` if none of them matches.
But this break in a very bad location, I agree.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D20689/new/
https://reviews.llvm.org/D20689
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
