[clang-tools-extra] r181806 - cpp11-migrate: Add override specifier before comments on inline methods
Edwin Vane
edwin.vane at intel.com
Tue May 14 10:34:12 PDT 2013
Author: revane
Date: Tue May 14 12:34:12 2013
New Revision: 181806
URL: http://llvm.org/viewvc/llvm-project?rev=181806&view=rev
Log:
cpp11-migrate: Add override specifier before comments on inline methods
This commit fixes a "FIXME" in the add-override transform. ' override' was
misplaced when a comment was between the function body and the end of the
'prototype'.
It also remove duplicated check for the main file from the last commit (and
fixes the typo in the comment above).
Author: Guillaume Papin <guillaume.papin at epitech.eu>
Removed:
clang-tools-extra/trunk/test/cpp11-migrate/AddOverride/comment_before_inline_body_fail.cpp
Modified:
clang-tools-extra/trunk/cpp11-migrate/AddOverride/AddOverrideActions.cpp
clang-tools-extra/trunk/docs/AddOverrideTransform.rst
clang-tools-extra/trunk/test/cpp11-migrate/AddOverride/basic.cpp
Modified: clang-tools-extra/trunk/cpp11-migrate/AddOverride/AddOverrideActions.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/AddOverride/AddOverrideActions.cpp?rev=181806&r1=181805&r2=181806&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/AddOverride/AddOverrideActions.cpp (original)
+++ clang-tools-extra/trunk/cpp11-migrate/AddOverride/AddOverrideActions.cpp Tue May 14 12:34:12 2013
@@ -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 MatchFi
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());
Modified: clang-tools-extra/trunk/docs/AddOverrideTransform.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/AddOverrideTransform.rst?rev=181806&r1=181805&r2=181806&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/AddOverrideTransform.rst (original)
+++ clang-tools-extra/trunk/docs/AddOverrideTransform.rst Tue May 14 12:34:12 2013
@@ -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;
};
Modified: clang-tools-extra/trunk/test/cpp11-migrate/AddOverride/basic.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/cpp11-migrate/AddOverride/basic.cpp?rev=181806&r1=181805&r2=181806&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/cpp11-migrate/AddOverride/basic.cpp (original)
+++ clang-tools-extra/trunk/test/cpp11-migrate/AddOverride/basic.cpp Tue May 14 12:34:12 2013
@@ -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.
Removed: clang-tools-extra/trunk/test/cpp11-migrate/AddOverride/comment_before_inline_body_fail.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/cpp11-migrate/AddOverride/comment_before_inline_body_fail.cpp?rev=181805&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/cpp11-migrate/AddOverride/comment_before_inline_body_fail.cpp (original)
+++ clang-tools-extra/trunk/test/cpp11-migrate/AddOverride/comment_before_inline_body_fail.cpp (removed)
@@ -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
-};
-
More information about the cfe-commits
mailing list