================ Comment at: clang-tidy/readability/RemoveVoidArg.h:38 @@ +37,3 @@ +private: + void processFunctionDecl(const ast_matchers::MatchFinder::MatchResult &Result, FunctionDecl const *const Function); + ---------------- sbenza wrote: > LegalizeAdulthood wrote: > > sbenza wrote: > > > alexfh wrote: > > > > Here and below: I'm against top-level `const` in function parameter > > > > types. It's just an implementation detail whether the method can modify > > > > its parameter. It doesn't make sense to pollute the interface with this. > > > > > > > > Also, it's more common in Clang/LLVM code to put the const related to > > > > the pointed/referenced object in front: > > > > > > > > void processFunctionDecl(const ast_matchers::MatchFinder::MatchResult > > > > &Result, const FunctionDecl *Function); > > > Any reason why these are members? > > > Making them free functions in the .cpp file would avoid filling the > > > header with these implementation details. > > > It would also remove the need to include Lexer.h here. > > They're members because ultimately on detection they ultimately call diag > > which is a member function. Otherwise, I would have made them free > > functions. > They could still be free functions, that return some data instead of taking > action directly. > But I understand now. They'd essentially have to return all the arguments in this statement:
``` diag(VoidLoc, Diagnostic) << FixItHint::CreateRemoval(VoidRange); ``` ...which feels tortured just to avoid creating member functions on the implementation. http://reviews.llvm.org/D7639 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
