This updated patch fixes the long header lines pointed out by Edwin and
removes the extra newlines suggested by Manuel
It also updates the patch with recent changes to the way transforms are
registered in the cpp11-migrate tool.
Edwin, I do not have checkin rights. Will you please check this in along with
the patch D629 to add the required AST matchers.
Hi revane, klimek,
http://llvm-reviews.chandlerc.com/D630
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D630?vs=1519&id=1569#toc
Files:
tools/extra/cpp11-migrate/AddOverride/AddOverride.cpp
tools/extra/cpp11-migrate/AddOverride/AddOverride.h
tools/extra/cpp11-migrate/AddOverride/AddOverrideActions.cpp
tools/extra/cpp11-migrate/AddOverride/AddOverrideActions.h
tools/extra/cpp11-migrate/AddOverride/AddOverrideMatchers.cpp
tools/extra/cpp11-migrate/AddOverride/AddOverrideMatchers.h
tools/extra/cpp11-migrate/tool/CMakeLists.txt
tools/extra/cpp11-migrate/tool/Cpp11Migrate.cpp
tools/extra/cpp11-migrate/tool/Makefile
tools/extra/docs/AddOverrideTransform.rst
tools/extra/docs/cpp11-migrate.rst
tools/extra/test/cpp11-migrate/AddOverride/basic.cpp
tools/extra/test/cpp11-migrate/AddOverride/comment_before_inline_body_fail.cpp
Index: tools/extra/cpp11-migrate/AddOverride/AddOverride.cpp
===================================================================
--- tools/extra/cpp11-migrate/AddOverride/AddOverride.cpp
+++ 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;
+}
Index: tools/extra/cpp11-migrate/AddOverride/AddOverride.h
===================================================================
--- tools/extra/cpp11-migrate/AddOverride/AddOverride.h
+++ 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
Index: tools/extra/cpp11-migrate/AddOverride/AddOverrideActions.cpp
===================================================================
--- tools/extra/cpp11-migrate/AddOverride/AddOverrideActions.cpp
+++ 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;
+ }
+ }
+}
+
Index: tools/extra/cpp11-migrate/AddOverride/AddOverrideActions.h
===================================================================
--- tools/extra/cpp11-migrate/AddOverride/AddOverrideActions.h
+++ 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
Index: tools/extra/cpp11-migrate/AddOverride/AddOverrideMatchers.cpp
===================================================================
--- tools/extra/cpp11-migrate/AddOverride/AddOverrideMatchers.cpp
+++ 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);
+}
+
Index: tools/extra/cpp11-migrate/AddOverride/AddOverrideMatchers.h
===================================================================
--- tools/extra/cpp11-migrate/AddOverride/AddOverrideMatchers.h
+++ 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
Index: tools/extra/cpp11-migrate/tool/CMakeLists.txt
===================================================================
--- tools/extra/cpp11-migrate/tool/CMakeLists.txt
+++ tools/extra/cpp11-migrate/tool/CMakeLists.txt
@@ -16,6 +16,9 @@
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}
)
Index: tools/extra/cpp11-migrate/tool/Cpp11Migrate.cpp
===================================================================
--- tools/extra/cpp11-migrate/tool/Cpp11Migrate.cpp
+++ 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 @@
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.
Index: tools/extra/cpp11-migrate/tool/Makefile
===================================================================
--- tools/extra/cpp11-migrate/tool/Makefile
+++ tools/extra/cpp11-migrate/tool/Makefile
@@ -29,6 +29,8 @@
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 \
Index: tools/extra/docs/AddOverrideTransform.rst
===================================================================
--- tools/extra/docs/AddOverrideTransform.rst
+++ 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
+ { }
+ };
+
Index: tools/extra/docs/cpp11-migrate.rst
===================================================================
--- tools/extra/docs/cpp11-migrate.rst
+++ tools/extra/docs/cpp11-migrate.rst
@@ -10,6 +10,7 @@
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 @@
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
Index: tools/extra/test/cpp11-migrate/AddOverride/basic.cpp
===================================================================
--- tools/extra/test/cpp11-migrate/AddOverride/basic.cpp
+++ 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;
+};
+
Index: tools/extra/test/cpp11-migrate/AddOverride/comment_before_inline_body_fail.cpp
===================================================================
--- tools/extra/test/cpp11-migrate/AddOverride/comment_before_inline_body_fail.cpp
+++ 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
+};
+
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
