[clang-tools-extra] r181596 - cpp11-migrate: Fix crash in AddOverride due to template instantiations

[Edwin Vane]

Author: revane
Date: Fri May 10 09:04:58 2013
New Revision: 181596

URL: http://llvm.org/viewvc/llvm-project?rev=181596&view=rev
Log:
cpp11-migrate: Fix crash in AddOverride due to template instantiations

This patch fixes different issues:
- override is not added in template 'contexts' (this will be further improved
  to handle safe situations, a test for this has been written already)
- the main file is now checked before the modifications are applied
- override is not applied now when dealing with pure methods since it was
  misplaced (ignoring it isn't the perfect solution but it seems difficult to
  find the location just before the pure-specifier)

Fixes PR15827

Author: Guillaume Papin <guillaume.papin at epitech.eu>


Added:
    clang-tools-extra/trunk/test/cpp11-migrate/AddOverride/pure_specifier_fail.cpp
Modified:
    clang-tools-extra/trunk/cpp11-migrate/AddOverride/AddOverrideActions.cpp
    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=181596&r1=181595&r2=181596&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/AddOverride/AddOverrideActions.cpp (original)
+++ clang-tools-extra/trunk/cpp11-migrate/AddOverride/AddOverrideActions.cpp Fri May 10 09:04:58 2013
@@ -36,28 +36,40 @@ void AddOverrideFixer::run(const MatchFi
   if (!SM.isFromMainFile(M->getLocStart()))
     return;
 
+  if (!SM.isFromMainFile(M->getLocStart()))
+    return;
+
   // 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;
-    }
+  if (M->hasAttr<OverrideAttr>())
+    return;
+
+  // FIXME: Pure methods are not supported yet as it is difficult to track down
+  // the location of '= 0'.
+  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);
+  } else {
+    StartLoc = SM.getSpellingLoc(M->getLocEnd());
+    StartLoc = Lexer::getLocForEndOfToken(StartLoc, 0, SM, LangOptions());
   }
+  Replace.insert(tooling::Replacement(SM, StartLoc, 0, " override"));
+  ++AcceptedChanges;
 }
-

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=181596&r1=181595&r2=181596&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/cpp11-migrate/AddOverride/basic.cpp (original)
+++ clang-tools-extra/trunk/test/cpp11-migrate/AddOverride/basic.cpp Fri May 10 09:04:58 2013
@@ -108,3 +108,41 @@ public:
   // CHECK: void h() const LLVM_OVERRIDE;
 };
 
+// Test that override isn't added at the wrong place for "pure overrides"
+struct APure {
+  virtual APure *clone() = 0;
+};
+struct BPure : APure {
+  virtual BPure *clone() { return new BPure(); }
+};
+struct CPure : BPure {
+  virtual BPure *clone() = 0;
+  // CHECK: struct CPure : BPure {
+  // CHECK-NOT: virtual BPure *clone() = 0 override;
+  // CHECK: };
+};
+struct DPure : CPure {
+  virtual DPure *clone() { return new DPure(); }
+};
+
+// Test that override is not added on dangerous template constructs
+struct Base1 {
+  virtual void f();
+};
+struct Base2 {};
+template<typename T> struct Derived : T {
+  void f(); // adding 'override' here will break instantiation of Derived<Base2>
+  // CHECK: struct Derived
+  // CHECK: void f();
+};
+Derived<Base1> d1;
+Derived<Base2> d2;
+
+template <typename T>
+class M : public A {
+public:
+  virtual void i();
+  // CHECK: class M : public A {
+  // CHECK: virtual void i() override;
+};
+M<int> b;

Added: clang-tools-extra/trunk/test/cpp11-migrate/AddOverride/pure_specifier_fail.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/cpp11-migrate/AddOverride/pure_specifier_fail.cpp?rev=181596&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/cpp11-migrate/AddOverride/pure_specifier_fail.cpp (added)
+++ clang-tools-extra/trunk/test/cpp11-migrate/AddOverride/pure_specifier_fail.cpp Fri May 10 09:04:58 2013
@@ -0,0 +1,20 @@
+// 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: *
+
+// Test that override isn't placed correctly after "pure overrides"
+struct A {
+  virtual A *clone() = 0;
+};
+struct B : A {
+  virtual B *clone() { return new B(); }
+};
+struct C : B {
+  virtual B *clone() = 0;
+  // CHECK: struct C : B {
+  // CHECK: virtual B *clone() override = 0;
+};
+struct D : C {
+  virtual D *clone() { return new D(); }
+};