[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

2020-03-18 Thread Nathan James via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9bb5685b2161: [clang-tidy] 
misc-unconventional-assign-operator suggest to use rvalue… (authored by 
tetsuo-cpp, committed by njames93).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75901/new/

https://reviews.llvm.org/D75901

Files:
  clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator-precxx11.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator-precxx11.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator-precxx11.cpp
@@ -0,0 +1,6 @@
+// RUN: %check_clang_tidy -std=c++98,c++03 %s 
misc-unconventional-assign-operator %t
+
+struct BadArgument {
+  BadArgument =(BadArgument &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should take 
'BadArgument const&' or 'BadArgument'
+};
Index: clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
@@ -75,7 +75,10 @@
   } else {
 static const char *const Messages[][2] = {
 {"ReturnType", "operator=() should return '%0&'"},
-{"ArgumentType", "operator=() should take '%0 const&', '%0&&' or 
'%0'"},
+{"ArgumentType",
+ getLangOpts().CPlusPlus11
+ ? "operator=() should take '%0 const&', '%0&&' or '%0'"
+ : "operator=() should take '%0 const&' or '%0'"},
 {"cv", "operator=() should not be marked '%1'"}};
 
 const auto *Method = Result.Nodes.getNodeAs("method");


Index: clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator-precxx11.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator-precxx11.cpp
@@ -0,0 +1,6 @@
+// RUN: %check_clang_tidy -std=c++98,c++03 %s misc-unconventional-assign-operator %t
+
+struct BadArgument {
+  BadArgument =(BadArgument &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should take 'BadArgument const&' or 'BadArgument'
+};
Index: clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
@@ -75,7 +75,10 @@
   } else {
 static const char *const Messages[][2] = {
 {"ReturnType", "operator=() should return '%0&'"},
-{"ArgumentType", "operator=() should take '%0 const&', '%0&&' or '%0'"},
+{"ArgumentType",
+ getLangOpts().CPlusPlus11
+ ? "operator=() should take '%0 const&', '%0&&' or '%0'"
+ : "operator=() should take '%0 const&' or '%0'"},
 {"cv", "operator=() should not be marked '%1'"}};
 
 const auto *Method = Result.Nodes.getNodeAs("method");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

2020-03-18 Thread Alex Cameron via Phabricator via cfe-commits 
tetsuo-cpp added a comment.

In D75901#1920397 , @njames93 wrote:

> In D75901#1916119 , @tetsuo-cpp 
> wrote:
>
> > @njames93 @MaskRay 
> >  Thanks for helping me with testing. I'll remember this for next time.
> >
> > I also saw this pattern of retrieving `LangOptions` from an `ASTContext` in 
> > some other checks (`RedundantExpressionCheck` and `StaticAssertCheck`) but 
> > I didn't change them to use `getLangOpts` since the `ASTContext` is used 
> > for other stuff.
>
>
> Its also not a good idea to change unrelated code in a review


Understood. Thanks @njames93.
Could you please help me commit this change?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75901/new/

https://reviews.llvm.org/D75901



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

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

2020-03-12 Thread Nathan James via Phabricator via cfe-commits 
njames93 added a comment.

In D75901#1916119 , @tetsuo-cpp wrote:

> @njames93 @MaskRay 
>  Thanks for helping me with testing. I'll remember this for next time.
>
> I also saw this pattern of retrieving `LangOptions` from an `ASTContext` in 
> some other checks (`RedundantExpressionCheck` and `StaticAssertCheck`) but I 
> didn't change them to use `getLangOpts` since the `ASTContext` is used for 
> other stuff.


Its also not a good idea to change unrelated code in a review


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75901/new/

https://reviews.llvm.org/D75901



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

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

2020-03-11 Thread Nathan James via Phabricator via cfe-commits 
njames93 accepted this revision.
njames93 added a comment.
This revision is now accepted and ready to land.

LGTM
For the record `check-clang-tools` is sufficient for testing all clang tidy 
checks


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75901/new/

https://reviews.llvm.org/D75901



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

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

2020-03-10 Thread Alex Cameron via Phabricator via cfe-commits 
tetsuo-cpp marked an inline comment as done.
tetsuo-cpp added a comment.

@njames93 @MaskRay 
Thanks for helping me with testing. I'll remember this for next time.

I also saw this pattern of retrieving `LangOptions` from an `ASTContext` in 
some other checks (`RedundantExpressionCheck` and `StaticAssertCheck`) but I 
didn't change them to use `getLangOpts` since the `ASTContext` is used for 
other stuff.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75901/new/

https://reviews.llvm.org/D75901



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

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

2020-03-10 Thread Alex Cameron via Phabricator via cfe-commits 
tetsuo-cpp updated this revision to Diff 249538.
tetsuo-cpp added a comment.

Address review comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75901/new/

https://reviews.llvm.org/D75901

Files:
  clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator-precxx11.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator-precxx11.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator-precxx11.cpp
@@ -0,0 +1,6 @@
+// RUN: %check_clang_tidy -std=c++98,c++03 %s 
misc-unconventional-assign-operator %t
+
+struct BadArgument {
+  BadArgument =(BadArgument &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should take 
'BadArgument const&' or 'BadArgument'
+};
Index: clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
@@ -75,7 +75,10 @@
   } else {
 static const char *const Messages[][2] = {
 {"ReturnType", "operator=() should return '%0&'"},
-{"ArgumentType", "operator=() should take '%0 const&', '%0&&' or 
'%0'"},
+{"ArgumentType",
+ getLangOpts().CPlusPlus11
+ ? "operator=() should take '%0 const&', '%0&&' or '%0'"
+ : "operator=() should take '%0 const&' or '%0'"},
 {"cv", "operator=() should not be marked '%1'"}};
 
 const auto *Method = Result.Nodes.getNodeAs("method");


Index: clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator-precxx11.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator-precxx11.cpp
@@ -0,0 +1,6 @@
+// RUN: %check_clang_tidy -std=c++98,c++03 %s misc-unconventional-assign-operator %t
+
+struct BadArgument {
+  BadArgument =(BadArgument &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should take 'BadArgument const&' or 'BadArgument'
+};
Index: clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
@@ -75,7 +75,10 @@
   } else {
 static const char *const Messages[][2] = {
 {"ReturnType", "operator=() should return '%0&'"},
-{"ArgumentType", "operator=() should take '%0 const&', '%0&&' or '%0'"},
+{"ArgumentType",
+ getLangOpts().CPlusPlus11
+ ? "operator=() should take '%0 const&', '%0&&' or '%0'"
+ : "operator=() should take '%0 const&' or '%0'"},
 {"cv", "operator=() should not be marked '%1'"}};
 
 const auto *Method = Result.Nodes.getNodeAs("method");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

2020-03-10 Thread Nathan James via Phabricator via cfe-commits 
njames93 added a comment.

If you need help for the test case. Create a file in the 
`clang-tools-extra/test/clang-tidy/checkers`. 
Name it something like `misc-unconventional-assign-operator-pre11.cpp` and 
paste this in

  // RUN: %check_clang_tidy -std=c++98,c++03 %s 
misc-unconventional-assign-operator %t
  
  struct BadArgument {
BadArgument& operator=(BadArgument&);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should take 
'BadArgument const&' or 'BadArgument'
  };


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75901/new/

https://reviews.llvm.org/D75901



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

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

2020-03-10 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

In D75901#1914254 , @njames93 wrote:

> Please add a test case for this


More concretely, a `check_clang_tidy -std=c++98` test in 
`test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75901/new/

https://reviews.llvm.org/D75901



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

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

2020-03-10 Thread Nathan James via Phabricator via cfe-commits 
njames93 added a comment.

Please add a test case for this




Comment at: 
clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp:76-77
   } else {
+const ASTContext *ASTCtx = Result.Context;
+const LangOptions  = ASTCtx->getLangOpts();
 static const char *const Messages[][2] = {

Remove these lines and change below to be `getLangOpts().CPlusPlus11`. 
`getLangOpts()` is a protected method in `ClangTidyCheck`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75901/new/

https://reviews.llvm.org/D75901



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

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

2020-03-10 Thread Alex Cameron via Phabricator via cfe-commits 
tetsuo-cpp created this revision.
tetsuo-cpp added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
tetsuo-cpp added reviewers: njames93, MaskRay.

Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=27702
I wasn't sure how this type of thing is usually tested. So any advice would be 
appreciated.
`check-llvm`, `check-clang` and `check-clang-tools` are clean for me.
**C++98**

  tetsuo@garland-c-16-sgp1-01:~/dev/llvm-project/test$ cat compile_commands.json
  [
  {
"directory": "/home/tetsuo/dev/llvm-project/test",
"command": "/usr/bin/c++  -std=gnu++98 -o 
CMakeFiles/test.dir/test.cpp.o -c /home/tetsuo/dev/llvm-project/test/test.cpp",
"file": "/home/tetsuo/dev/llvm-project/test/test.cpp"
  }
  ]
  tetsuo@garland-c-16-sgp1-01:~/dev/llvm-project/test$ ../build/bin/clang-tidy 
--checks=misc-unconventional-assign-operator test.cpp
  3053 warnings generated.
  /home/tetsuo/dev/llvm-project/test/test.cpp:7:3: warning: operator=() should 
take 'Foo const&' or 'Foo' [misc-unconventional-assign-operator]
Foo =(Foo ) {
^
  Suppressed 3052 warnings (3052 in non-user code).
  Use -header-filter=.* to display errors from all non-system headers. Use 
-system-headers to display errors from system headers as well.

**C++17**

  tetsuo@garland-c-16-sgp1-01:~/dev/llvm-project/test$ cat compile_commands.json
  [
  {
"directory": "/home/tetsuo/dev/llvm-project/test",
"command": "/usr/bin/c++  -std=gnu++17 -o 
CMakeFiles/test.dir/test.cpp.o -c /home/tetsuo/dev/llvm-project/test/test.cpp",
"file": "/home/tetsuo/dev/llvm-project/test/test.cpp"
  }
  ]
  tetsuo@garland-c-16-sgp1-01:~/dev/llvm-project/test$ ../build/bin/clang-tidy 
--checks=misc-unconventional-assign-operator test.cpp
  5377 warnings generated.
  /home/tetsuo/dev/llvm-project/test/test.cpp:7:3: warning: operator=() should 
take 'Foo const&', 'Foo&&' or 'Foo' [misc-unconventional-assign-operator]
Foo =(Foo ) {
^
  Suppressed 5376 warnings (5376 in non-user code).
  Use -header-filter=.* to display errors from all non-system headers. Use 
-system-headers to display errors from system headers as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75901

Files:
  clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp


Index: clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
@@ -73,9 +73,14 @@
   if (const auto *RetStmt = Result.Nodes.getNodeAs("returnStmt")) {
 diag(RetStmt->getBeginLoc(), "operator=() should always return '*this'");
   } else {
+const ASTContext *ASTCtx = Result.Context;
+const LangOptions  = ASTCtx->getLangOpts();
 static const char *const Messages[][2] = {
 {"ReturnType", "operator=() should return '%0&'"},
-{"ArgumentType", "operator=() should take '%0 const&', '%0&&' or 
'%0'"},
+{"ArgumentType",
+ Opts.CPlusPlus11
+ ? "operator=() should take '%0 const&', '%0&&' or '%0'"
+ : "operator=() should take '%0 const&' or '%0'"},
 {"cv", "operator=() should not be marked '%1'"}};
 
 const auto *Method = Result.Nodes.getNodeAs("method");


Index: clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
@@ -73,9 +73,14 @@
   if (const auto *RetStmt = Result.Nodes.getNodeAs("returnStmt")) {
 diag(RetStmt->getBeginLoc(), "operator=() should always return '*this'");
   } else {
+const ASTContext *ASTCtx = Result.Context;
+const LangOptions  = ASTCtx->getLangOpts();
 static const char *const Messages[][2] = {
 {"ReturnType", "operator=() should return '%0&'"},
-{"ArgumentType", "operator=() should take '%0 const&', '%0&&' or '%0'"},
+{"ArgumentType",
+ Opts.CPlusPlus11
+ ? "operator=() should take '%0 const&', '%0&&' or '%0'"
+ : "operator=() should take '%0 const&' or '%0'"},
 {"cv", "operator=() should not be marked '%1'"}};
 
 const auto *Method = Result.Nodes.getNodeAs("method");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits