[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.


================
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:130
+    if (isa<TypedefDecl>(*(FirstVarIt + 1)))
+      return "typedef " + TypeString;
+
----------------
Should we suggest `using x = ...;` in C++11 code?


================
Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.h:25
+class OneNamePerDeclarationCheck : public ClangTidyCheck {
+private:
+  std::string getUserWrittenType(const DeclStmt *DeclStmt, SourceManager &SM);
----------------
firolino wrote:
> firolino wrote:
> > alexfh wrote:
> > > nit: move the private section to the end.
> > [[ http://llvm.org/docs/CodingStandards.html#use-of-class-and-struct-keywords | LLVM Coding Standards ]] is putting the private section at the beginning as well. Even in the code base, it seems to be the general rule. While you are reading code from top to bottom, you need to get the necessary context (variables and their types) first, before proceeding to the functional code. Otherwise, you would have to scroll all the time down to the private variables - when passing its usage in a function - to get its type. This would impair the reading fluency and thus the straight-forwarding understanding of the code.
> Or is it, because there is just a function and not a private variable?
I don't think there's a rule to this effect in LLVM Coding Standards or a widely accepted convention, it's rather my personal preference (maybe because it's consistent with the Google C++ Style Guide, https://google.github.io/styleguide/cppguide.html#Declaration_Order). Also, the argument you're using doesn't seem to apply here, since the private implementation details aren't used in the public part below it (because, there are no non-empty definitions in it).


https://reviews.llvm.org/D27621