Adding the AddOverride transform for cpp11-migrate

This transform adds the override specifier to methods that overrides virtual
methods from a base class that don't already have this specifier.

Author: Philip Dunstan <phil@phildunstan.com>
llvm-svn: 179127

13 files changed, 451 insertions, 0 deletions

diff --git a/clang-tools-extra/cpp11-migrate/AddOverride/AddOverride.cpp b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverride.cpp
new file mode 100644
index 00000000000..d75c658ec69
--- /dev/null
+++ b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverride.cpp
@@ -0,0 +1,63 @@
+//===-- AddOverride/AddOverride.cpp - add C++11 override -------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// \brief This file provides the implementation of the AddOverrideTransform
+/// class.
+///
+//===----------------------------------------------------------------------===//
+
+#include "AddOverride.h"
+#include "AddOverrideActions.h"
+#include "AddOverrideMatchers.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Tooling.h"
+
+using clang::ast_matchers::MatchFinder;
+using namespace clang::tooling;
+using namespace clang;
+
+int AddOverrideTransform::apply(const FileContentsByPath &InputStates,
+                                RiskLevel MaxRisk,
+                                const CompilationDatabase &Database,
+                                const std::vector<std::string> &SourcePaths,
+                                FileContentsByPath &ResultStates) {
+  RefactoringTool AddOverrideTool(Database, SourcePaths);
+
+  for (FileContentsByPath::const_iterator I = InputStates.begin(),
+       E = InputStates.end();
+       I != E; ++I) {
+    AddOverrideTool.mapVirtualFile(I->first, I->second);
+  }
+
+  unsigned AcceptedChanges = 0;
+
+  MatchFinder Finder;
+  AddOverrideFixer Fixer(AddOverrideTool.getReplacements(), AcceptedChanges);
+
+  Finder.addMatcher(makeCandidateForOverrideAttrMatcher(), &Fixer);
+
+  if (int result = AddOverrideTool.run(newFrontendActionFactory(&Finder))) {
+    llvm::errs() << "Error encountered during translation.\n";
+    return result;
+  }
+
+  RewriterContainer Rewrite(AddOverrideTool.getFiles(), InputStates);
+
+  // FIXME: Do something if some replacements didn't get applied?
+  AddOverrideTool.applyAllReplacements(Rewrite.getRewriter());
+
+  collectResults(Rewrite.getRewriter(), InputStates, ResultStates);
+
+  setAcceptedChanges(AcceptedChanges);
+
+  return 0;
+}
diff --git a/clang-tools-extra/cpp11-migrate/AddOverride/AddOverride.h b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverride.h
new file mode 100644
index 00000000000..9b1aa1535ba
--- /dev/null
+++ b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverride.h
@@ -0,0 +1,37 @@
+//===-- AddOverride/AddOverride.h - add C++11 override ---------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// \brief This file provides the definition of the AddOverrideTransform
+/// class which is the main interface to the transform that tries to add
+/// the override keyword to declarations of member function that override
+/// virtual functions in a base class.
+///
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_ADD_OVERRIDE_H
+#define LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_ADD_OVERRIDE_H
+
+#include "Core/Transform.h"
+#include "llvm/Support/Compiler.h"
+
+/// \brief Subclass of Transform that adds the C++11 override keyword to
+/// member functions overriding base class virtual functions.
+class AddOverrideTransform : public Transform {
+public:
+  AddOverrideTransform() : Transform("AddOverride") {}
+
+  /// \see Transform::run().
+  virtual int apply(const FileContentsByPath &InputStates,
+                    RiskLevel MaxRiskLEvel,
+                    const clang::tooling::CompilationDatabase &Database,
+                    const std::vector<std::string> &SourcePaths,
+                    FileContentsByPath &ResultStates) LLVM_OVERRIDE;
+};
+
+#endif // LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_ADD_OVERRIDE_H
diff --git a/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideActions.cpp b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideActions.cpp
new file mode 100644
index 00000000000..a40f673132c
--- /dev/null
+++ b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideActions.cpp
@@ -0,0 +1,59 @@
+//===-- AddOverride/AddOverrideActions.cpp - add C++11 override-*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+///  \file
+///  \brief This file contains the definition of the AddOverrideFixer class
+///  which is used as an ASTMatcher callback.
+///
+//===----------------------------------------------------------------------===//
+
+#include "AddOverrideActions.h"
+#include "AddOverrideMatchers.h"
+
+#include "clang/Basic/CharInfo.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+using namespace clang::tooling;
+using namespace clang;
+
+void AddOverrideFixer::run(const MatchFinder::MatchResult &Result) {
+  SourceManager &SM = *Result.SourceManager;
+
+  const CXXMethodDecl *M = Result.Nodes.getDeclAs<CXXMethodDecl>(MethodId);
+  assert(M && "Bad Callback. No node provided");
+
+  // First check that there isn't already an override attribute.
+  if (!M->hasAttr<OverrideAttr>()) {
+    if (M->getLocStart().isFileID()) {
+      SourceLocation StartLoc;
+      if (M->hasInlineBody()) {
+        // Start at the beginning of the body and rewind back to the last
+        // non-whitespace character. We will insert the override keyword
+        // after that character.
+        // FIXME: This transform won't work if there is a comment between
+        // the end of the function prototype and the start of the body.
+        StartLoc = M->getBody()->getLocStart();
+        do {
+          StartLoc = StartLoc.getLocWithOffset(-1);
+        } while (isWhitespace(*FullSourceLoc(StartLoc, SM).getCharacterData()));
+        StartLoc = StartLoc.getLocWithOffset(1);
+      } else {
+        StartLoc = SM.getSpellingLoc(M->getLocEnd());
+        StartLoc = Lexer::getLocForEndOfToken(StartLoc, 0, SM, LangOptions());
+      }
+      Replace.insert(tooling::Replacement(SM, StartLoc, 0, " override"));
+      ++AcceptedChanges;
+    }
+  }
+}
+
diff --git a/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideActions.h b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideActions.h
new file mode 100644
index 00000000000..b2fdcec83cc
--- /dev/null
+++ b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideActions.h
@@ -0,0 +1,38 @@
+//===-- AddOverride/AddOverrideActions.h - add C++11 override --*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+///  \file
+///  \brief This file contains the declaration of the AddOverrideFixer class
+///  which is used as a ASTMatcher callback.
+///
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_ADD_OVERRIDE_ACTIONS_H
+#define LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_ADD_OVERRIDE_ACTIONS_H
+
+#include "Core/Transform.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/Refactoring.h"
+
+/// \brief The callback to be used for add-override migration matchers.
+///
+class AddOverrideFixer : public clang::ast_matchers::MatchFinder::MatchCallback {
+public:
+  AddOverrideFixer(clang::tooling::Replacements &Replace,
+                   unsigned &AcceptedChanges) :
+    Replace(Replace), AcceptedChanges(AcceptedChanges) {}
+
+  /// \brief Entry point to the callback called when matches are made.
+  virtual void run(const clang::ast_matchers::MatchFinder::MatchResult &Result);
+
+private:
+  clang::tooling::Replacements &Replace;
+  unsigned &AcceptedChanges;
+};
+
+#endif // LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_ADD_OVERRIDE_ACTIONS_H
diff --git a/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideMatchers.cpp b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideMatchers.cpp
new file mode 100644
index 00000000000..1e27002f852
--- /dev/null
+++ b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideMatchers.cpp
@@ -0,0 +1,28 @@
+//===-- AddOverride/AddOverrideMatchers.cpp - C++11 override ---*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+///  \file
+///  \brief This file contains the definitions for matcher-generating functions
+///  and a custom AST_MATCHER for identifying casts of type CK_NullTo*.
+///
+//===----------------------------------------------------------------------===//
+#include "AddOverrideMatchers.h"
+#include "clang/AST/ASTContext.h"
+
+using namespace clang::ast_matchers;
+using namespace clang;
+
+const char *MethodId = "method";
+
+DeclarationMatcher makeCandidateForOverrideAttrMatcher() {
+  return methodDecl(hasParent(recordDecl()),
+                    isOverride(),
+                    unless(destructorDecl())).bind(MethodId);
+}
+
diff --git a/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideMatchers.h b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideMatchers.h
new file mode 100644
index 00000000000..995919135f3
--- /dev/null
+++ b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideMatchers.h
@@ -0,0 +1,27 @@
+//===-- AddOverride/AddOverrideMatchers.h - add C++11 override -*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+///  \file
+///  \brief This file contains the declarations for matcher-generating functions
+///  and names for bound nodes found by AST matchers.
+///
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_ADD_OVERRIDE_MATCHERS_H
+#define LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_ADD_OVERRIDE_MATCHERS_H
+
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+/// Name to bind with matched expressions.
+extern const char *MethodId;
+
+/// \brief Create a matcher that finds member function declarations that are
+/// candidates for adding the override attribute.
+clang::ast_matchers::DeclarationMatcher makeCandidateForOverrideAttrMatcher();
+
+#endif // LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_ADD_OVERRIDE_MATCHERS_H
diff --git a/clang-tools-extra/cpp11-migrate/tool/CMakeLists.txt b/clang-tools-extra/cpp11-migrate/tool/CMakeLists.txt
index c3d85a61801..e3bff8ba148 100644
--- a/clang-tools-extra/cpp11-migrate/tool/CMakeLists.txt
+++ b/clang-tools-extra/cpp11-migrate/tool/CMakeLists.txt
@@ -16,6 +16,9 @@ list(APPEND Cpp11MigrateSources ${UseNullptrSources})
 file(GLOB_RECURSE UseAutoSources "../UseAuto/*.cpp")
 list(APPEND Cpp11MigrateSources ${UseAutoSources})
 
+file(GLOB_RECURSE AddOverrideSources "../AddOverride/*.cpp")
+list(APPEND Cpp11MigrateSources ${AddOverrideSources})
+
 add_clang_executable(cpp11-migrate
   ${Cpp11MigrateSources}
   )
diff --git a/clang-tools-extra/cpp11-migrate/tool/Cpp11Migrate.cpp b/clang-tools-extra/cpp11-migrate/tool/Cpp11Migrate.cpp
index bddfe31c846..dc335e9e76f 100644
--- a/clang-tools-extra/cpp11-migrate/tool/Cpp11Migrate.cpp
+++ b/clang-tools-extra/cpp11-migrate/tool/Cpp11Migrate.cpp
@@ -20,6 +20,7 @@
 #include "LoopConvert/LoopConvert.h"
 #include "UseNullptr/UseNullptr.h"
 #include "UseAuto/UseAuto.h"
+#include "AddOverride/AddOverride.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
@@ -71,6 +72,9 @@ int main(int argc, const char **argv) {
   TransformManager.registerTransform(
       "use-auto", "Use of 'auto' type specifier",
       &ConstructTransform<UseAutoTransform>);
+  TransformManager.registerTransform(
+      "add-override", "Make use of override specifier where possible",
+      &ConstructTransform<AddOverrideTransform>);
   // Add more transform options here.
 
   // This causes options to be parsed.
diff --git a/clang-tools-extra/cpp11-migrate/tool/Makefile b/clang-tools-extra/cpp11-migrate/tool/Makefile
index c1d0a6aa7b5..193f0696011 100644
--- a/clang-tools-extra/cpp11-migrate/tool/Makefile
+++ b/clang-tools-extra/cpp11-migrate/tool/Makefile
@@ -29,6 +29,8 @@ SOURCES += $(addprefix ../UseNullptr/,$(notdir $(wildcard $(PROJ_SRC_DIR)/../Use
 BUILT_SOURCES += $(ObjDir)/../UseNullptr/.objdir
 SOURCES += $(addprefix ../UseAuto/,$(notdir $(wildcard $(PROJ_SRC_DIR)/../UseAuto/*.cpp)))
 BUILT_SOURCES += $(ObjDir)/../UseAuto/.objdir
+SOURCES += $(addprefix ../AddOverride/,$(notdir $(wildcard $(PROJ_SRC_DIR)/../AddOverride/*.cpp)))
+BUILT_SOURCES += $(ObjDir)/../AddOverride/.objdir
 
 LINK_COMPONENTS := $(TARGETS_TO_BUILD) asmparser bitreader support mc
 USEDLIBS = clangTooling.a clangFrontend.a clangSerialization.a clangDriver.a \
diff --git a/clang-tools-extra/docs/AddOverrideTransform.rst b/clang-tools-extra/docs/AddOverrideTransform.rst
new file mode 100644
index 00000000000..8057f2fda1c
--- /dev/null
+++ b/clang-tools-extra/docs/AddOverrideTransform.rst
@@ -0,0 +1,47 @@
+.. index:: Add-Override Transform
+
+======================
+Add-Override Transform
+======================
+
+The Add-Override Transform adds the ``override`` specifier to member
+functions that override a virtual function in a base class and that
+don't already have the specifier. The transform is enabled with the 
+:option:`-add-override` option of :program:`cpp11-migrate`.
+For example:
+
+.. code-block:: c++
+
+  class A {
+  public:
+    virtual void h() const;
+  };
+
+  class B : public A {
+  public:
+    void h() const;
+
+    // The declaration of h is transformed to
+    void h() const override;
+  };
+
+
+Known Limitations
+-----------------
+* This transform will fail if a method declaration has an inlined method
+  body and there is a comment between the method declaration and the body.
+  In this case, the override keyword will incorrectly be inserted at the 
+  end of the comment.
+
+.. code-block:: c++
+
+  class B : public A {
+  public:
+    virtual void h() const // comment
+    { }
+
+    // The declaration of h is transformed to
+    virtual void h() const // comment override
+    { }
+  };
+
diff --git a/clang-tools-extra/docs/cpp11-migrate.rst b/clang-tools-extra/docs/cpp11-migrate.rst
index 6e787f28e35..70e833683dc 100644
--- a/clang-tools-extra/docs/cpp11-migrate.rst
+++ b/clang-tools-extra/docs/cpp11-migrate.rst
@@ -10,6 +10,7 @@ cpp11-migrate User's Manual
    UseAutoTransform
    UseNullptrTransform
    LoopConvertTransform
+   AddOverrideTransform
 
 :program:`cpp11-migrate` is a standalone tool used to automatically convert
 C++98 and C++03 code to use features of the new C++11 standard where
@@ -51,6 +52,13 @@ Command Line Options
   Replace the type specifier of variable declarations with the ``auto`` type
   specifier. See :doc:`UseAutoTransform`.
 
+.. option:: -add-override
+
+  Adds the override specifier to member functions where it is appropriate. That
+  is, the override specifier is added to member functions that override a
+  virtual function in a base class and that don't already have the specifier.
+  See :doc:`AddOverrideTransform`.
+
 .. option:: -p=<build-path>
 
   ``<build-path>`` is a CMake build directory containing a file named
diff --git a/clang-tools-extra/test/cpp11-migrate/AddOverride/basic.cpp b/clang-tools-extra/test/cpp11-migrate/AddOverride/basic.cpp
new file mode 100644
index 00000000000..1c3616bfcbb
--- /dev/null
+++ b/clang-tools-extra/test/cpp11-migrate/AddOverride/basic.cpp
@@ -0,0 +1,110 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: cpp11-migrate -add-override %t.cpp -- -I %S -std=c++11
+// RUN: FileCheck -input-file=%t.cpp %s
+
+class A {
+public:
+  virtual ~A();
+  // CHECK: virtual ~A();
+  void f();
+  virtual void h() const;
+  // CHECK: virtual void h() const;
+  virtual void i() = 0;
+  // CHECK: virtual void i() = 0;
+};
+
+// Test that override isn't added to non-virtual functions.
+class B : public A {
+public:
+  void f();
+  // CHECK: class B
+  // CHECK: void f();
+};
+
+// Test that override is added to functions that override virtual functions.
+class C : public A {
+public:
+  void h() const;
+  // CHECK: class C
+  // CHECK: void h() const override;
+};
+
+// Test that override isn't add to functions that overload but not override.
+class D : public A {
+public:
+  void h();
+  // CHECK: class D
+  // CHECK: void h();
+};
+
+// Test that override isn't added again to functions that already have it.
+class E : public A {
+public:
+  void h() const override;
+  // CHECK: class E
+  // CHECK: void h() const override;
+};
+
+// Test that override isn't added to the destructor.
+class F : public A {
+public:
+  virtual ~F();
+  // CHECK: class F
+  // CHECK: virtual ~F();
+};
+
+// Test that override is placed before any end of line comments.
+class G : public A {
+public:
+  void h() const; // comment
+  // CHECK: class G
+  // CHECK: void h() const override; // comment
+};
+
+// Test that override is placed correctly if there is an inline body.
+class H : public A {
+public:
+  void h() const { }
+  // CHECK: class H
+  // CHECK: void h() const override { }
+};
+
+// Test that override is placed correctly if there is a body on the next line.
+class I : public A {
+public:
+  void h() const
+  { }
+  // CHECK: class I
+  // CHECK: void h() const override
+  // CHECK: { }
+};
+
+// Test that override is placed correctly if there is a body outside the class.
+class J : public A {
+public:
+  void h() const;
+  // CHECK: class J
+  // CHECK: void h() const override;
+};
+
+void J::h() const {
+  // CHECK: void J::h() const {
+}
+
+// Test that override is placed correctly if there is a trailing return type.
+class K : public A {
+public:
+  auto h() const -> void;
+  // CHECK: class K
+  // CHECK: auto h() const -> void override;
+};
+
+// Test that override isn't added if it is already specified via a macro.
+class L : public A {
+public:
+#define LLVM_OVERRIDE override
+  void h() const LLVM_OVERRIDE;
+  // CHECK: class L
+  // CHECK: void h() const LLVM_OVERRIDE;
+};
+
diff --git a/clang-tools-extra/test/cpp11-migrate/AddOverride/comment_before_inline_body_fail.cpp b/clang-tools-extra/test/cpp11-migrate/AddOverride/comment_before_inline_body_fail.cpp
new file mode 100644
index 00000000000..57c2ab0453f
--- /dev/null
+++ b/clang-tools-extra/test/cpp11-migrate/AddOverride/comment_before_inline_body_fail.cpp
@@ -0,0 +1,25 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: cpp11-migrate -add-override %t.cpp -- -I %S
+// RUN: FileCheck -input-file=%t.cpp %s
+// XFAIL: *
+
+class A {
+public:
+  virtual void h() const;
+  // CHECK: virtual void h() const;
+};
+
+// Test that the override is correctly placed if there
+// is an inline comment between the function declaration
+// and the function body.
+// This test fails with the override keyword being added
+// to the end of the comment. This failure occurs because
+// the insertion point is incorrectly calculated if there
+// is an inline comment before the method body.
+class B : public A {
+public:
+  virtual void h() const // comment
+  { }
+  // CHECK: virtual void h() const override
+};
+