from:"Robert Bolter via cfe\-commits"

Re: [PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

2016-03-31 Thread Robert Bolter via cfe-commits

Rob updated this revision to Diff 52227.
Rob added a comment.

I tried

  check_clang_tidy.py modernize-use-override-ms.cpp modernize-use-override temp 
-- -- -fms-extensions

and

  check_clang_tidy.py modernize-use-override-ms.cpp modernize-use-override temp 

on OSX and nethier work as expected.

So instead I have changed the test to use visibility attributes when _MSC_VER 
is not defined.

Also updated the release notes.


http://reviews.llvm.org/D18396

Files:
  clang-tidy/modernize/UseOverrideCheck.cpp
  docs/ReleaseNotes.rst
  test/clang-tidy/modernize-use-override-ms.cpp
  test/clang-tidy/modernize-use-override.cpp

Index: test/clang-tidy/modernize-use-override.cpp
===
--- test/clang-tidy/modernize-use-override.cpp
+++ test/clang-tidy/modernize-use-override.cpp
@@ -21,6 +21,7 @@
   virtual void d2();
   virtual void e() = 0;
   virtual void f() = 0;
+  virtual void f2() const = 0;
   virtual void g() = 0;
 
   virtual void j() const;
@@ -74,7 +75,11 @@
 
   virtual void f()=0;
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
-  // CHECK-FIXES: {{^}}  void f()override =0;
+  // CHECK-FIXES: {{^}}  void f() override =0;
+
+  virtual void f2() const=0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  void f2() const override =0;
 
   virtual void g() ABSTRACT;
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
Index: test/clang-tidy/modernize-use-override-ms.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-override-ms.cpp
@@ -0,0 +1,30 @@
+// RUN: %check_clang_tidy %s modernize-use-override %t -- -- -fms-extensions
+
+// This test is designed to test ms-extension __declspec(dllexport) attributes.
+// On non-windows platforms these are not available so we test visibility attributes instead.
+#if defined(_MSC_VER)
+#  define EXPORT __declspec(dllexport)
+#else
+#  define EXPORT __attribute__((visibility("default")))
+#endif
+
+class Base {
+  virtual EXPORT void a();
+};
+
+class EXPORT InheritedBase {
+  virtual void a();
+};
+
+class Derived : public Base {
+  virtual EXPORT void a();
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  EXPORT void a() override;
+};
+
+class EXPORT InheritedDerived : public InheritedBase {
+  virtual void a();
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void a() override;
+};
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,6 +67,12 @@
 
   This check selectively replaces string literals containing escaped
   characters with raw string literals.
+  
+- Improved ``modernize-use-override`` check
+
+  The fix-its placement around __declspec attributes on windows and other 
+  attributes is improved.
+  
 
 Clang-tidy changes from 3.7 to 3.8
 ^^
Index: clang-tidy/modernize/UseOverrideCheck.cpp
===
--- clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tidy/modernize/UseOverrideCheck.cpp
@@ -95,9 +95,9 @@
: "'override' is";
 StringRef Correct = HasFinal ? "'final'" : "'override'";
 
-Message =
-(llvm::Twine(Redundant) +
- " redundant since the function is already declared " + Correct).str();
+Message = (llvm::Twine(Redundant) +
+   " redundant since the function is already declared " + Correct)
+  .str();
   }
 
   DiagnosticBuilder Diag = diag(Method->getLocation(), Message);
@@ -118,21 +118,24 @@
   if (!HasFinal && !HasOverride) {
 SourceLocation InsertLoc;
 StringRef ReplacementText = "override ";
+SourceLocation MethodLoc = Method->getLocation();
 
 for (Token T : Tokens) {
-  if (T.is(tok::kw___attribute)) {
+  if (T.is(tok::kw___attribute) &&
+  !Sources.isBeforeInTranslationUnit(T.getLocation(), MethodLoc)) {
 InsertLoc = T.getLocation();
 break;
   }
 }
 
 if (Method->hasAttrs()) {
   for (const clang::Attr *A : Method->getAttrs()) {
-if (!A->isImplicit()) {
+if (!A->isImplicit() && !A->isInherited()) {
   SourceLocation Loc =
   Sources.getExpansionLoc(A->getRange().getBegin());
-  if (!InsertLoc.isValid() ||
-  Sources.isBeforeInTranslationUnit(Loc, InsertLoc))
+  if ((!InsertLoc.isValid() ||
+   Sources.isBeforeInTranslationUnit(Loc, InsertLoc)) &&
+  !Sources.isBeforeInTranslationUnit(Loc, MethodLoc))
 InsertLoc = Loc;
 }
   }
@@ -163,6 +166,9 @@
 Tokens.back().

Re: [PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

2016-04-04 Thread Robert Bolter via cfe-commits

Rob updated this revision to Diff 52539.
Rob added a comment.

adding -std=c++11 and it works as expected. yay


http://reviews.llvm.org/D18396

Files:
  clang-tidy/modernize/UseOverrideCheck.cpp
  docs/ReleaseNotes.rst
  test/clang-tidy/modernize-use-override-ms.cpp
  test/clang-tidy/modernize-use-override.cpp

Index: test/clang-tidy/modernize-use-override.cpp
===
--- test/clang-tidy/modernize-use-override.cpp
+++ test/clang-tidy/modernize-use-override.cpp
@@ -21,6 +21,7 @@
   virtual void d2();
   virtual void e() = 0;
   virtual void f() = 0;
+  virtual void f2() const = 0;
   virtual void g() = 0;
 
   virtual void j() const;
@@ -74,7 +75,11 @@
 
   virtual void f()=0;
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
-  // CHECK-FIXES: {{^}}  void f()override =0;
+  // CHECK-FIXES: {{^}}  void f() override =0;
+
+  virtual void f2() const=0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  void f2() const override =0;
 
   virtual void g() ABSTRACT;
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
Index: test/clang-tidy/modernize-use-override-ms.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-override-ms.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s modernize-use-override %t -- -- -fms-extensions -std=c++11
+
+// This test is designed to test ms-extension __declspec(dllexport) attributes.
+#define EXPORT __declspec(dllexport)
+
+class Base {
+  virtual EXPORT void a();
+};
+
+class EXPORT InheritedBase {
+  virtual void a();
+};
+
+class Derived : public Base {
+  virtual EXPORT void a();
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  EXPORT void a() override;
+};
+
+class EXPORT InheritedDerived : public InheritedBase {
+  virtual void a();
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void a() override;
+};
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,6 +67,12 @@
 
   This check selectively replaces string literals containing escaped
   characters with raw string literals.
+  
+- Improved ``modernize-use-override`` check
+
+  The fix-its placement around __declspec attributes on windows and other 
+  attributes is improved.
+  
 
 Clang-tidy changes from 3.7 to 3.8
 ^^
Index: clang-tidy/modernize/UseOverrideCheck.cpp
===
--- clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tidy/modernize/UseOverrideCheck.cpp
@@ -95,9 +95,9 @@
: "'override' is";
 StringRef Correct = HasFinal ? "'final'" : "'override'";
 
-Message =
-(llvm::Twine(Redundant) +
- " redundant since the function is already declared " + Correct).str();
+Message = (llvm::Twine(Redundant) +
+   " redundant since the function is already declared " + Correct)
+  .str();
   }
 
   DiagnosticBuilder Diag = diag(Method->getLocation(), Message);
@@ -118,21 +118,24 @@
   if (!HasFinal && !HasOverride) {
 SourceLocation InsertLoc;
 StringRef ReplacementText = "override ";
+SourceLocation MethodLoc = Method->getLocation();
 
 for (Token T : Tokens) {
-  if (T.is(tok::kw___attribute)) {
+  if (T.is(tok::kw___attribute) &&
+  !Sources.isBeforeInTranslationUnit(T.getLocation(), MethodLoc)) {
 InsertLoc = T.getLocation();
 break;
   }
 }
 
 if (Method->hasAttrs()) {
   for (const clang::Attr *A : Method->getAttrs()) {
-if (!A->isImplicit()) {
+if (!A->isImplicit() && !A->isInherited()) {
   SourceLocation Loc =
   Sources.getExpansionLoc(A->getRange().getBegin());
-  if (!InsertLoc.isValid() ||
-  Sources.isBeforeInTranslationUnit(Loc, InsertLoc))
+  if ((!InsertLoc.isValid() ||
+   Sources.isBeforeInTranslationUnit(Loc, InsertLoc)) &&
+  !Sources.isBeforeInTranslationUnit(Loc, MethodLoc))
 InsertLoc = Loc;
 }
   }
@@ -163,6 +166,9 @@
 Tokens.back().is(tok::kw_delete)) &&
   GetText(Tokens[Tokens.size() - 2], Sources) == "=") {
 InsertLoc = Tokens[Tokens.size() - 2].getLocation();
+// Check if we need to insert a space.
+if ((Tokens[Tokens.size() - 2].getFlags() & Token::LeadingSpace) == 0)
+  ReplacementText = " override ";
   } else if (GetText(Tokens.back(), Sources) == "ABSTRACT") {
 InsertLoc = Tokens.back().getLocation();
   }
___
cfe-

Re: [PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

2016-04-04 Thread Robert Bolter via cfe-commits

Rob updated this revision to Diff 52543.
Rob added a comment.

Adding back quotes in the docs


http://reviews.llvm.org/D18396

Files:
  clang-tidy/modernize/UseOverrideCheck.cpp
  docs/ReleaseNotes.rst
  test/clang-tidy/modernize-use-override-ms.cpp
  test/clang-tidy/modernize-use-override.cpp

Index: test/clang-tidy/modernize-use-override.cpp
===
--- test/clang-tidy/modernize-use-override.cpp
+++ test/clang-tidy/modernize-use-override.cpp
@@ -21,6 +21,7 @@
   virtual void d2();
   virtual void e() = 0;
   virtual void f() = 0;
+  virtual void f2() const = 0;
   virtual void g() = 0;
 
   virtual void j() const;
@@ -74,7 +75,11 @@
 
   virtual void f()=0;
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
-  // CHECK-FIXES: {{^}}  void f()override =0;
+  // CHECK-FIXES: {{^}}  void f() override =0;
+
+  virtual void f2() const=0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  void f2() const override =0;
 
   virtual void g() ABSTRACT;
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
Index: test/clang-tidy/modernize-use-override-ms.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-override-ms.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s modernize-use-override %t -- -- -fms-extensions -std=c++11
+
+// This test is designed to test ms-extension __declspec(dllexport) attributes.
+#define EXPORT __declspec(dllexport)
+
+class Base {
+  virtual EXPORT void a();
+};
+
+class EXPORT InheritedBase {
+  virtual void a();
+};
+
+class Derived : public Base {
+  virtual EXPORT void a();
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  EXPORT void a() override;
+};
+
+class EXPORT InheritedDerived : public InheritedBase {
+  virtual void a();
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void a() override;
+};
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,6 +67,12 @@
 
   This check selectively replaces string literals containing escaped
   characters with raw string literals.
+  
+- Improved ``modernize-use-override`` check
+
+  The fix-its placement around ``__declspec`` attributes on windows and other 
+  attributes is improved.
+  
 
 Clang-tidy changes from 3.7 to 3.8
 ^^
Index: clang-tidy/modernize/UseOverrideCheck.cpp
===
--- clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tidy/modernize/UseOverrideCheck.cpp
@@ -95,9 +95,9 @@
: "'override' is";
 StringRef Correct = HasFinal ? "'final'" : "'override'";
 
-Message =
-(llvm::Twine(Redundant) +
- " redundant since the function is already declared " + Correct).str();
+Message = (llvm::Twine(Redundant) +
+   " redundant since the function is already declared " + Correct)
+  .str();
   }
 
   DiagnosticBuilder Diag = diag(Method->getLocation(), Message);
@@ -118,21 +118,24 @@
   if (!HasFinal && !HasOverride) {
 SourceLocation InsertLoc;
 StringRef ReplacementText = "override ";
+SourceLocation MethodLoc = Method->getLocation();
 
 for (Token T : Tokens) {
-  if (T.is(tok::kw___attribute)) {
+  if (T.is(tok::kw___attribute) &&
+  !Sources.isBeforeInTranslationUnit(T.getLocation(), MethodLoc)) {
 InsertLoc = T.getLocation();
 break;
   }
 }
 
 if (Method->hasAttrs()) {
   for (const clang::Attr *A : Method->getAttrs()) {
-if (!A->isImplicit()) {
+if (!A->isImplicit() && !A->isInherited()) {
   SourceLocation Loc =
   Sources.getExpansionLoc(A->getRange().getBegin());
-  if (!InsertLoc.isValid() ||
-  Sources.isBeforeInTranslationUnit(Loc, InsertLoc))
+  if ((!InsertLoc.isValid() ||
+   Sources.isBeforeInTranslationUnit(Loc, InsertLoc)) &&
+  !Sources.isBeforeInTranslationUnit(Loc, MethodLoc))
 InsertLoc = Loc;
 }
   }
@@ -163,6 +166,9 @@
 Tokens.back().is(tok::kw_delete)) &&
   GetText(Tokens[Tokens.size() - 2], Sources) == "=") {
 InsertLoc = Tokens[Tokens.size() - 2].getLocation();
+// Check if we need to insert a space.
+if ((Tokens[Tokens.size() - 2].getFlags() & Token::LeadingSpace) == 0)
+  ReplacementText = " override ";
   } else if (GetText(Tokens.back(), Sources) == "ABSTRACT") {
 InsertLoc = Tokens.back().getLocation();
   }
___
cfe-commits maili

Re: [PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

2016-04-04 Thread Robert Bolter via cfe-commits

Rob added a comment.

ok, I'm not sure if its worth rolling back the change to 
modernize-use-override-ms.cpp, see ammended comments above.

Apart from this it should be good to go.

I think I am going to need someone to submit the patch for me, I can't use svn 
due to 'error 400' so I've been using the git mirror.

Comment at: test/clang-tidy/modernize-use-override-ms.cpp:1
@@ +1,2 @@
+// RUN: %check_clang_tidy %s modernize-use-override %t
+

alexfh wrote:
> Does this test pass on linux? If no additional compiler arguments needed and 
> it just works, maybe just merge this test code to the main test file?
That's a good question.  I have been running it with the additional 
-fms-extensions argument, but on windows I find it works with or without that 
option.  I havent tested it on linux.  I could maybe rummage up a virtual 
machine and give it a go.

http://reviews.llvm.org/D18396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

2016-04-04 Thread Robert Bolter via cfe-commits

Rob added a comment.

Thanks :)


Repository:
  rL LLVM

http://reviews.llvm.org/D18396



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

clang-tidy modernize-use-override

2016-03-21 Thread Robert Bolter via cfe-commits

Hi,

First time poster here, Please advise...

Can I contribute these patches for clang-tidy modernize-use-override addressing 
two problems:

1: missing spaces on pure function decls

Orig:
void pure() const=0
Problem:
void pure() constoverride =0
Fixed:
void pure() const override =0

2: This is ms-extension specific, but possibly applies to other attribute types 
as I see incorrect placement of override for Inherited Attributes and atts 
located before the method

Orig:
class __declspec(dllexport) X : public Y
  {
void p(); //inherits decl att
  };
Problem:
class override __declspec(dllexport) class X : public Y
  {
  void p();
  };
Fixed:
class __declspec(dllexport) class X : public Y
  {
  void p() override;
  };

I added test/clang-tidy/modernize-use-override-ms.cpp and modified 
modernize-use-override.cpp to test these fixes.


I've also added a -quiet option to clang-tidy/tool/run-clang-tidy.py  and a 
progress countdown which I personally find useful :)


Thanks,
Rob.




Autodesk Limited
Registered Office: One Discovery Place, Columbus Drive, Farnborough, Hampshire 
GU14 0NZ
Registered in England and Wales, No. 1839239


0001-run-clang-tidy-Adding-a-quiet-option-to-suppress-out.patch
Description: 0001-run-clang-tidy-Adding-a-quiet-option-to-suppress-out.patch


0001-modernize-use-override-fix-for-const-0-without-space.patch
Description: 0001-modernize-use-override-fix-for-const-0-without-space.patch
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

2016-03-23 Thread Robert Bolter via cfe-commits

Rob created this revision.
Rob added reviewers: alexfh, aaron.ballman.
Rob added a subscriber: cfe-commits.

This patch is to address 2 problems I found with 
Clang-tidy:modernize-use-override.

1: missing spaces on pure function decls.

Orig:
void pure() const=0
Problem:
void pure() constoverride =0
Fixed:
void pure() const override =0

2: This is ms-extension specific, but possibly applies to other attribute 
types.  The override is placed before the attribute which doesn’t work well 
with declspec as this attribute can be inherited or placed before the method 
identifier.

Orig:
class __declspec(dllexport) X : public Y
  {
  void p();
  };
Problem:
class override __declspec(dllexport) class X : public Y
  {
  void p();
  };
Fixed:
class __declspec(dllexport) class X : public Y
  {
  void p() override;
  };


http://reviews.llvm.org/D18396

Files:
  clang-tidy/modernize/UseOverrideCheck.cpp
  test/clang-tidy/modernize-use-override-ms.cpp
  test/clang-tidy/modernize-use-override.cpp

Index: test/clang-tidy/modernize-use-override.cpp
===
--- test/clang-tidy/modernize-use-override.cpp
+++ test/clang-tidy/modernize-use-override.cpp
@@ -21,6 +21,7 @@
   virtual void d2();
   virtual void e() = 0;
   virtual void f() = 0;
+  virtual void f2() const = 0;
   virtual void g() = 0;
 
   virtual void j() const;
@@ -74,7 +75,11 @@
 
   virtual void f()=0;
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
-  // CHECK-FIXES: {{^}}  void f()override =0;
+  // CHECK-FIXES: {{^}}  void f() override =0;
+
+  virtual void f2() const=0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  void f2() const override =0;
 
   virtual void g() ABSTRACT;
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
Index: test/clang-tidy/modernize-use-override-ms.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-override-ms.cpp
@@ -0,0 +1,45 @@
+// RUN: %check_clang_tidy %s modernize-use-override %t
+
+// This attribute type is inherited and precedes the declaration
+#define EXPORT __declspec(dllexport)
+
+class EXPORT ExpBaseMacro {
+  virtual void a();
+};
+
+class __declspec(dllexport) ExpBase {
+  virtual void a();
+};
+
+class BaseMacro {
+  virtual EXPORT void a();
+};
+
+class Base {
+  virtual __declspec(dllexport) void a();
+};
+
+
+class EXPORT ExpDerivedMacro : public ExpBaseMacro {
+  virtual void a();
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void a() override;
+};
+
+class __declspec(dllexport) ExpDerived : public ExpBase {
+  virtual void a();
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  void a() override;
+};
+
+class DerivedMacro : public BaseMacro {
+  virtual EXPORT void a();
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: prefer using
+  // CHECK-FIXES: {{^}}  EXPORT void a() override;
+};
+
+class Derived : public Base {
+  virtual __declspec(dllexport) void a();
+  // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: prefer using
+  // CHECK-FIXES: {{^}}  __declspec(dllexport) void a() override;
+};
Index: clang-tidy/modernize/UseOverrideCheck.cpp
===
--- clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tidy/modernize/UseOverrideCheck.cpp
@@ -118,25 +118,28 @@
   if (!HasFinal && !HasOverride) {
 SourceLocation InsertLoc;
 StringRef ReplacementText = "override ";
+SourceLocation MethodLoc = Method->getLocation();
 
 for (Token T : Tokens) {
-  if (T.is(tok::kw___attribute)) {
+  if (T.is(tok::kw___attribute) && 
+  !(Sources.isBeforeInTranslationUnit(T.getLocation(), MethodLoc))) {
 InsertLoc = T.getLocation();
 break;
   }
 }
 
 if (Method->hasAttrs()) {
   for (const clang::Attr *A : Method->getAttrs()) {
-if (!A->isImplicit()) {
+if (!A->isImplicit() && !A->isInherited()) {
   SourceLocation Loc =
   Sources.getExpansionLoc(A->getRange().getBegin());
-  if (!InsertLoc.isValid() ||
-  Sources.isBeforeInTranslationUnit(Loc, InsertLoc))
+  if ((!InsertLoc.isValid() ||
+  Sources.isBeforeInTranslationUnit(Loc, InsertLoc)) &&
+  !Sources.isBeforeInTranslationUnit(Loc, MethodLoc))
 InsertLoc = Loc;
+  }
 }
   }
-}
 
 if (InsertLoc.isInvalid() && Method->doesThisDeclarationHaveABody() &&
 Method->getBody() && !Method->isDefaulted()) {
@@ -163,6 +166,9 @@
 Tokens.back().is(tok::kw_delete)) &&
   GetText(Tokens[Tokens.size() - 2], Sources) == "=") {
 InsertLoc = Tokens[Tokens.size() - 2].getLocation();
+// Check if we need to insert a space.
+if ((Tokens[Tokens.

Re: [PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

2016-03-29 Thread Robert Bolter via cfe-commits

Rob updated this revision to Diff 51904.
Rob added a comment.

Update to address review.
Clang-format applied and nit fixed.


http://reviews.llvm.org/D18396

Files:
  clang-tidy/modernize/UseOverrideCheck.cpp
  test/clang-tidy/modernize-use-override-ms.cpp
  test/clang-tidy/modernize-use-override.cpp

Index: test/clang-tidy/modernize-use-override.cpp
===
--- test/clang-tidy/modernize-use-override.cpp
+++ test/clang-tidy/modernize-use-override.cpp
@@ -21,6 +21,7 @@
   virtual void d2();
   virtual void e() = 0;
   virtual void f() = 0;
+  virtual void f2() const = 0;
   virtual void g() = 0;
 
   virtual void j() const;
@@ -74,7 +75,11 @@
 
   virtual void f()=0;
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
-  // CHECK-FIXES: {{^}}  void f()override =0;
+  // CHECK-FIXES: {{^}}  void f() override =0;
+
+  virtual void f2() const=0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  void f2() const override =0;
 
   virtual void g() ABSTRACT;
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
Index: test/clang-tidy/modernize-use-override-ms.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-override-ms.cpp
@@ -0,0 +1,45 @@
+// RUN: %check_clang_tidy %s modernize-use-override %t
+
+// This attribute type is inherited and precedes the declaration
+#define EXPORT __declspec(dllexport)
+
+class EXPORT ExpBaseMacro {
+  virtual void a();
+};
+
+class __declspec(dllexport) ExpBase {
+  virtual void a();
+};
+
+class BaseMacro {
+  virtual EXPORT void a();
+};
+
+class Base {
+  virtual __declspec(dllexport) void a();
+};
+
+
+class EXPORT ExpDerivedMacro : public ExpBaseMacro {
+  virtual void a();
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void a() override;
+};
+
+class __declspec(dllexport) ExpDerived : public ExpBase {
+  virtual void a();
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  void a() override;
+};
+
+class DerivedMacro : public BaseMacro {
+  virtual EXPORT void a();
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: prefer using
+  // CHECK-FIXES: {{^}}  EXPORT void a() override;
+};
+
+class Derived : public Base {
+  virtual __declspec(dllexport) void a();
+  // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: prefer using
+  // CHECK-FIXES: {{^}}  __declspec(dllexport) void a() override;
+};
Index: clang-tidy/modernize/UseOverrideCheck.cpp
===
--- clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tidy/modernize/UseOverrideCheck.cpp
@@ -95,9 +95,9 @@
: "'override' is";
 StringRef Correct = HasFinal ? "'final'" : "'override'";
 
-Message =
-(llvm::Twine(Redundant) +
- " redundant since the function is already declared " + Correct).str();
+Message = (llvm::Twine(Redundant) +
+   " redundant since the function is already declared " + Correct)
+  .str();
   }
 
   DiagnosticBuilder Diag = diag(Method->getLocation(), Message);
@@ -118,21 +118,24 @@
   if (!HasFinal && !HasOverride) {
 SourceLocation InsertLoc;
 StringRef ReplacementText = "override ";
+SourceLocation MethodLoc = Method->getLocation();
 
 for (Token T : Tokens) {
-  if (T.is(tok::kw___attribute)) {
+  if (T.is(tok::kw___attribute) &&
+  !Sources.isBeforeInTranslationUnit(T.getLocation(), MethodLoc)) {
 InsertLoc = T.getLocation();
 break;
   }
 }
 
 if (Method->hasAttrs()) {
   for (const clang::Attr *A : Method->getAttrs()) {
-if (!A->isImplicit()) {
+if (!A->isImplicit() && !A->isInherited()) {
   SourceLocation Loc =
   Sources.getExpansionLoc(A->getRange().getBegin());
-  if (!InsertLoc.isValid() ||
-  Sources.isBeforeInTranslationUnit(Loc, InsertLoc))
+  if ((!InsertLoc.isValid() ||
+   Sources.isBeforeInTranslationUnit(Loc, InsertLoc)) &&
+  !Sources.isBeforeInTranslationUnit(Loc, MethodLoc))
 InsertLoc = Loc;
 }
   }
@@ -163,6 +166,9 @@
 Tokens.back().is(tok::kw_delete)) &&
   GetText(Tokens[Tokens.size() - 2], Sources) == "=") {
 InsertLoc = Tokens[Tokens.size() - 2].getLocation();
+// Check if we need to insert a space.
+if ((Tokens[Tokens.size() - 2].getFlags() & Token::LeadingSpace) == 0)
+  ReplacementText = " override ";
   } else if (GetText(Tokens.back(), Sources) == "ABSTRACT") {
 InsertLoc = Tokens.back().getLocation();
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

Re: [PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

Re: [PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

Re: [PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

Re: [PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

clang-tidy modernize-use-override

[PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

Re: [PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

8 matches

Site Navigation

Mail list logo

Footer information