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

Alexander Kornienko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 17 05:39:11 PST 2017

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

A few more comments.

Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:153
+  const SourceRange FVLoc(DeclStmt->getLocStart(), Location);
+  std::string UserWrittenType =
+      Lexer::getSourceText(CharSourceRange::getCharRange(FVLoc), SM,
I wonder whether re-lexing the whole range and working on tokens instead would be a better and more robust approach? You wouldn't have to deal with whitespace and comments then. What do you think?

Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:163
+  // const int *&   -> const int
+  // long **        -> long int
+  size_t Pos = std::string::npos;
Why do we want `long int` and not `long`?

Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.h:25
+class OneNamePerDeclarationCheck : public ClangTidyCheck {
+  std::string getUserWrittenType(const DeclStmt *DeclStmt, SourceManager &SM);
nit: move the private section to the end.

Comment at: clang-tidy/utils/LexerUtils.cpp:41-56
+  const auto &SM = Context.getSourceManager();
+  const auto &LO = Context.getLangOpts();
+  auto StartOfFile = SM.getLocForStartOfFile(SM.getFileID(Location));
+  Token CurrentToken;
+  CurrentToken.setKind(tok::unknown);
Is it significantly more efficient than

  Token Tok;
  do {
    Tok = getPreviousNonCommentToken(Context, Location);
  } while (!Tok.isOneOf(tok::unknown, TokenToFind));
  return Tok.getLocation();


If it's not, then this function might be not so much useful to be here. At least, its interface is rather specific to the use case you have. Why, for example, it returns a location instead of the token itself?


More information about the cfe-commits mailing list