diff options
4 files changed, 41 insertions, 50 deletions
diff --git a/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideActions.cpp b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideActions.cpp index 197befa4f4f..b93aee927d3 100644 --- a/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideActions.cpp +++ b/clang-tools-extra/cpp11-migrate/AddOverride/AddOverrideActions.cpp @@ -26,16 +26,41 @@ using namespace clang::ast_matchers; using namespace clang::tooling; using namespace clang; +namespace { + +SourceLocation +backwardSkipWhitespacesAndComments(const SourceManager &SM, + const clang::ASTContext &Context, + SourceLocation Loc) { + for (;;) { + do { + Loc = Loc.getLocWithOffset(-1); + } while (isWhitespace(*FullSourceLoc(Loc, SM).getCharacterData())); + + Token Tok; + SourceLocation Beginning = + Lexer::GetBeginningOfToken(Loc, SM, Context.getLangOpts()); + const bool Invalid = + Lexer::getRawToken(Beginning, Tok, SM, Context.getLangOpts()); + + assert(!Invalid && "Expected a valid token."); + if (Invalid || Tok.getKind() != tok::comment) + return Loc.getLocWithOffset(1); + } +} + +} // end anonymous namespace + 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"); - // Check that the method declaration in the main file - if (!SM.isFromMainFile(M->getLocStart())) - return; + if (const FunctionDecl *TemplateMethod = M->getTemplateInstantiationPattern()) + M = cast<CXXMethodDecl>(TemplateMethod); + // Check that the method declaration is in the main file if (!SM.isFromMainFile(M->getLocStart())) return; @@ -48,24 +73,14 @@ void AddOverrideFixer::run(const MatchFinder::MatchResult &Result) { if (M->isPure()) return; - if (const FunctionDecl *TemplateMethod = M->getTemplateInstantiationPattern()) - M = cast<CXXMethodDecl>(TemplateMethod); - if (M->getParent()->hasAnyDependentBases()) return; 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); + // Insert the override specifier before the function body. + StartLoc = backwardSkipWhitespacesAndComments(SM, *Result.Context, + M->getBody()->getLocStart()); } else { StartLoc = SM.getSpellingLoc(M->getLocEnd()); StartLoc = Lexer::getLocForEndOfToken(StartLoc, 0, SM, LangOptions()); diff --git a/clang-tools-extra/docs/AddOverrideTransform.rst b/clang-tools-extra/docs/AddOverrideTransform.rst index 8057f2fda1c..ea537fefc0f 100644 --- a/clang-tools-extra/docs/AddOverrideTransform.rst +++ b/clang-tools-extra/docs/AddOverrideTransform.rst @@ -28,20 +28,17 @@ For example: 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. +* This transform will not insert the override keyword if a method is + pure. At the moment it's not possible to track down the pure + specifier location. .. code-block:: c++ class B : public A { public: - virtual void h() const // comment - { } + virtual void h() const = 0; - // The declaration of h is transformed to - virtual void h() const // comment override - { } + // The declaration of h is NOT transformed to + virtual void h() const override = 0; }; diff --git a/clang-tools-extra/test/cpp11-migrate/AddOverride/basic.cpp b/clang-tools-extra/test/cpp11-migrate/AddOverride/basic.cpp index 1c1c95d8146..4bc471015df 100644 --- a/clang-tools-extra/test/cpp11-migrate/AddOverride/basic.cpp +++ b/clang-tools-extra/test/cpp11-migrate/AddOverride/basic.cpp @@ -57,8 +57,12 @@ public: class G : public A { public: void h() const; // comment + void i() // comment + {} // CHECK: class G // CHECK: void h() const override; // comment + // CHECK: void i() override // comment + // CHECK-NEXT: {} }; // Test that override is placed correctly if there is an inline body. 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 deleted file mode 100644 index 57c2ab0453f..00000000000 --- a/clang-tools-extra/test/cpp11-migrate/AddOverride/comment_before_inline_body_fail.cpp +++ /dev/null @@ -1,25 +0,0 @@ -// 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 -}; - |
