[PATCH] cpp11-migrate: Add override specifier before comments on inline methods
Guillaume Papin
guillaume.papin at epitech.eu
Mon May 13 08:04:05 PDT 2013
Address the comments from the review. Update the AddOverride
documentation as the known limitations have changed recently.
Hi revane,
http://llvm-reviews.chandlerc.com/D782
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D782?vs=1929&id=1941#toc
Files:
cpp11-migrate/AddOverride/AddOverrideActions.cpp
docs/AddOverrideTransform.rst
test/cpp11-migrate/AddOverride/basic.cpp
test/cpp11-migrate/AddOverride/comment_before_inline_body_fail.cpp
Index: cpp11-migrate/AddOverride/AddOverrideActions.cpp
===================================================================
--- cpp11-migrate/AddOverride/AddOverrideActions.cpp
+++ cpp11-migrate/AddOverride/AddOverrideActions.cpp
@@ -26,16 +26,41 @@
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 @@
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());
Index: docs/AddOverrideTransform.rst
===================================================================
--- docs/AddOverrideTransform.rst
+++ docs/AddOverrideTransform.rst
@@ -28,20 +28,17 @@
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;
};
Index: test/cpp11-migrate/AddOverride/basic.cpp
===================================================================
--- test/cpp11-migrate/AddOverride/basic.cpp
+++ test/cpp11-migrate/AddOverride/basic.cpp
@@ -57,8 +57,12 @@
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.
Index: test/cpp11-migrate/AddOverride/comment_before_inline_body_fail.cpp
===================================================================
--- 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
-};
-
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D782.3.patch
Type: text/x-patch
Size: 5370 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130513/01f41e85/attachment.bin>
More information about the cfe-commits
mailing list