[clang-tools-extra] r208954 - Initial version of clang-tidy check to use override instead of virual.

Sean Silva chisophugis at gmail.com
Fri May 16 17:53:47 PDT 2014


+static SmallVector<Token, 16> ParseTokens(CharSourceRange Range,
+                                          const SourceManager &Sources,
+                                          LangOptions LangOpts) {

+static StringRef GetText(const Token& Tok, const SourceManager &Sources) {

Naming.

+      for (const clang::Attr *attr : Method->getAttrs()) {

Naming. Maybe just use "A"?

+    for (unsigned i = 0, e = Tokens.size(); i != e; ++i) {
+      if (Tokens[i].is(tok::raw_identifier) &&
+          GetText(Tokens[i], Sources) == "virtual") {
+        Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+            Tokens[i].getLocation(), Tokens[i].getLocation()));
+        break;
+      }
+    }

Can this be range-ified?

-- Sean Silva


On Fri, May 16, 2014 at 3:30 AM, Daniel Jasper <djasper at google.com> wrote:

> Author: djasper
> Date: Fri May 16 04:30:09 2014
> New Revision: 208954
>
> URL: http://llvm.org/viewvc/llvm-project?rev=208954&view=rev
> Log:
> Initial version of clang-tidy check to use override instead of virual.
>
> Review: http://reviews.llvm.org/D3688
>
> Added:
>     clang-tools-extra/trunk/clang-tidy/misc/UseOverride.cpp
>     clang-tools-extra/trunk/clang-tidy/misc/UseOverride.h
>     clang-tools-extra/trunk/test/clang-tidy/use-override.cpp
> Modified:
>     clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
>     clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
>     clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh
>
> Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=208954&r1=208953&r2=208954&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
> +++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Fri May 16
> 04:30:09 2014
> @@ -4,6 +4,7 @@ add_clang_library(clangTidyMiscModule
>    ArgumentCommentCheck.cpp
>    MiscTidyModule.cpp
>    RedundantSmartptrGet.cpp
> +  UseOverride.cpp
>
>    LINK_LIBS
>    clangAST
>
> Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=208954&r1=208953&r2=208954&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
> +++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Fri May 16
> 04:30:09 2014
> @@ -12,6 +12,7 @@
>  #include "../ClangTidyModuleRegistry.h"
>  #include "ArgumentCommentCheck.h"
>  #include "RedundantSmartptrGet.h"
> +#include "UseOverride.h"
>
>  namespace clang {
>  namespace tidy {
> @@ -25,6 +26,9 @@ public:
>      CheckFactories.addCheckFactory(
>          "misc-redundant-smartptr-get",
>          new ClangTidyCheckFactory<RedundantSmartptrGet>());
> +    CheckFactories.addCheckFactory(
> +        "misc-use-override",
> +        new ClangTidyCheckFactory<UseOverride>());
>    }
>  };
>
>
> Added: clang-tools-extra/trunk/clang-tidy/misc/UseOverride.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UseOverride.cpp?rev=208954&view=auto
>
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/misc/UseOverride.cpp (added)
> +++ clang-tools-extra/trunk/clang-tidy/misc/UseOverride.cpp Fri May 16
> 04:30:09 2014
> @@ -0,0 +1,131 @@
> +//===--- UseOverride.cpp - clang-tidy
> -------------------------------------===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
>
> +//===----------------------------------------------------------------------===//
> +
> +#include "UseOverride.h"
> +#include "clang/AST/ASTContext.h"
> +#include "clang/ASTMatchers/ASTMatchFinder.h"
> +#include "clang/Lex/Lexer.h"
> +
> +using namespace clang::ast_matchers;
> +
> +namespace clang {
> +namespace tidy {
> +
> +void UseOverride::registerMatchers(MatchFinder *Finder) {
> +  Finder->addMatcher(methodDecl(isOverride()).bind("method"), this);
> +}
> +
> +// Re-lex the tokens to get precise locations to insert 'override' and
> remove
> +// 'virtual'.
> +static SmallVector<Token, 16> ParseTokens(CharSourceRange Range,
> +                                          const SourceManager &Sources,
> +                                          LangOptions LangOpts) {
> +  std::pair<FileID, unsigned> LocInfo =
> +      Sources.getDecomposedLoc(Range.getBegin());
> +  StringRef File = Sources.getBufferData(LocInfo.first);
> +  const char *TokenBegin = File.data() + LocInfo.second;
> +  Lexer RawLexer(Sources.getLocForStartOfFile(LocInfo.first), LangOpts,
> +                 File.begin(), TokenBegin, File.end());
> +  SmallVector<Token, 16> Tokens;
> +  Token Tok;
> +  while (!RawLexer.LexFromRawLexer(Tok)) {
> +    if (Tok.is(tok::semi) || Tok.is(tok::l_brace))
> +      break;
> +    if (Sources.isBeforeInTranslationUnit(Range.getEnd(),
> Tok.getLocation()))
> +      break;
> +    Tokens.push_back(Tok);
> +  }
> +  return Tokens;
> +}
> +
> +static StringRef GetText(const Token& Tok, const SourceManager &Sources) {
> +  return {Sources.getCharacterData(Tok.getLocation()), Tok.getLength()};
> +}
> +
> +void UseOverride::check(const MatchFinder::MatchResult &Result) {
> +  const FunctionDecl *Method =
> Result.Nodes.getStmtAs<FunctionDecl>("method");
> +  const SourceManager &Sources = *Result.SourceManager;
> +
> +  assert(Method != nullptr);
> +  if (Method->getInstantiatedFromMemberFunction() != nullptr)
> +    Method = Method->getInstantiatedFromMemberFunction();
> +
> +  if (Method->isImplicit() || Method->getLocation().isMacroID() ||
> +      Method->isOutOfLine())
> +    return;
> +
> +  if (Method->getAttr<clang::OverrideAttr>() != nullptr &&
> +      !Method->isVirtualAsWritten())
> +    return; // Nothing to do.
> +
> +  DiagnosticBuilder Diag = diag(Method->getLocation(),
> +                                "Prefer using 'override' instead of
> 'virtual'");
> +
> +  CharSourceRange FileRange =
> +      Lexer::makeFileCharRange(CharSourceRange::getTokenRange(
> +                                   Method->getLocStart(),
> Method->getLocEnd()),
> +                               Sources, Result.Context->getLangOpts());
> +
> +  if (!FileRange.isValid())
> +    return;
> +
> +  // FIXME: Instead of re-lexing and looking for specific macros such as
> +  // 'ABSTRACT', properly store the location of 'virtual' and '= 0' in
> each
> +  // FunctionDecl.
> +  SmallVector<Token, 16> Tokens = ParseTokens(FileRange, Sources,
> +
>  Result.Context->getLangOpts());
> +
> +  // Add 'override' on inline declarations that don't already have it.
> +  if (Method->getAttr<clang::OverrideAttr>() == nullptr) {
> +    SourceLocation InsertLoc;
> +    StringRef ReplacementText = "override ";
> +
> +    if (Method->hasAttrs()) {
> +      for (const clang::Attr *attr : Method->getAttrs()) {
> +        if (!attr->isImplicit()) {
> +          InsertLoc = Sources.getExpansionLoc(attr->getLocation());
> +          break;
> +        }
> +      }
> +    }
> +
> +    if (InsertLoc.isInvalid() && Method->doesThisDeclarationHaveABody()) {
> +      InsertLoc = Method->getBody()->getLocStart();
> +    }
> +
> +    if (!InsertLoc.isValid()) {
> +      if (Tokens.size() > 2 && GetText(Tokens.back(), Sources) == "0" &&
> +          GetText(Tokens[Tokens.size() - 2], Sources) == "=") {
> +        InsertLoc = Tokens[Tokens.size() - 2].getLocation();
> +      } else if (GetText(Tokens.back(), Sources) == "ABSTRACT") {
> +        InsertLoc = Tokens.back().getLocation();
> +      }
> +    }
> +
> +    if (!InsertLoc.isValid()) {
> +      InsertLoc = FileRange.getEnd();
> +      ReplacementText = " override";
> +    }
> +    Diag << FixItHint::CreateInsertion(InsertLoc, ReplacementText);
> +  }
> +
> +  if (Method->isVirtualAsWritten()) {
> +    for (unsigned i = 0, e = Tokens.size(); i != e; ++i) {
> +      if (Tokens[i].is(tok::raw_identifier) &&
> +          GetText(Tokens[i], Sources) == "virtual") {
> +        Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
> +            Tokens[i].getLocation(), Tokens[i].getLocation()));
> +        break;
> +      }
> +    }
> +  }
> +}
> +
> +} // namespace tidy
> +} // namespace clang
>
> Added: clang-tools-extra/trunk/clang-tidy/misc/UseOverride.h
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UseOverride.h?rev=208954&view=auto
>
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/misc/UseOverride.h (added)
> +++ clang-tools-extra/trunk/clang-tidy/misc/UseOverride.h Fri May 16
> 04:30:09 2014
> @@ -0,0 +1,29 @@
> +//===--- UseOverride.h - clang-tidy -----------------------------*- C++
> -*-===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
>
> +//===----------------------------------------------------------------------===//
> +
> +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USE_OVERRIDE_H
> +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USE_OVERRIDE_H
> +
> +#include "../ClangTidy.h"
> +
> +namespace clang {
> +namespace tidy {
> +
> +/// \brief Use C++11's 'override' and remove 'virtual' where applicable.
> +class UseOverride : public ClangTidyCheck {
> +public:
> +  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
> +  void check(const ast_matchers::MatchFinder::MatchResult &Result)
> override;
> +};
> +
> +} // namespace tidy
> +} // namespace clang
> +
> +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USE_OVERRIDE_H
> +
>
> Modified: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh?rev=208954&r1=208953&r2=208954&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh
> (original)
> +++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh Fri
> May 16 04:30:09 2014
> @@ -8,4 +8,4 @@ TEMPORARY_FILE=$3.cpp
>
>  grep -Ev "// *[A-Z-]+:" ${INPUT_FILE} > ${TEMPORARY_FILE}
>  clang-tidy ${TEMPORARY_FILE} -fix --checks="-*,${CHECK_TO_RUN}" --
> --std=c++11
> -FileCheck -input-file=${TEMPORARY_FILE} ${INPUT_FILE}
> +FileCheck -input-file=${TEMPORARY_FILE} ${INPUT_FILE} -strict-whitespace
>
> Added: clang-tools-extra/trunk/test/clang-tidy/use-override.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/use-override.cpp?rev=208954&view=auto
>
> ==============================================================================
> --- clang-tools-extra/trunk/test/clang-tidy/use-override.cpp (added)
> +++ clang-tools-extra/trunk/test/clang-tidy/use-override.cpp Fri May 16
> 04:30:09 2014
> @@ -0,0 +1,134 @@
> +// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s misc-use-override %t
> +// REQUIRES: shell
> +
> +#define ABSTRACT = 0
> +
> +#define OVERRIDE override
> +#define VIRTUAL virtual
> +#define NOT_VIRTUAL
> +#define NOT_OVERRIDE
> +
> +#define MUST_USE_RESULT __attribute__((warn_unused_result))
> +
> +struct MUST_USE_RESULT MustUseResultObject {};
> +
> +struct Base {
> +  virtual ~Base() {}
> +  virtual void a();
> +  virtual void b();
> +  virtual void c();
> +  virtual void d();
> +  virtual void e() = 0;
> +  virtual void f() = 0;
> +  virtual void g() = 0;
> +
> +  virtual void j() const;
> +  virtual MustUseResultObject k();
> +  virtual bool l() MUST_USE_RESULT;
> +};
> +
> +struct SimpleCases : public Base {
> +public:
> +  virtual ~SimpleCases();
> +  // CHECK: {{^  ~SimpleCases\(\) override;}}
> +
> +  void a();
> +  // CHECK: {{^  void a\(\) override;}}
> +  void b() override;
> +  // CHECK: {{^  void b\(\) override;}}
> +  virtual void c();
> +  // CHECK: {{^  void c\(\) override;}}
> +  virtual void d() override;
> +  // CHECK: {{^  void d\(\) override;}}
> +
> +  virtual void e() = 0;
> +  // CHECK: {{^  void e\(\) override = 0;}}
> +  virtual void f()=0;
> +  // CHECK: {{^  void f\(\)override =0;}}
> +  virtual void g() ABSTRACT;
> +  // CHECK: {{^  void g\(\) override ABSTRACT;}}
> +
> +  virtual void j() const;
> +  // CHECK: {{^  void j\(\) const override;}}
> +  virtual MustUseResultObject k();  // Has an implicit attribute.
> +  // CHECK: {{^  MustUseResultObject k\(\) override;}}
> +  virtual bool l() MUST_USE_RESULT; // Has an explicit attribute
> +  // CHECK: {{^  bool l\(\) override MUST_USE_RESULT;}}
> +};
> +
> +void SimpleCases::i() {}
> +// CHECK: {{^void SimpleCases::i\(\) {}}}
> +
> +struct InlineDefinitions : public Base {
> +public:
> +  virtual ~InlineDefinitions() {}
> +  // CHECK: {{^  ~InlineDefinitions\(\) override {}}}
> +
> +  void a() {}
> +  // CHECK: {{^  void a\(\) override {}}}
> +  void b() override {}
> +  // CHECK: {{^  void b\(\) override {}}}
> +  virtual void c() {}
> +  // CHECK: {{^  void c\(\) override {}}}
> +  virtual void d() override {}
> +  // CHECK: {{^  void d\(\) override {}}}
> +
> +  virtual void j() const {}
> +  // CHECK: {{^  void j\(\) const override {}}}
> +  virtual MustUseResultObject k() {}  // Has an implicit attribute.
> +  // CHECK: {{^  MustUseResultObject k\(\) override {}}}
> +  virtual bool l() MUST_USE_RESULT {} // Has an explicit attribute
> +  // CHECK: {{^  bool l\(\) override MUST_USE_RESULT {}}}
> +};
> +
> +struct Macros : public Base {
> +  // Tests for 'virtual' and 'override' being defined through macros.
> Basically
> +  // give up for now.
> +  NOT_VIRTUAL void a() NOT_OVERRIDE;
> +  // CHECK: {{^  NOT_VIRTUAL void a\(\) override NOT_OVERRIDE;}}
> +
> +  VIRTUAL void b() NOT_OVERRIDE;
> +  // CHECK: {{^  VIRTUAL void b\(\) override NOT_OVERRIDE;}}
> +
> +  NOT_VIRTUAL void c() OVERRIDE;
> +  // CHECK: {{^  NOT_VIRTUAL void c\(\) OVERRIDE;}}
> +
> +  VIRTUAL void d() OVERRIDE;
> +  // CHECK: {{^  VIRTUAL void d\(\) OVERRIDE;}}
> +
> +#define FUNC(name, return_type) return_type name()
> +  FUNC(void, e);
> +  // CHECK: {{^  FUNC\(void, e\);}}
> +
> +#define F virtual void f();
> +  F
> +  // CHECK: {{^  F}}
> +};
> +
> +// Tests for templates.
> +template <typename T> struct TemplateBase {
> +  virtual void f(T t);
> +};
> +
> +template <typename T> struct DerivedFromTemplate : public TemplateBase<T>
> {
> +  virtual void f(T t);
> +  // CHECK: {{^  void f\(T t\) override;}}
> +};
> +void f() { DerivedFromTemplate<int>().f(2); }
> +
> +template <class C>
> +struct UnusedMemberInstantiation : public C {
> +  virtual ~UnusedMemberInstantiation() {}
> +  // CHECK: {{^  ~UnusedMemberInstantiation\(\) override {}}}
> +};
> +struct IntantiateWithoutUse : public UnusedMemberInstantiation<Base> {};
> +
> +// The OverrideAttr isn't propagated to specializations in all cases.
> Make sure
> +// we don't add "override" a second time.
> +template <int I>
> +struct MembersOfSpecializations : public Base {
> +  void a() override;
> +  // CHECK: {{^  void a\(\) override;}}
> +};
> +template <> void MembersOfSpecializations<3>::a() {}
> +void f() { D<3>().a(); };
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140516/94858675/attachment.html>


More information about the cfe-commits mailing list