[clang-tools-extra] r361735 - DeleteNullPointerCheck now deletes until the end brace of the condition.

[Mads Ravn via cfe-commits]

Author: madsravn
Date: Sun May 26 10:00:38 2019
New Revision: 361735

URL: http://llvm.org/viewvc/llvm-project?rev=361735=rev
Log:
DeleteNullPointerCheck now deletes until the end brace of the condition.

Patch by Jonathan Camilleri

Differential Revision https://reviews.llvm.org/D61861

Modified:
clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-delete-null-pointer.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp?rev=361735=361734=361735=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp 
Sun May 26 10:00:38 2019
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "DeleteNullPointerCheck.h"
+#include "../utils/LexerUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
@@ -62,9 +63,11 @@ void DeleteNullPointerCheck::check(const
 
   Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
   IfWithDelete->getBeginLoc(),
-  Lexer::getLocForEndOfToken(IfWithDelete->getCond()->getEndLoc(), 0,
- *Result.SourceManager,
- Result.Context->getLangOpts(;
+  utils::lexer::getPreviousToken(IfWithDelete->getThen()->getBeginLoc(),
+ *Result.SourceManager,
+ Result.Context->getLangOpts())
+  .getLocation()));
+
   if (Compound) {
 Diag << FixItHint::CreateRemoval(
 CharSourceRange::getTokenRange(Compound->getLBracLoc()));

Modified: 
clang-tools-extra/trunk/test/clang-tidy/readability-delete-null-pointer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-delete-null-pointer.cpp?rev=361735=361734=361735=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/readability-delete-null-pointer.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-delete-null-pointer.cpp 
Sun May 26 10:00:38 2019
@@ -3,6 +3,15 @@
 #define NULL 0
 
 void f() {
+  int *ps = 0;
+  if (ps /**/) // #0
+delete ps;
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'if' statement is unnecessary; 
deleting null pointer has no effect [readability-delete-null-pointer]
+
+  // CHECK-FIXES: int *ps = 0;
+  // CHECK-FIXES-NEXT: {{^  }}// #0
+  // CHECK-FIXES-NEXT: delete ps;
+
   int *p = 0;
 
   // #1


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

[clang-tools-extra] r301167 - [clang-tidy] New check: modernize-replace-random-shuffle.

[Mads Ravn via cfe-commits]

Author: madsravn
Date: Mon Apr 24 04:27:20 2017
New Revision: 301167

URL: http://llvm.org/viewvc/llvm-project?rev=301167=rev
Log:
[clang-tidy] New check: modernize-replace-random-shuffle.

This check will find occurrences of ``std::random_shuffle`` and replace it with 
``std::shuffle``. In C++17 ``std::random_shuffle`` will no longer be available 
and thus we need to replace it.

Example of case that it fixes

```
  std::vector v;

  // First example
  std::random_shuffle(vec.begin(), vec.end());

```

Reviewers: hokein, aaron.ballman, alexfh, malcolm.parsons, mclow.lists

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D30158

Added:
clang-tools-extra/trunk/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/ReplaceRandomShuffleCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-replace-random-shuffle.rst
clang-tools-extra/trunk/test/clang-tidy/modernize-replace-random-shuffle.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt?rev=301167=301166=301167=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt Mon Apr 24 
04:27:20 2017
@@ -13,6 +13,7 @@ add_clang_library(clangTidyModernizeModu
   RawStringLiteralCheck.cpp
   RedundantVoidArgCheck.cpp
   ReplaceAutoPtrCheck.cpp
+  ReplaceRandomShuffleCheck.cpp
   ReturnBracedInitListCheck.cpp
   ShrinkToFitCheck.cpp
   UseAutoCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp?rev=301167=301166=301167=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp Mon 
Apr 24 04:27:20 2017
@@ -19,6 +19,7 @@
 #include "RawStringLiteralCheck.h"
 #include "RedundantVoidArgCheck.h"
 #include "ReplaceAutoPtrCheck.h"
+#include "ReplaceRandomShuffleCheck.h"
 #include "ReturnBracedInitListCheck.h"
 #include "ShrinkToFitCheck.h"
 #include "UseAutoCheck.h"
@@ -54,6 +55,8 @@ public:
 "modernize-redundant-void-arg");
 CheckFactories.registerCheck(
 "modernize-replace-auto-ptr");
+CheckFactories.registerCheck(
+"modernize-replace-random-shuffle");
 CheckFactories.registerCheck(
 "modernize-return-braced-init-list");
 CheckFactories.registerCheck("modernize-shrink-to-fit");

Added: 
clang-tools-extra/trunk/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp?rev=301167=auto
==
--- clang-tools-extra/trunk/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp 
(added)
+++ clang-tools-extra/trunk/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp 
Mon Apr 24 04:27:20 2017
@@ -0,0 +1,109 @@
+//===--- ReplaceRandomShuffleCheck.cpp - 
clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ReplaceRandomShuffleCheck.h"
+#include "../utils/FixItHintUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+ReplaceRandomShuffleCheck::ReplaceRandomShuffleCheck(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+  Options.get("IncludeStyle", "llvm"))) {}
+
+void ReplaceRandomShuffleCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus11)
+return;
+
+  const auto Begin = hasArgument(0, expr());
+  const auto End = hasArgument(1, expr());
+  const auto RandomFunc = hasArgument(2, expr().bind("randomFunc"));
+  Finder->addMatcher(
+  callExpr(anyOf(allOf(Begin, End, argumentCountIs(2)),
+   

[clang-tools-extra] r294913 - [clang-tidy] Fix for bug 31838: readability-delete-null-pointer does not work for class members

[Mads Ravn via cfe-commits]

Author: madsravn
Date: Sun Feb 12 14:35:42 2017
New Revision: 294913

URL: http://llvm.org/viewvc/llvm-project?rev=294913=rev
Log:
[clang-tidy] Fix for bug 31838: readability-delete-null-pointer does not work 
for class members

Fix for commit r294912 which had a small error in the AST matcher.

Modified:
clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp?rev=294913=294912=294913=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp 
Sun Feb 12 14:35:42 2017
@@ -43,7 +43,7 @@ void DeleteNullPointerCheck::registerMat
   ifStmt(hasCondition(anyOf(PointerCondition, 
BinaryPointerCheckCondition)),
  hasThen(anyOf(
  DeleteExpr, DeleteMemberExpr,
- compoundStmt(has(anyOf(DeleteExpr, DeleteMemberExpr)),
+ compoundStmt(anyOf(has(DeleteExpr), has(DeleteMemberExpr)),
   statementCountIs(1))
  .bind("compound"
   .bind("ifWithDelete"),


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

[clang-tools-extra] r294912 - [clang-tidy] Fix for bug 31838: readability-delete-null-pointer does not work for class members

[Mads Ravn via cfe-commits]

Author: madsravn
Date: Sun Feb 12 14:09:59 2017
New Revision: 294912

URL: http://llvm.org/viewvc/llvm-project?rev=294912=rev
Log:
[clang-tidy] Fix for bug 31838: readability-delete-null-pointer does not work 
for class members

I have made a small fix for readability-delete-null-pointer check so it also 
checks for class members.

Example of case that it fixes
```
  struct A {
void foo() {
  if(mp)
delete mp;
}
int *mp;
  };
```

Reviewers: JDevlieghere, aaron.ballman, alexfh, malcolm.parsons

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D29726

Modified:
clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.h
clang-tools-extra/trunk/test/clang-tidy/readability-delete-null-pointer.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp?rev=294912=294911=294912=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp 
Sun Feb 12 14:09:59 2017
@@ -24,8 +24,15 @@ void DeleteNullPointerCheck::registerMat
 to(decl(equalsBoundNode("deletedPointer"
   .bind("deleteExpr");
 
-  const auto PointerExpr =
-  ignoringImpCasts(declRefExpr(to(decl().bind("deletedPointer";
+  const auto DeleteMemberExpr =
+  cxxDeleteExpr(has(castExpr(has(memberExpr(hasDeclaration(
+
fieldDecl(equalsBoundNode("deletedMemberPointer"
+  .bind("deleteMemberExpr");
+
+  const auto PointerExpr = ignoringImpCasts(anyOf(
+  declRefExpr(to(decl().bind("deletedPointer"))),
+  memberExpr(hasDeclaration(fieldDecl().bind("deletedMemberPointer");
+
   const auto PointerCondition = castExpr(hasCastKind(CK_PointerToBoolean),
  hasSourceExpression(PointerExpr));
   const auto BinaryPointerCheckCondition =
@@ -34,9 +41,11 @@ void DeleteNullPointerCheck::registerMat
 
   Finder->addMatcher(
   ifStmt(hasCondition(anyOf(PointerCondition, 
BinaryPointerCheckCondition)),
- hasThen(anyOf(DeleteExpr,
-   compoundStmt(has(DeleteExpr), statementCountIs(1))
-   .bind("compound"
+ hasThen(anyOf(
+ DeleteExpr, DeleteMemberExpr,
+ compoundStmt(has(anyOf(DeleteExpr, DeleteMemberExpr)),
+  statementCountIs(1))
+ .bind("compound"
   .bind("ifWithDelete"),
   this);
 }

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.h?rev=294912=294911=294912=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.h 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.h Sun 
Feb 12 14:09:59 2017
@@ -16,7 +16,8 @@ namespace clang {
 namespace tidy {
 namespace readability {
 
-/// Check whether the 'if' statement is unnecessary before calling 'delete' on 
a pointer.
+/// Check whether the 'if' statement is unnecessary before calling 'delete' on 
a
+/// pointer.
 ///
 /// For the user-facing documentation see:
 /// 
http://clang.llvm.org/extra/clang-tidy/checks/readability-delete-null-pointer.html

Modified: 
clang-tools-extra/trunk/test/clang-tidy/readability-delete-null-pointer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-delete-null-pointer.cpp?rev=294912=294911=294912=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/readability-delete-null-pointer.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-delete-null-pointer.cpp 
Sun Feb 12 14:09:59 2017
@@ -59,6 +59,16 @@ void f() {
   } else {
 c2 = c;
   }
+  struct A {
+void foo() {
+  if (mp) // #6
+delete mp;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: 'if' statement is 
unnecessary; deleting null pointer has no effect 
[readability-delete-null-pointer]
+  // CHECK-FIXES: {{^  }}// #6
+  // CHECK-FIXES-NEXT: delete mp;
+}
+int *mp;
+  };
 }
 
 void g() {


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

r290766 - [clang] Minor fix to libASTMatcherTutorial

[Mads Ravn via cfe-commits]

Author: madsravn
Date: Fri Dec 30 14:49:44 2016
New Revision: 290766

URL: http://llvm.org/viewvc/llvm-project?rev=290766=rev
Log:
[clang] Minor fix to libASTMatcherTutorial

There was a small error in the code in the tutorial. The tutorial contains a 
few errors which results in code not being able to compile.

One error was described here: https://llvm.org/bugs/show_bug.cgi?id=25583 .

I found and fixed the error and one additional error.

Reviewers: aaron.ballman, malcolm.parsons

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D28180

Modified:
cfe/trunk/docs/LibASTMatchersTutorial.rst

Modified: cfe/trunk/docs/LibASTMatchersTutorial.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersTutorial.rst?rev=290766=290765=290766=diff
==
--- cfe/trunk/docs/LibASTMatchersTutorial.rst (original)
+++ cfe/trunk/docs/LibASTMatchersTutorial.rst Fri Dec 30 14:49:44 2016
@@ -496,9 +496,9 @@ And change ``LoopPrinter::run`` to
 
   void LoopPrinter::run(const MatchFinder::MatchResult ) {
 ASTContext *Context = Result.Context;
-const ForStmt *FS = Result.Nodes.getStmtAs("forLoop");
+const ForStmt *FS = Result.Nodes.getNodeAs("forLoop");
 // We do not want to convert header files!
-if (!FS || 
!Context->getSourceManager().isFromMainFile(FS->getForLoc()))
+if (!FS || 
!Context->getSourceManager().isWrittenInMainFile(FS->getForLoc()))
   return;
 const VarDecl *IncVar = Result.Nodes.getNodeAs("incVarName");
 const VarDecl *CondVar = 
Result.Nodes.getNodeAs("condVarName");


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

[clang-tools-extra] r290747 - [clang-tidy] Add check 'misc-string-compare'.

[Mads Ravn via cfe-commits]

Author: madsravn
Date: Fri Dec 30 04:09:46 2016
New Revision: 290747

URL: http://llvm.org/viewvc/llvm-project?rev=290747=rev
Log:
[clang-tidy] Add check 'misc-string-compare'.

I have a created a new check for clang tidy: misc-string-compare. This will 
check for incorrect usage of std::string::compare when used to check equality 
or inequality of string instead of the string equality or inequality operators.

Example:
```
  std::string str1, str2;
  if (str1.compare(str2)) {
  }
```

Reviewers: hokein, aaron.ballman, alexfh, malcolm.parsons

Subscribers: xazax.hun, Eugene.Zelenko, cfe-commits, malcolm.parsons, Prazek, 
mgorny, JDevlieghere

Differential Revision: https://reviews.llvm.org/D27210

Added:
clang-tools-extra/trunk/clang-tidy/misc/StringCompareCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/StringCompareCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-string-compare.rst
clang-tools-extra/trunk/test/clang-tidy/misc-string-compare.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=290747=290746=290747=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Fri Dec 30 04:09:46 
2016
@@ -28,6 +28,7 @@ add_clang_library(clangTidyMiscModule
   SizeofContainerCheck.cpp
   SizeofExpressionCheck.cpp
   StaticAssertCheck.cpp
+  StringCompareCheck.cpp
   StringConstructorCheck.cpp
   StringIntegerAssignmentCheck.cpp
   StringLiteralWithEmbeddedNulCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=290747=290746=290747=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Fri Dec 30 
04:09:46 2016
@@ -35,6 +35,7 @@
 #include "SizeofContainerCheck.h"
 #include "SizeofExpressionCheck.h"
 #include "StaticAssertCheck.h"
+#include "StringCompareCheck.h"
 #include "StringConstructorCheck.h"
 #include "StringIntegerAssignmentCheck.h"
 #include "StringLiteralWithEmbeddedNulCheck.h"
@@ -106,6 +107,7 @@ public:
 CheckFactories.registerCheck(
 "misc-sizeof-expression");
 CheckFactories.registerCheck("misc-static-assert");
+CheckFactories.registerCheck("misc-string-compare");
 CheckFactories.registerCheck(
 "misc-string-constructor");
 CheckFactories.registerCheck(

Added: clang-tools-extra/trunk/clang-tidy/misc/StringCompareCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/StringCompareCheck.cpp?rev=290747=auto
==
--- clang-tools-extra/trunk/clang-tidy/misc/StringCompareCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/StringCompareCheck.cpp Fri Dec 30 
04:09:46 2016
@@ -0,0 +1,82 @@
+//===--- MiscStringCompare.cpp - 
clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "StringCompareCheck.h"
+#include "../utils/FixItHintUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+static const StringRef CompareMessage = "do not use 'compare' to test equality 
"
+"of strings; use the string equality "
+"operator instead";
+
+void StringCompareCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+return;
+
+  const auto StrCompare = cxxMemberCallExpr(
+  callee(cxxMethodDecl(hasName("compare"),
+   ofClass(classTemplateSpecializationDecl(
+   hasName("::std::basic_string"),
+  hasArgument(0, expr().bind("str2")), argumentCountIs(1),
+  callee(memberExpr().bind("str1")));
+
+  // First and second case: cast str.compare(str) to boolean.
+  
Finder->addMatcher(implicitCastExpr(hasImplicitDestinationType(booleanType()),
+  has(StrCompare))
+ .bind("match1"),
+

Re: [PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

[Mads Ravn via cfe-commits]

Hi,

The last mail was only meant to contain the question about the comment to
misc-suspicious-string-compare check.

Do you reckon I should remove that match from my check? Or should we move
it?

Best regards,
Mads Ravn

On Mon, Dec 26, 2016 at 8:48 PM Mads Ravn via Phabricator <
revi...@reviews.llvm.org> wrote:

> madsravn marked 2 inline comments as done.
> madsravn added inline comments.
>
>
> 
> Comment at: clang-tidy/misc/MiscStringCompareCheck.h:24
> +///
> http://clang.llvm.org/extra/clang-tidy/checks/misc-string-compare-check.html
> +class MiscStringCompareCheck : public ClangTidyCheck {
> +public:
> 
> malcolm.parsons wrote:
> > Remove `Misc`.
> >
> > Did you use add_new_check.py to add this check?
> No, but the files I was looking at had the same naming convention. Maybe
> something has changed in that regards recently?
>
> I will fix it.
>
>
> 
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:25
> +"operator instead";
> +static const StringRef GuaranteeMessage = "'compare' is not guaranteed to
> "
> +  "return -1 or 1; check for
> bigger or "
> 
> malcolm.parsons wrote:
> > misc-suspicious-string-compare warns about suspicious `strcmp()`; maybe
> it should handle `string::compare()` too.
> Do you suggest that I move this check to misc-suspicious-string-compare?
> Or should we just remove it from here?
>
>
> 
> Comment at: docs/clang-tidy/checks/misc-string-compare.rst:10
> +equality or inequality operators. The compare method is intended for
> sorting
> +functions and thus returns ``-1``, ``0`` or ``1`` depending on the
> lexicographical
> +relationship between the strings compared. If an equality or inequality
> check
> 
> xazax.hun wrote:
> > As far as I remember this is not true. The  ``compare`` method can
> return any integer number, only the sign is defined. It is not guaranteed
> to return -1 or 1 in case of inequality.
> This is true. I checked it - it is just some implementations which tend to
> use -1, 0 and 1. However, the specification says negative, 0 and positive.
> I will correct it. Thanks
>
>
> 
> Comment at: test/clang-tidy/misc-string-compare.cpp:9
> +
> +  if(str1.compare(str2)) {}
> +  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: do not use compare to test
> equality of strings; use the string equality operator instead
> [misc-string-compare]
> 
> malcolm.parsons wrote:
> > Some other test ideas:
> >
> > ```
> > if (str1.compare("foo")) {}
> >
> > return str1.compare(str2) == 0;
> >
> > func(str1.compare(str2) != 0);
> >
> > if (str2.empty() || str1.compare(str2) != 0) {}
> > ```
> None of those fit the ast match.
>
> I think it's fine as it is now. If the matcher will be expanded to check
> for some of those cases, I think more test cases are needed.
>
>
> https://reviews.llvm.org/D27210
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

[Mads Ravn via cfe-commits]

Hi Piotr,

Thank you for your detailed comments :)

I would love some help with the other fixit. I have some notes on it at
home. But my main catch is that is an implicit cast to boolean from
str.compare(str) with maybe an ! in front of it. And I need to fix that to
str.compare(str) == 0 or str.compare(str) != 0.

Where should I put the warning in a static const global variable? Is it
still in StringCompare.cpp or do we have a  joint files for these?

Best regards,
Mads Ravn

On Sun, Dec 18, 2016 at 11:26 PM Piotr Padlewski via Phabricator <
revi...@reviews.llvm.org> wrote:

> Prazek added a comment.
>
> Do you need some help with implementing the other fixit, or you just need
> some extra time? It seems to be almost the same as your second fixit
>
>
>
> 
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:69-70
> +diag(Matched->getLocStart(),
> + "do not use 'compare' to test equality of strings; "
> + "use the string equality operator instead");
> +
> 
> Take this warning to some static const global variable
>
>
> 
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:71
> + "use the string equality operator instead");
> +
> +  if (const auto *Matched = Result.Nodes.getNodeAs("match2")) {
> 
> match1 and match2 are in different matchers (passed to register matchers)?
>
> If so put return here after diag to finish control flow for this case.
>
>
> 
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:81
> +  auto Diag = diag(Matched->getLocStart(),
> +   "do not use 'compare' to test equality of strings;
> "
> +   "use the string equality operator instead");
> 
> and use it here
>
>
> 
> Comment at: clang-tidy/misc/StringCompareCheck.h:10-11
> +
> +#ifndef STRING_COMPARE_CHECK_H
> +#define STRING_COMPARE_CHECK_H
> +
> 
> This should be much longer string. Do you know about ./add_new_check?
>
> Please make one similar to other checks
>
>
> 
> Comment at: clang-tidy/misc/StringCompareCheck.h:36
> +
> +#endif // STRING_COMPARE_CHECK_H
> 
> DITTO
>
>
> 
> Comment at: test/clang-tidy/misc-string-compare.cpp:35-40
> +  if (str1.compare(str2)) {
> +  }
> +  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: do not use 'compare' to test
> equality of strings; use the string equality operator instead
> [misc-string-compare]
> +  if (!str1.compare(str2)) {
> +  }
> +  // CHECK-MESSAGES: [[@LINE-2]]:8: warning: do not use 'compare' to test
> equality of strings; use the string equality operator instead
> [misc-string-compare]
> 
> Why this one doesn't have fixit hint?
>
>
> 
> Comment at: test/clang-tidy/misc-string-compare.cpp:70
> +  }
> +  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: do not use 'compare' to test
> equality of strings;
> +  if (str3->compare(str2) == 0) {
> 
> no fixit?
>
>
> https://reviews.llvm.org/D27210
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

[Mads Ravn via cfe-commits]

Hi Piotr,

That is a good point. Because it is not always -1 or 1 that determines
lexicographical higher or lower.

However, I don't think that is in the scope of this check. This check
checks for string comparison (equality or inequality). Adding a match for
if the user is using the compare function semantically wrong might make the
check too ambiguous. Since str.compare(str) == 0 will check for equality
and str.compare(str) != 0 will check for inequality. But str.compare(str)
== 1 will check whether one string is lexicographically smaller than the
other (and == -1 also). What do you think?

Best regards,
Mads Ravn

On Thu, Dec 15, 2016 at 8:17 PM Piotr Padlewski via Phabricator <
revi...@reviews.llvm.org> wrote:

> Prazek added a comment.
>
> Good job.
> I think it is resonable to warn in cases like:
>
>   if (str.compare(str2) == 1)
>
> or even
>
>   if(str.compare(str2) == -1)
>
> Sometimes people check for 1 or -1 instead of > 0 and < 0. If I remember
> corectly PVS studio found some bugs like this.
>
>
>
> 
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:27
> +   hasName("::std::basic_string"),
> +  hasArgument(0, declRefExpr()), callee(memberExpr()));
> +
> 
> malcolm.parsons wrote:
> > I don't think you need to check what the first argument is.
> +1, just check if you are calling function with 1 argument.
> you can still use hasArgument(0, expr().bind("str2")) to bind argument
>
>
> 
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:25
> +return;
> +  const auto strCompare = cxxMemberCallExpr(
> +  callee(cxxMethodDecl(hasName("compare"),
> 
> Start with upper case
>
>
> https://reviews.llvm.org/D27210
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

[Mads Ravn via cfe-commits]

Hi Malcolm,

I will look into fixing the two cases only.
argumentCountIs(1) is sufficient to narrow the matching to only string
compare with one argument.

Best regards,
Mads Ravn

On Mon, Dec 12, 2016 at 10:38 AM Malcolm Parsons via Phabricator <
revi...@reviews.llvm.org> wrote:

> malcolm.parsons added inline comments.
>
>
> 
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:25
> +  callee(cxxMethodDecl(hasName("compare"),
> +   ofClass(classTemplateSpecializationDecl(
> +   hasName("::std::basic_string"),
> 
> malcolm.parsons wrote:
> > malcolm.parsons wrote:
> > > This needs to check that it's one of the single parameter overloads of
> compare.
> > Add `parameterCountIs(1)`.
> Actually, the `argumentCountIs(1)` below should be sufficient.
>
>
> https://reviews.llvm.org/D27210
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

[Mads Ravn via cfe-commits]

Hi guys,

Do you have any extra comments for this?

Best regards

On Sat, Dec 3, 2016 at 1:34 PM Mads Ravn via Phabricator <
revi...@reviews.llvm.org> wrote:

> madsravn updated this revision to Diff 80177.
> madsravn added a comment.
>
> Did as comments suggested: Fixed the description about compare returning
> -1, 0 or 1. Fixed the ast matcher to only find compare with one argument.
> Clang-formatted everything. Added a new test (str.compare("foo")) and wrote
> a FIXME for the fixit.
>
>
> https://reviews.llvm.org/D27210
>
> Files:
>   clang-tidy/misc/CMakeLists.txt
>   clang-tidy/misc/MiscTidyModule.cpp
>   clang-tidy/misc/StringCompareCheck.cpp
>   clang-tidy/misc/StringCompareCheck.h
>   docs/ReleaseNotes.rst
>   docs/clang-tidy/checks/list.rst
>   docs/clang-tidy/checks/misc-string-compare.rst
>   test/clang-tidy/misc-string-compare.cpp
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

[Mads Ravn via cfe-commits]

Hi Malcolm,

Thanks. I will fix the last couple of things in the weekend and hopefully
have something worth showing there.

alexfh suggested that fixits seemed easy to implement. I am having a few
doubts as to how I would make fixits for case 1 & 2. How important would it
be to implement fixits at this point?

Best regards,
Mads Ravn

On Fri, Dec 2, 2016 at 10:57 AM Malcolm Parsons 
wrote:

> On 2 December 2016 at 09:50, Mads Ravn  wrote:
> >> Comment at: test/clang-tidy/misc-string-compare.cpp:9
> >> +  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: do not use compare to test
> >> equality of strings; use the string equality operator instead
> >> [misc-string-compare]
> > What do you mean by this comment?
>
> I was replying to my previous line comment, but the line that was
> commented on has changed since.
>
> My comment was:
>
> There's still no test for the single parameter c-string overload.
>
> --
> Malcolm Parsons
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

[Mads Ravn via cfe-commits]

Hi Malcolm,

Matching for the single parameter overload of compare is probably a good
idea. I will do that.

> Comment at: test/clang-tidy/misc-string-compare.cpp:9
> +  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: do not use compare to test
equality of strings; use the string equality operator instead
[misc-string-compare]
What do you mean by this comment?

Best regards,
Mads Ravn

On Fri, Dec 2, 2016 at 10:26 AM Malcolm Parsons via Phabricator <
revi...@reviews.llvm.org> wrote:

> malcolm.parsons added inline comments.
>
>
> 
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:25
> +  callee(cxxMethodDecl(hasName("compare"),
> +   ofClass(classTemplateSpecializationDecl(
> +   hasName("::std::basic_string"),
> 
> This needs to check that it's one of the single parameter overloads of
> compare.
>
>
> 
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:27
> +   hasName("::std::basic_string"),
> +  hasArgument(0, declRefExpr()), callee(memberExpr()));
> +
> 
> I don't think you need to check what the first argument is.
>
>
> 
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:39
> +  binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
> + hasEitherOperand(strCompare),
> +
>  hasEitherOperand(integerLiteral(equals(0
> 
> Is this clang-formatted?
>
>
> 
> Comment at: test/clang-tidy/misc-string-compare.cpp:9
> +
> +  if(str1.compare(str2)) {}
> +  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: do not use compare to test
> equality of strings; use the string equality operator instead
> [misc-string-compare]
> 
> malcolm.parsons wrote:
> > Some other test ideas:
> >
> > ```
> > if (str1.compare("foo")) {}
> >
> > return str1.compare(str2) == 0;
> >
> > func(str1.compare(str2) != 0);
> >
> > if (str2.empty() || str1.compare(str2) != 0) {}
> > ```
> There's still no test for the single parameter c-string overload.
>
>
> https://reviews.llvm.org/D27210
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

[Mads Ravn via cfe-commits]

Hi Malcolm,

Thanks for the suggestions, I have been reading up on the fixits.
My initial four cases has been reduced to two a little more general cases:
1 & 2: implicitCast to bool str1.compare(str2). This case covers both
!str1.compare(str2) and str1.compare(str2)
3 & 4: str1.compare(str2) == 0 and str1.compare(str2) != 0.

I see the idea for the fixit clearly for case 3 & 4. Just erase
.compare(str2) and replace 0 with str2. I have a quick question though:
Given the declRefExpr().bind("str2"), how do I read the name of it in
clang-tidy? Or should I just bind 0 as well and then create replacement
with str where const auto str = Result.Nodes.getNodeAs("str2") ?

Where I seem to find a little trouble is how to fixit case 1 & 2 now that
they are reduced to one case. How do I check whether or not there is a
unary operator in front of the implicitCast?

Thank you,
Mads Ravn

On Thu, Dec 1, 2016 at 8:53 PM Mads Ravn via Phabricator <
revi...@reviews.llvm.org> wrote:

> madsravn updated this revision to Diff 79961.
> madsravn added a comment.
>
> Fixed broken tests.
>
>
> https://reviews.llvm.org/D27210
>
> Files:
>   clang-tidy/misc/CMakeLists.txt
>   clang-tidy/misc/MiscTidyModule.cpp
>   clang-tidy/misc/StringCompareCheck.cpp
>   clang-tidy/misc/StringCompareCheck.h
>   docs/ReleaseNotes.rst
>   docs/clang-tidy/checks/list.rst
>   docs/clang-tidy/checks/misc-string-compare.rst
>   test/clang-tidy/misc-string-compare.cpp
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

[Mads Ravn via cfe-commits]

Hi Alexander,

I have now implemented your suggestions - all but the fixit one. If I have
added bindings for str1 and str2 in ast matcher, how would I go about
creating a replacement for the entire implicitCastExpr or binaryOperator? I
can't find any example in the code for clang-tidy to suggest how I would
build a new node for the AST. Or I am looking at it from a wrong direction?

Could you hint me in the right direction as to how I would make the fixit?

Best regards,
Mads Ravn

On Thu, Dec 1, 2016 at 3:41 PM Alexander Kornienko via Phabricator <
revi...@reviews.llvm.org> wrote:

> alexfh requested changes to this revision.
> alexfh added inline comments.
> This revision now requires changes to proceed.
>
>
> 
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:29
> +
> +  // First and second case: cast str.compare(str) to boolean
> +  Finder->addMatcher(
> 
> Please add trailing periods here and elsewhere.
>
>
> 
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:36-50
> +  // Third case: str.compare(str) == 0
> +  Finder->addMatcher(
> +  binaryOperator(hasOperatorName("=="),
> + hasEitherOperand(strCompare),
> +
>  hasEitherOperand(integerLiteral(equals(0
> +  .bind("match"),
> +  this);
> 
> These two matchers can be merged to avoid repetition.
>
>
> 
> Comment at: clang-tidy/misc/StringCompareCheck.cpp:55-57
> +diag(Matched->getLocStart(),
> + "do not use compare to test equality of strings; "
> + "use the string equality operator instead");
> 
> It looks like it's relatively straightforward to implement fixit hints.
> WDYT?
>
>
> 
> Comment at: test/clang-tidy/misc-string-compare.cpp:29
> +  if(!str1.compare(str2)) {}
> +  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: do not use compare to test
> equality of strings; use the string equality operator instead
> [misc-string-compare]
> +  if(str1.compare(str2) == 0) {}
> 
> Truncate all CHECK patterns after the first one after `of strings;`
>
>
> https://reviews.llvm.org/D27210
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

[Mads Ravn via cfe-commits]

I think I got it. I will throw a new diff up within the hour.

Thanks for the ideas :)

On Wed, Nov 30, 2016 at 6:48 PM Malcolm Parsons 
wrote:

> On 30 November 2016 at 17:18, Mads Ravn  wrote:
> > So remove the ifStmt from the third and fourth case?
>
> I was thinking all cases.
> Can the first case be restricted to casts to bool?
> If not, keep the cast to int case with an ifStmt and add a cast to bool
> case.
> Does it matter whether the cast is implicit?
>
> --
> Malcolm Parsons
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

[Mads Ravn via cfe-commits]

So remove the ifStmt from the third and fourth case?

So that I keep if(str1.compare(str2)) and if(!str1.compare(str2)), and
change the other two to str1.compare(str2) == 0 and str1.compare(str2) != 0
?

That makes good sense. Then I could also add some of the test cases you
mentioned earlier.

On Wed, Nov 30, 2016 at 5:59 PM Malcolm Parsons via Phabricator <
revi...@reviews.llvm.org> wrote:

> malcolm.parsons added a comment.
>
> I don't know why you're restricting this check to only match within the
> condition of an if statement.
>
>
> https://reviews.llvm.org/D27210
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r279803 - [clang-tidy] Added hh, hxx and hpp to header guard checks.

[Mads Ravn via cfe-commits]

Author: madsravn
Date: Fri Aug 26 00:59:53 2016
New Revision: 279803

URL: http://llvm.org/viewvc/llvm-project?rev=279803=rev
Log:
[clang-tidy] Added hh, hxx and hpp to header guard checks.

Changed the extension check to include the option of ",h,hh,hpp,hxx" instead of 
just returning whether the file ended with ".h".

Differential revision: https://reviews.llvm.org/D20512

Modified:
clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.cpp
clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.h
clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.h
clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.h
clang-tools-extra/trunk/docs/clang-tidy/checks/llvm-header-guard.rst

Modified: clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.cpp?rev=279803=279802=279803=diff
==
--- clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.cpp Fri Aug 26 
00:59:53 2016
@@ -13,8 +13,8 @@ namespace clang {
 namespace tidy {
 namespace llvm {
 
-bool LLVMHeaderGuardCheck::shouldFixHeaderGuard(StringRef Filename) {
-  return Filename.endswith(".h");
+bool LLVMHeaderGuardCheck::shouldFixHeaderGuard(StringRef FileName) {
+  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
 }
 
 std::string LLVMHeaderGuardCheck::getHeaderGuard(StringRef Filename,

Modified: clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.h?rev=279803=279802=279803=diff
==
--- clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.h Fri Aug 26 
00:59:53 2016
@@ -17,13 +17,30 @@ namespace tidy {
 namespace llvm {
 
 /// Finds and fixes header guards that do not adhere to LLVM style.
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/llvm-header-guard.html
+/// The check supports these options:
+///   - `HeaderFileExtensions`: a comma-separated list of filename extensions 
of
+/// header files (The filename extension should not contain "." prefix).
+/// ",h,hh,hpp,hxx" by default.
+/// For extension-less header files, using an empty string or leaving an
+/// empty string between "," if there are other filename extensions.
 class LLVMHeaderGuardCheck : public utils::HeaderGuardCheck {
 public:
   LLVMHeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
-  : HeaderGuardCheck(Name, Context) {}
+  : HeaderGuardCheck(Name, Context),
+RawStringHeaderFileExtensions(
+Options.getLocalOrGlobal("HeaderFileExtensions", ",h,hh,hpp,hxx")) 
{
+utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+ HeaderFileExtensions, ',');
+  }
   bool shouldSuggestEndifComment(StringRef Filename) override { return false; }
   bool shouldFixHeaderGuard(StringRef Filename) override;
   std::string getHeaderGuard(StringRef Filename, StringRef OldGuard) override;
+
+private:
+  std::string RawStringHeaderFileExtensions;
+  utils::HeaderFileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace llvm

Modified: clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp?rev=279803=279802=279803=diff
==
--- clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp Fri 
Aug 26 00:59:53 2016
@@ -61,6 +61,15 @@ bool parseHeaderFileExtensions(StringRef
   return true;
 }
 
+bool isHeaderFileExtension(StringRef FileName,
+   HeaderFileExtensionsSet HeaderFileExtensions) {
+  StringRef extension = ::llvm::sys::path::extension(FileName);
+  if (extension.startswith("."))
+extension = extension.substr(1);
+
+  return HeaderFileExtensions.count(extension) > 0;
+}
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang

Modified: clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.h?rev=279803=279802=279803=diff
==
--- clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.h 
(original)
+++ 

Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files

[Mads Ravn via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL279803: [clang-tidy] Added hh, hxx and hpp to header guard 
checks. (authored by madsravn).

Changed prior to commit:
  https://reviews.llvm.org/D20512?vs=69250=69320#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D20512

Files:
  clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tools-extra/trunk/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
  clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
  clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.h
  clang-tools-extra/trunk/docs/clang-tidy/checks/llvm-header-guard.rst

Index: clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.h
===
--- clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -12,8 +12,8 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace clang {
 namespace tidy {
@@ -41,6 +41,10 @@
HeaderFileExtensionsSet ,
char delimiter);
 
+/// \brief Decides whether a file has a header file extension.
+bool isHeaderFileExtension(StringRef FileName,
+   HeaderFileExtensionsSet HeaderFileExtensions);
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
===
--- clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExtensionsUtils.cpp
@@ -61,6 +61,15 @@
   return true;
 }
 
+bool isHeaderFileExtension(StringRef FileName,
+   HeaderFileExtensionsSet HeaderFileExtensions) {
+  StringRef extension = ::llvm::sys::path::extension(FileName);
+  if (extension.startswith("."))
+extension = extension.substr(1);
+
+  return HeaderFileExtensions.count(extension) > 0;
+}
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.h
@@ -11,16 +11,28 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARD_H
 
 #include "../ClangTidy.h"
+#include "../utils/HeaderFileExtensionsUtils.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 
 /// Finds and fixes header guards.
+/// The check supports these options:
+///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
+/// header files (The filename extension should not contain "." prefix).
+/// ",h,hh,hpp,hxx" by default.
+/// For extension-less header files, using an empty string or leaving an
+/// empty string between "," if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+RawStringHeaderFileExtensions(
+Options.getLocalOrGlobal("HeaderFileExtensions", ",h,hh,hpp,hxx")) {
+utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+ HeaderFileExtensions, ',');
+  }
   void registerPPCallbacks(CompilerInstance ) override;
 
   /// Returns ``true`` if the check should suggest inserting a trailing comment
@@ -39,6 +51,10 @@
   /// Gets the canonical header guard for a file.
   virtual std::string getHeaderGuard(StringRef Filename,
  StringRef OldGuard = StringRef()) = 0;
+
+private:
+  std::string RawStringHeaderFileExtensions;
+  utils::HeaderFileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace utils
Index: clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
===
--- clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/HeaderGuard.cpp
@@ -20,7 +20,7 @@
 
 /// \brief canonicalize a path by removing ./ and ../ components.
 static std::string cleanPath(StringRef Path) {
-  SmallString<256> Result =  Path;
+  SmallString<256> Result = Path;
   llvm::sys::path::remove_dots(Result, true);
   return Result.str();
 }
@@ -274,13 +274,13 @@
 }
 
 bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
-  return FileName.endswith(".h");
+  return utils::isHeaderFileExtension(FileName, 

Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files

[Mads Ravn via cfe-commits]

madsravn updated this revision to Diff 69250.
madsravn added a comment.

Last change - documentation should be fine now.


https://reviews.llvm.org/D20512

Files:
  clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tidy/llvm/HeaderGuardCheck.h
  clang-tidy/utils/HeaderFileExtensionsUtils.cpp
  clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tidy/utils/HeaderGuard.cpp
  clang-tidy/utils/HeaderGuard.h
  docs/clang-tidy/checks/llvm-header-guard.rst

Index: docs/clang-tidy/checks/llvm-header-guard.rst
===
--- docs/clang-tidy/checks/llvm-header-guard.rst
+++ docs/clang-tidy/checks/llvm-header-guard.rst
@@ -3,4 +3,11 @@
 llvm-header-guard
 =
 
-TODO: add docs
+Finds and fixes header guards that do not adhere to LLVM style.
+
+Options
+---
+
+.. option:: HeaderFileExtensions
+
+  A comma-separated list of filename extensions of header files (The filename extension should not contain "." prefix). Default value is ",h,hh,hpp,hxx".  For extension-less header files, using an empty string or leaving an empty string between "," if there are other filename extensions.
Index: clang-tidy/utils/HeaderGuard.h
===
--- clang-tidy/utils/HeaderGuard.h
+++ clang-tidy/utils/HeaderGuard.h
@@ -11,16 +11,28 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARD_H
 
 #include "../ClangTidy.h"
+#include "../utils/HeaderFileExtensionsUtils.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 
 /// Finds and fixes header guards.
+/// The check supports these options:
+///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
+/// header files (The filename extension should not contain "." prefix).
+/// ",h,hh,hpp,hxx" by default.
+/// For extension-less header files, using an empty string or leaving an
+/// empty string between "," if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+RawStringHeaderFileExtensions(
+Options.getLocalOrGlobal("HeaderFileExtensions", ",h,hh,hpp,hxx")) {
+utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+ HeaderFileExtensions, ',');
+  }
   void registerPPCallbacks(CompilerInstance ) override;
 
   /// \brief Returns true if the checker should suggest inserting a trailing
@@ -39,6 +51,10 @@
   /// \brief Get the canonical header guard for a file.
   virtual std::string getHeaderGuard(StringRef Filename,
  StringRef OldGuard = StringRef()) = 0;
+
+private:
+  std::string RawStringHeaderFileExtensions;
+  utils::HeaderFileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace utils
Index: clang-tidy/utils/HeaderGuard.cpp
===
--- clang-tidy/utils/HeaderGuard.cpp
+++ clang-tidy/utils/HeaderGuard.cpp
@@ -20,7 +20,7 @@
 
 /// \brief canonicalize a path by removing ./ and ../ components.
 static std::string cleanPath(StringRef Path) {
-  SmallString<256> Result =  Path;
+  SmallString<256> Result = Path;
   llvm::sys::path::remove_dots(Result, true);
   return Result.str();
 }
@@ -274,13 +274,13 @@
 }
 
 bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
-  return FileName.endswith(".h");
+  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
 }
 
 bool HeaderGuardCheck::shouldFixHeaderGuard(StringRef FileName) { return true; }
 
 bool HeaderGuardCheck::shouldSuggestToAddHeaderGuard(StringRef FileName) {
-  return FileName.endswith(".h");
+  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
 }
 
 std::string HeaderGuardCheck::formatEndIf(StringRef HeaderGuard) {
Index: clang-tidy/utils/HeaderFileExtensionsUtils.h
===
--- clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -12,8 +12,8 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace clang {
 namespace tidy {
@@ -41,6 +41,10 @@
HeaderFileExtensionsSet ,
char delimiter);
 
+/// \brief Decides whether a file has a header file extension.
+bool isHeaderFileExtension(StringRef FileName,
+   HeaderFileExtensionsSet HeaderFileExtensions);
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tidy/utils/HeaderFileExtensionsUtils.cpp
===
--- clang-tidy/utils/HeaderFileExtensionsUtils.cpp
+++ 

Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files

[Mads Ravn via cfe-commits]

madsravn added inline comments.


Comment at: docs/clang-tidy/checks/llvm-header-guard.rst:13
@@ +12,3 @@
+
+  ...
+

alexfh wrote:
> `...` was meant to represent the description of the option. Not literally 
> `...` ;)
> 
> The description should be indented by at least two columns and should start 
> with `A comma-separated ...`
Sorry :) I just found the literal `  ...` in some of the other .rst files and 
it somewhat fit the context. I'll get it fixed.


https://reviews.llvm.org/D20512



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

Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files

[Mads Ravn via cfe-commits]

madsravn updated this revision to Diff 69164.

https://reviews.llvm.org/D20512

Files:
  clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tidy/llvm/HeaderGuardCheck.h
  clang-tidy/utils/HeaderFileExtensionsUtils.cpp
  clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tidy/utils/HeaderGuard.cpp
  clang-tidy/utils/HeaderGuard.h
  docs/clang-tidy/checks/llvm-header-guard.rst

Index: docs/clang-tidy/checks/llvm-header-guard.rst
===
--- docs/clang-tidy/checks/llvm-header-guard.rst
+++ docs/clang-tidy/checks/llvm-header-guard.rst
@@ -3,4 +3,11 @@
 llvm-header-guard
 =
 
-TODO: add docs
+Finds and fixes header guards that do not adhere to LLVM style.
+
+Options
+---
+
+.. option:: HeaderFileExtensions
+
+A comma-separated list of filename extensions of header files (The filename extension should not contain "." prefix). Default value is ",h,hh,hpp,hxx".  For extension-less header files, using an empty string or leaving an empty string between "," if there are other filename extensions.
Index: clang-tidy/utils/HeaderGuard.h
===
--- clang-tidy/utils/HeaderGuard.h
+++ clang-tidy/utils/HeaderGuard.h
@@ -11,16 +11,28 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARD_H
 
 #include "../ClangTidy.h"
+#include "../utils/HeaderFileExtensionsUtils.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 
 /// Finds and fixes header guards.
+/// The check supports these options:
+///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
+/// header files (The filename extension should not contain "." prefix).
+/// ",h,hh,hpp,hxx" by default.
+/// For extension-less header files, using an empty string or leaving an
+/// empty string between "," if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+RawStringHeaderFileExtensions(
+Options.getLocalOrGlobal("HeaderFileExtensions", ",h,hh,hpp,hxx")) {
+utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+ HeaderFileExtensions, ',');
+  }
   void registerPPCallbacks(CompilerInstance ) override;
 
   /// \brief Returns true if the checker should suggest inserting a trailing
@@ -39,6 +51,10 @@
   /// \brief Get the canonical header guard for a file.
   virtual std::string getHeaderGuard(StringRef Filename,
  StringRef OldGuard = StringRef()) = 0;
+
+private:
+  std::string RawStringHeaderFileExtensions;
+  utils::HeaderFileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace utils
Index: clang-tidy/utils/HeaderGuard.cpp
===
--- clang-tidy/utils/HeaderGuard.cpp
+++ clang-tidy/utils/HeaderGuard.cpp
@@ -20,7 +20,7 @@
 
 /// \brief canonicalize a path by removing ./ and ../ components.
 static std::string cleanPath(StringRef Path) {
-  SmallString<256> Result =  Path;
+  SmallString<256> Result = Path;
   llvm::sys::path::remove_dots(Result, true);
   return Result.str();
 }
@@ -274,13 +274,13 @@
 }
 
 bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
-  return FileName.endswith(".h");
+  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
 }
 
 bool HeaderGuardCheck::shouldFixHeaderGuard(StringRef FileName) { return true; }
 
 bool HeaderGuardCheck::shouldSuggestToAddHeaderGuard(StringRef FileName) {
-  return FileName.endswith(".h");
+  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
 }
 
 std::string HeaderGuardCheck::formatEndIf(StringRef HeaderGuard) {
Index: clang-tidy/utils/HeaderFileExtensionsUtils.h
===
--- clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -12,8 +12,8 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace clang {
 namespace tidy {
@@ -41,6 +41,10 @@
HeaderFileExtensionsSet ,
char delimiter);
 
+/// \brief Decides whether a file has a header file extension.
+bool isHeaderFileExtension(StringRef FileName,
+   HeaderFileExtensionsSet HeaderFileExtensions);
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tidy/utils/HeaderFileExtensionsUtils.cpp
===
--- clang-tidy/utils/HeaderFileExtensionsUtils.cpp
+++ clang-tidy/utils/HeaderFileExtensionsUtils.cpp
@@ -61,6 +61,15 @@
   return true;
 }
 
+bool 

Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files

[Mads Ravn via cfe-commits]

madsravn updated this revision to Diff 69158.
madsravn marked 2 inline comments as done.
madsravn added a comment.

Documentation fix.


https://reviews.llvm.org/D20512

Files:
  clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tidy/llvm/HeaderGuardCheck.h
  clang-tidy/utils/HeaderFileExtensionsUtils.cpp
  clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tidy/utils/HeaderGuard.cpp
  clang-tidy/utils/HeaderGuard.h
  docs/clang-tidy/checks/llvm-header-guard.rst

Index: docs/clang-tidy/checks/llvm-header-guard.rst
===
--- docs/clang-tidy/checks/llvm-header-guard.rst
+++ docs/clang-tidy/checks/llvm-header-guard.rst
@@ -3,4 +3,13 @@
 llvm-header-guard
 =
 
-TODO: add docs
+Finds and fixes header guards that do not adhere to LLVM style.
+
+Options
+---
+
+.. option:: HeaderFileExtensions
+
+  ...
+
+``HeaderFileExtensions``: a comma-separated list of filename extensions of header files (The filename extension should not contain "." prefix). ",h,hh,hpp,hxx" by default. For extension-less header files, using an empty string or leaving an empty string between "," if there are other filename extensions.
Index: clang-tidy/utils/HeaderGuard.h
===
--- clang-tidy/utils/HeaderGuard.h
+++ clang-tidy/utils/HeaderGuard.h
@@ -11,16 +11,28 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARD_H
 
 #include "../ClangTidy.h"
+#include "../utils/HeaderFileExtensionsUtils.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 
 /// Finds and fixes header guards.
+/// The check supports these options:
+///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
+/// header files (The filename extension should not contain "." prefix).
+/// ",h,hh,hpp,hxx" by default.
+/// For extension-less header files, using an empty string or leaving an
+/// empty string between "," if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+RawStringHeaderFileExtensions(
+Options.getLocalOrGlobal("HeaderFileExtensions", ",h,hh,hpp,hxx")) {
+utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+ HeaderFileExtensions, ',');
+  }
   void registerPPCallbacks(CompilerInstance ) override;
 
   /// \brief Returns true if the checker should suggest inserting a trailing
@@ -39,6 +51,10 @@
   /// \brief Get the canonical header guard for a file.
   virtual std::string getHeaderGuard(StringRef Filename,
  StringRef OldGuard = StringRef()) = 0;
+
+private:
+  std::string RawStringHeaderFileExtensions;
+  utils::HeaderFileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace utils
Index: clang-tidy/utils/HeaderGuard.cpp
===
--- clang-tidy/utils/HeaderGuard.cpp
+++ clang-tidy/utils/HeaderGuard.cpp
@@ -20,7 +20,7 @@
 
 /// \brief canonicalize a path by removing ./ and ../ components.
 static std::string cleanPath(StringRef Path) {
-  SmallString<256> Result =  Path;
+  SmallString<256> Result = Path;
   llvm::sys::path::remove_dots(Result, true);
   return Result.str();
 }
@@ -274,13 +274,13 @@
 }
 
 bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
-  return FileName.endswith(".h");
+  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
 }
 
 bool HeaderGuardCheck::shouldFixHeaderGuard(StringRef FileName) { return true; }
 
 bool HeaderGuardCheck::shouldSuggestToAddHeaderGuard(StringRef FileName) {
-  return FileName.endswith(".h");
+  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
 }
 
 std::string HeaderGuardCheck::formatEndIf(StringRef HeaderGuard) {
Index: clang-tidy/utils/HeaderFileExtensionsUtils.h
===
--- clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -12,8 +12,8 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace clang {
 namespace tidy {
@@ -41,6 +41,10 @@
HeaderFileExtensionsSet ,
char delimiter);
 
+/// \brief Decides whether a file has a header file extension.
+bool isHeaderFileExtension(StringRef FileName,
+   HeaderFileExtensionsSet HeaderFileExtensions);
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tidy/utils/HeaderFileExtensionsUtils.cpp
===
--- clang-tidy/utils/HeaderFileExtensionsUtils.cpp

Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files

[Mads Ravn via cfe-commits]

madsravn updated this revision to Diff 69152.
madsravn marked 5 inline comments as done.
madsravn added a comment.

More suggestions by alexfh fixed.


https://reviews.llvm.org/D20512

Files:
  clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tidy/llvm/HeaderGuardCheck.h
  clang-tidy/utils/HeaderFileExtensionsUtils.cpp
  clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tidy/utils/HeaderGuard.cpp
  clang-tidy/utils/HeaderGuard.h
  docs/clang-tidy/checks/llvm-header-guard.rst

Index: docs/clang-tidy/checks/llvm-header-guard.rst
===
--- docs/clang-tidy/checks/llvm-header-guard.rst
+++ docs/clang-tidy/checks/llvm-header-guard.rst
@@ -3,4 +3,9 @@
 llvm-header-guard
 =
 
-TODO: add docs
+Finds and fixes header guards that do not adhere to LLVM style.
+
+For the user-facing documentation see: http://clang.llvm.org/extra/clang-tidy/checks/llvm-header-guard.html
+
+The check supports these options:
+``HeaderFileExtensions``: a comma-separated list of filename extensions of header files (The filename extension should not contain "." prefix). ",h,hh,hpp,hxx" by default. For extension-less header files, using an empty string or leaving an empty string between "," if there are other filename extensions.
Index: clang-tidy/utils/HeaderGuard.h
===
--- clang-tidy/utils/HeaderGuard.h
+++ clang-tidy/utils/HeaderGuard.h
@@ -11,16 +11,28 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARD_H
 
 #include "../ClangTidy.h"
+#include "../utils/HeaderFileExtensionsUtils.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 
 /// Finds and fixes header guards.
+/// The check supports these options:
+///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
+/// header files (The filename extension should not contain "." prefix).
+/// ",h,hh,hpp,hxx" by default.
+/// For extension-less header files, using an empty string or leaving an
+/// empty string between "," if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+RawStringHeaderFileExtensions(
+Options.getLocalOrGlobal("HeaderFileExtensions", ",h,hh,hpp,hxx")) {
+utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+ HeaderFileExtensions, ',');
+  }
   void registerPPCallbacks(CompilerInstance ) override;
 
   /// \brief Returns true if the checker should suggest inserting a trailing
@@ -39,6 +51,10 @@
   /// \brief Get the canonical header guard for a file.
   virtual std::string getHeaderGuard(StringRef Filename,
  StringRef OldGuard = StringRef()) = 0;
+
+private:
+  std::string RawStringHeaderFileExtensions;
+  utils::HeaderFileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace utils
Index: clang-tidy/utils/HeaderGuard.cpp
===
--- clang-tidy/utils/HeaderGuard.cpp
+++ clang-tidy/utils/HeaderGuard.cpp
@@ -20,7 +20,7 @@
 
 /// \brief canonicalize a path by removing ./ and ../ components.
 static std::string cleanPath(StringRef Path) {
-  SmallString<256> Result =  Path;
+  SmallString<256> Result = Path;
   llvm::sys::path::remove_dots(Result, true);
   return Result.str();
 }
@@ -274,13 +274,13 @@
 }
 
 bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
-  return FileName.endswith(".h");
+  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
 }
 
 bool HeaderGuardCheck::shouldFixHeaderGuard(StringRef FileName) { return true; }
 
 bool HeaderGuardCheck::shouldSuggestToAddHeaderGuard(StringRef FileName) {
-  return FileName.endswith(".h");
+  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
 }
 
 std::string HeaderGuardCheck::formatEndIf(StringRef HeaderGuard) {
Index: clang-tidy/utils/HeaderFileExtensionsUtils.h
===
--- clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -12,8 +12,8 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace clang {
 namespace tidy {
@@ -41,6 +41,10 @@
HeaderFileExtensionsSet ,
char delimiter);
 
+/// \brief Decides whether a file has a header file extension.
+bool isHeaderFileExtension(StringRef FileName,
+   HeaderFileExtensionsSet HeaderFileExtensions);
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tidy/utils/HeaderFileExtensionsUtils.cpp

Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files

[Mads Ravn via cfe-commits]

madsravn updated this revision to Diff 69143.
madsravn added a comment.

Fixed suggested by alexfh


https://reviews.llvm.org/D20512

Files:
  clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tidy/llvm/HeaderGuardCheck.h
  clang-tidy/utils/HeaderFileExtensionsUtils.cpp
  clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tidy/utils/HeaderGuard.cpp
  clang-tidy/utils/HeaderGuard.h

Index: clang-tidy/utils/HeaderGuard.h
===
--- clang-tidy/utils/HeaderGuard.h
+++ clang-tidy/utils/HeaderGuard.h
@@ -11,16 +11,28 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARD_H
 
 #include "../ClangTidy.h"
+#include "../utils/HeaderFileExtensionsUtils.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 
 /// Finds and fixes header guards.
+/// The check supports these options:
+///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
+/// header files (The filename extension should not contain "." prefix).
+/// ",h,hh,hpp,hxx" by default.
+/// For extension-less header files, using an empty string or leaving an
+/// empty string between "," if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+RawStringHeaderFileExtensions(
+Options.getLocalOrGlobal("HeaderFileExtensions", ",h,hh,hpp,hxx")) {
+utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+ HeaderFileExtensions, ',');
+  }
   void registerPPCallbacks(CompilerInstance ) override;
 
   /// \brief Returns true if the checker should suggest inserting a trailing
@@ -39,6 +51,10 @@
   /// \brief Get the canonical header guard for a file.
   virtual std::string getHeaderGuard(StringRef Filename,
  StringRef OldGuard = StringRef()) = 0;
+
+private:
+  std::string RawStringHeaderFileExtensions;
+  utils::HeaderFileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace utils
Index: clang-tidy/utils/HeaderGuard.cpp
===
--- clang-tidy/utils/HeaderGuard.cpp
+++ clang-tidy/utils/HeaderGuard.cpp
@@ -20,7 +20,7 @@
 
 /// \brief canonicalize a path by removing ./ and ../ components.
 static std::string cleanPath(StringRef Path) {
-  SmallString<256> Result =  Path;
+  SmallString<256> Result = Path;
   llvm::sys::path::remove_dots(Result, true);
   return Result.str();
 }
@@ -274,13 +274,13 @@
 }
 
 bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
-  return FileName.endswith(".h");
+  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
 }
 
 bool HeaderGuardCheck::shouldFixHeaderGuard(StringRef FileName) { return true; }
 
 bool HeaderGuardCheck::shouldSuggestToAddHeaderGuard(StringRef FileName) {
-  return FileName.endswith(".h");
+  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
 }
 
 std::string HeaderGuardCheck::formatEndIf(StringRef HeaderGuard) {
Index: clang-tidy/utils/HeaderFileExtensionsUtils.h
===
--- clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -12,8 +12,8 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
-#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace clang {
 namespace tidy {
@@ -41,6 +41,10 @@
HeaderFileExtensionsSet ,
char delimiter);
 
+/// \brief Decides whether a file has a header file extension
+bool isHeaderFileExtension(StringRef FileName,
+   HeaderFileExtensionsSet HeaderFileExtensions);
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tidy/utils/HeaderFileExtensionsUtils.cpp
===
--- clang-tidy/utils/HeaderFileExtensionsUtils.cpp
+++ clang-tidy/utils/HeaderFileExtensionsUtils.cpp
@@ -61,6 +61,15 @@
   return true;
 }
 
+bool isHeaderFileExtension(StringRef FileName,
+   HeaderFileExtensionsSet HeaderFileExtensions) {
+  StringRef extension = ::llvm::sys::path::extension(FileName);
+  if (extension.startswith("."))
+extension = extension.substr(1);
+
+  return HeaderFileExtensions.count(extension) > 0;
+}
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tidy/llvm/HeaderGuardCheck.h
===
--- clang-tidy/llvm/HeaderGuardCheck.h
+++ clang-tidy/llvm/HeaderGuardCheck.h
@@ -17,13 +17,28 @@
 namespace llvm {
 
 /// Finds and fixes header guards that do not adhere to LLVM style.
+/// The check supports these options:

Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files

[Mads Ravn via cfe-commits]

madsravn updated this revision to Diff 69131.
madsravn marked 8 inline comments as done.
madsravn added a comment.

Updated the patch as suggested by hokein and alexfh.


https://reviews.llvm.org/D20512

Files:
  clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tidy/llvm/HeaderGuardCheck.h
  clang-tidy/utils/HeaderFileExtensionsUtils.cpp
  clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tidy/utils/HeaderGuard.cpp
  clang-tidy/utils/HeaderGuard.h

Index: clang-tidy/utils/HeaderGuard.h
===
--- clang-tidy/utils/HeaderGuard.h
+++ clang-tidy/utils/HeaderGuard.h
@@ -11,16 +11,29 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARD_H
 
 #include "../ClangTidy.h"
+#include "../utils/HeaderFileExtensionsUtils.h"
+#include "llvm/Support/Path.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 
 /// Finds and fixes header guards.
+/// The check supports these options:
+///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
+/// header files (The filename extension should not contain "." prefix).
+/// ",h,hh,hpp,hxx" by default.
+/// For extension-less header files, using an empty string or leaving an
+/// empty string between "," if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+RawStringHeaderFileExtensions(
+  Options.getLocalOrGlobal("HeaderFileExtensions", ",h,hh,hpp,hxx")) {
+utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+ HeaderFileExtensions, ',');
+  }
   void registerPPCallbacks(CompilerInstance ) override;
 
   /// \brief Returns true if the checker should suggest inserting a trailing
@@ -39,6 +52,9 @@
   /// \brief Get the canonical header guard for a file.
   virtual std::string getHeaderGuard(StringRef Filename,
  StringRef OldGuard = StringRef()) = 0;
+private:
+  utils::HeaderFileExtensionsSet HeaderFileExtensions;
+  std::string RawStringHeaderFileExtensions;
 };
 
 } // namespace utils
Index: clang-tidy/utils/HeaderGuard.cpp
===
--- clang-tidy/utils/HeaderGuard.cpp
+++ clang-tidy/utils/HeaderGuard.cpp
@@ -274,13 +274,13 @@
 }
 
 bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
-  return FileName.endswith(".h");
+  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
 }
 
 bool HeaderGuardCheck::shouldFixHeaderGuard(StringRef FileName) { return true; }
 
 bool HeaderGuardCheck::shouldSuggestToAddHeaderGuard(StringRef FileName) {
-  return FileName.endswith(".h");
+  return utils::isHeaderFileExtension(FileName, HeaderFileExtensions);
 }
 
 std::string HeaderGuardCheck::formatEndIf(StringRef HeaderGuard) {
Index: clang-tidy/utils/HeaderFileExtensionsUtils.h
===
--- clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -41,6 +41,10 @@
HeaderFileExtensionsSet ,
char delimiter);
 
+/// \brief Decides whether a file has a header file extension
+bool isHeaderFileExtension(StringRef FileName,
+   HeaderFileExtensionsSet HeaderFileExtensions);
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tidy/utils/HeaderFileExtensionsUtils.cpp
===
--- clang-tidy/utils/HeaderFileExtensionsUtils.cpp
+++ clang-tidy/utils/HeaderFileExtensionsUtils.cpp
@@ -61,6 +61,17 @@
   return true;
 }
 
+bool isHeaderFileExtension(StringRef FileName, HeaderFileExtensionsSet HeaderFileExtensions) {
+  StringRef extension = ::llvm::sys::path::extension(FileName);
+  if (extension.size() > 0 && extension.front() == '.') {
+extension = extension.substr(1);
+  }
+  if (HeaderFileExtensions.count(extension))
+return true;
+
+  return false;
+}
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tidy/llvm/HeaderGuardCheck.h
===
--- clang-tidy/llvm/HeaderGuardCheck.h
+++ clang-tidy/llvm/HeaderGuardCheck.h
@@ -17,13 +17,27 @@
 namespace llvm {
 
 /// Finds and fixes header guards that do not adhere to LLVM style.
+/// The check supports these options:
+///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
+/// header files (The filename extension should not contain "." prefix).
+/// ",h,hh,hpp,hxx" by default.
+/// For extension-less header files, using an empty string or leaving an
+/// empty string between "," if there are other filename extensions.
 class LLVMHeaderGuardCheck : public 

Re: [PATCH] D20510: [PATCH] Fix for bug 27802 misc-macro-parentheses breaks variadic macros

[Mads Ravn via cfe-commits]

madsravn closed this revision.
madsravn added a comment.

Code committed.


http://reviews.llvm.org/D20510



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

Re: [clang-tools-extra] r270575 - [clang-tidy] modernize-pass-by-value bugfix. Reverting lit-style test

[Mads Ravn via cfe-commits]

I'm glad. I'm finally getting the hang of this (I think) :)

Best regards,
Mads Ravn

On Tue, May 24, 2016 at 7:01 PM Renato Golin <renato.go...@linaro.org>
wrote:

> On 24 May 2016 at 17:09, Mads Ravn via cfe-commits
> <cfe-commits@lists.llvm.org> wrote:
> > Author: madsravn
> > Date: Tue May 24 11:09:24 2016
> > New Revision: 270575
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=270575=rev
> > Log:
> > [clang-tidy] modernize-pass-by-value bugfix. Reverting lit-style test
>
> This seems to have done the trick, thanks!
>
> --renato
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r270575 - [clang-tidy] modernize-pass-by-value bugfix. Reverting lit-style test

[Mads Ravn via cfe-commits]

Author: madsravn
Date: Tue May 24 11:09:24 2016
New Revision: 270575

URL: http://llvm.org/viewvc/llvm-project?rev=270575=rev
Log:
[clang-tidy] modernize-pass-by-value bugfix. Reverting lit-style test

Adding to revision 270567. The lit-style test was wrong. This is being fixed by 
this commit.

This is the bug on bugzilla: https://llvm.org/bugs/show_bug.cgi?id=27731

This is the code review on phabricator: http://reviews.llvm.org/D20365

Modified:
clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp

Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp?rev=270575=270574=270575=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp Tue May 
24 11:09:24 2016
@@ -150,8 +150,7 @@ template  struct N {
 // Test with value parameter.
 struct O {
   O(Movable M) : M(M) {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass by value and use std::move
-  // CHECK-FIXES: O(Movable M) : M(std::move(M)) {}
+  // CHECK-FIXES: O(Movable M) : M(M) {}
   Movable M;
 };
 
@@ -183,8 +182,7 @@ typedef ::Movable RMovable;
 }
 struct R {
   R(ns_R::RMovable M) : M(M) {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass by value and use std::move
-  // CHECK-FIXES: R(ns_R::RMovable M) : M(std::move(M)) {}
+  // CHECK-FIXES: R(ns_R::RMovable M) : M(M) {}
   ns_R::RMovable M;
 };
 
@@ -198,6 +196,7 @@ struct S {
 // Test that types that are trivially copyable will not use std::move. This 
will
 // cause problems with misc-move-const-arg, as it will revert it.
 struct T {
-  std::array a_;
   T(std::array a) : a_(a) {}
+  // CHECK-FIXES: T(std::array a) : a_(a) {} 
+  std::array a_;
 };


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

[clang-tools-extra] r270567 - [clang-tidy] modernize-pass-by-value bugfix. Reverting lit-style test

[Mads Ravn via cfe-commits]

Author: madsravn
Date: Tue May 24 10:13:44 2016
New Revision: 270567

URL: http://llvm.org/viewvc/llvm-project?rev=270567=rev
Log:
[clang-tidy] modernize-pass-by-value bugfix. Reverting lit-style test

Adding to revision 270565. The lit-style test was wrong. This is being fixed by 
this commit.

This is the bug on bugzilla: https://llvm.org/bugs/show_bug.cgi?id=27731

This is the code review on phabricator: http://reviews.llvm.org/D20365

Modified:
clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp

Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp?rev=270567=270566=270567=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp Tue May 
24 10:13:44 2016
@@ -1,9 +1,7 @@
-#include 
-#include 
-
 // RUN: %check_clang_tidy %s modernize-pass-by-value %t -- -- -std=c++11 
-fno-delayed-template-parsing
 
 // CHECK-FIXES: #include 
+#include 
 
 namespace {
 // POD types are trivially move constructible.
@@ -20,7 +18,7 @@ struct NotMovable {
 }
 
 struct A {
-  A(Movable M) : M(std::move(M)) {}
+  A(const Movable ) : M(M) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass by value and use std::move 
[modernize-pass-by-value]
   // CHECK-FIXES: A(Movable M) : M(std::move(M)) {}
   Movable M;
@@ -49,17 +47,17 @@ struct C {
 
 // Test that both declaration and definition are updated.
 struct D {
-  D(Movable M);
+  D(const Movable );
   // CHECK-FIXES: D(Movable M);
   Movable M;
 };
-D::D(Movable M) : M(std::move(M)) {}
+D::D(const Movable ) : M(M) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: pass by value and use std::move
 // CHECK-FIXES: D::D(Movable M) : M(std::move(M)) {}
 
 // Test with default parameter.
 struct E {
-  E(Movable M = Movable()) : M(std::move(M)) {}
+  E(const Movable  = Movable()) : M(M) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass by value and use std::move
   // CHECK-FIXES: E(Movable M = Movable()) : M(std::move(M)) {}
   Movable M;
@@ -74,11 +72,11 @@ struct F {
 
 // Test unnamed parameter in declaration.
 struct G {
-  G(Movable );
+  G(const Movable &);
   // CHECK-FIXES: G(Movable );
   Movable M;
 };
-G::G(Movable M) : M(std::move(M)) {}
+G::G(const Movable ) : M(M) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: pass by value and use std::move
 // CHECK-FIXES: G::G(Movable M) : M(std::move(M)) {}
 
@@ -87,12 +85,12 @@ namespace ns_H {
 typedef ::Movable HMovable;
 }
 struct H {
-  H(ns_H::HMovable M);
+  H(const ns_H::HMovable );
   // CHECK-FIXES: H(ns_H::HMovable M);
   ns_H::HMovable M;
 };
 using namespace ns_H;
-H::H(HMovable M) : M(std::move(M)) {}
+H::H(const HMovable ) : M(M) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: pass by value and use std::move
 // CHECK-FIXES: H(HMovable M) : M(std::move(M)) {}
 
@@ -125,14 +123,14 @@ struct K_Movable {
 
 // Test with movable type with an user defined move constructor.
 struct K {
-  K(K_Movable M) : M(std::move(M)) {}
+  K(const K_Movable ) : M(M) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass by value and use std::move
   // CHECK-FIXES: K(K_Movable M) : M(std::move(M)) {}
   K_Movable M;
 };
 
 template  struct L {
-  L(Movable M) : M(std::move(M)) {}
+  L(const Movable ) : M(M) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass by value and use std::move
   // CHECK-FIXES: L(Movable M) : M(std::move(M)) {}
   Movable M;
@@ -141,7 +139,7 @@ L l(Movable());
 
 // Test with a non-instantiated template class.
 template  struct N {
-  N(Movable M) : M(std::move(M)) {}
+  N(const Movable ) : M(M) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass by value and use std::move
   // CHECK-FIXES: N(Movable M) : M(std::move(M)) {}
 
@@ -151,7 +149,7 @@ template  struct N {
 
 // Test with value parameter.
 struct O {
-  O(Movable M) : M(std::move(M)) {}
+  O(Movable M) : M(M) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass by value and use std::move
   // CHECK-FIXES: O(Movable M) : M(std::move(M)) {}
   Movable M;
@@ -167,8 +165,8 @@ struct P {
 // Test with multiples parameters where some need to be changed and some don't.
 // need to.
 struct Q {
-  Q(const Movable , Movable B, Movable C, double D)
-  : A(A), B(std::move(B)), C(std::move(C)), D(D) {}
+  Q(const Movable , const Movable , const Movable , double D)
+  : A(A), B(B), C(C), D(D) {}
   // CHECK-MESSAGES: :[[@LINE-2]]:23: warning: pass by value and use std::move
   // CHECK-MESSAGES: :[[@LINE-3]]:41: warning: pass by value and use std::move
   // CHECK-FIXES:  Q(const Movable , Movable B, Movable C, double D)
@@ -184,7 +182,7 @@ namespace ns_R {
 typedef ::Movable RMovable;
 }
 struct R {
-  R(ns_R::RMovable M) : M(std::move(M)) {}
+  R(ns_R::RMovable M) : M(M) {}
   // CHECK-MESSAGES: 

[clang-tools-extra] r270565 - [clang-tidy] modernize-pass-by-value bugfix

[Mads Ravn via cfe-commits]

Author: madsravn
Date: Tue May 24 10:00:16 2016
New Revision: 270565

URL: http://llvm.org/viewvc/llvm-project?rev=270565=rev
Log:
[clang-tidy] modernize-pass-by-value bugfix

Modified the clang-tidy PassByValue check. It now stops adding std::move to 
type which is trivially copyable because that caused the clang-tidy 
MoveConstArg to complain and revert, thus creating a cycle.

I have also added a lit-style test to verify the bugfix.

This is the bug on bugzilla: https://llvm.org/bugs/show_bug.cgi?id=27731

This is the code review on phabricator: http://reviews.llvm.org/D20365

Modified:
clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp

Modified: clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp?rev=270565=270564=270565=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp Tue May 
24 10:00:16 2016
@@ -181,6 +181,11 @@ void PassByValueCheck::check(const Match
   if (!paramReferredExactlyOnce(Ctor, ParamDecl))
 return;
 
+  // If the parameter is trivial to copy, don't move it. Moving a trivivally
+  // copyable type will cause a problem with misc-move-const-arg
+  if (ParamDecl->getType().isTriviallyCopyableType(*Result.Context)) 
+return;
+
   auto Diag = diag(ParamDecl->getLocStart(), "pass by value and use 
std::move");
 
   // Iterate over all declarations of the constructor.

Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp?rev=270565=270564=270565=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp Tue May 
24 10:00:16 2016
@@ -1,3 +1,6 @@
+#include 
+#include 
+
 // RUN: %check_clang_tidy %s modernize-pass-by-value %t -- -- -std=c++11 
-fno-delayed-template-parsing
 
 // CHECK-FIXES: #include 
@@ -17,7 +20,7 @@ struct NotMovable {
 }
 
 struct A {
-  A(const Movable ) : M(M) {}
+  A(Movable M) : M(std::move(M)) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass by value and use std::move 
[modernize-pass-by-value]
   // CHECK-FIXES: A(Movable M) : M(std::move(M)) {}
   Movable M;
@@ -46,17 +49,17 @@ struct C {
 
 // Test that both declaration and definition are updated.
 struct D {
-  D(const Movable );
+  D(Movable M);
   // CHECK-FIXES: D(Movable M);
   Movable M;
 };
-D::D(const Movable ) : M(M) {}
+D::D(Movable M) : M(std::move(M)) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: pass by value and use std::move
 // CHECK-FIXES: D::D(Movable M) : M(std::move(M)) {}
 
 // Test with default parameter.
 struct E {
-  E(const Movable  = Movable()) : M(M) {}
+  E(Movable M = Movable()) : M(std::move(M)) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass by value and use std::move
   // CHECK-FIXES: E(Movable M = Movable()) : M(std::move(M)) {}
   Movable M;
@@ -71,11 +74,11 @@ struct F {
 
 // Test unnamed parameter in declaration.
 struct G {
-  G(const Movable &);
+  G(Movable );
   // CHECK-FIXES: G(Movable );
   Movable M;
 };
-G::G(const Movable ) : M(M) {}
+G::G(Movable M) : M(std::move(M)) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: pass by value and use std::move
 // CHECK-FIXES: G::G(Movable M) : M(std::move(M)) {}
 
@@ -84,12 +87,12 @@ namespace ns_H {
 typedef ::Movable HMovable;
 }
 struct H {
-  H(const ns_H::HMovable );
+  H(ns_H::HMovable M);
   // CHECK-FIXES: H(ns_H::HMovable M);
   ns_H::HMovable M;
 };
 using namespace ns_H;
-H::H(const HMovable ) : M(M) {}
+H::H(HMovable M) : M(std::move(M)) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: pass by value and use std::move
 // CHECK-FIXES: H(HMovable M) : M(std::move(M)) {}
 
@@ -122,14 +125,14 @@ struct K_Movable {
 
 // Test with movable type with an user defined move constructor.
 struct K {
-  K(const K_Movable ) : M(M) {}
+  K(K_Movable M) : M(std::move(M)) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass by value and use std::move
   // CHECK-FIXES: K(K_Movable M) : M(std::move(M)) {}
   K_Movable M;
 };
 
 template  struct L {
-  L(const Movable ) : M(M) {}
+  L(Movable M) : M(std::move(M)) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass by value and use std::move
   // CHECK-FIXES: L(Movable M) : M(std::move(M)) {}
   Movable M;
@@ -138,7 +141,7 @@ L l(Movable());
 
 // Test with a non-instantiated template class.
 template  struct N {
-  N(const Movable ) : M(M) {}
+  N(Movable M) : M(std::move(M)) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass by value and use std::move
  

Re: [PATCH] D20365: [PATCH] clang-tidy: Bug 27731 - modernize-pass-by-value suggest using std::move for types that perform copies on move

[Mads Ravn via cfe-commits]

madsravn added a comment.

@alexfh I don't know how I could miss that. But I got my commit access and 
committed the code myself. Thanks though.

@prazek Yes I reverted. The code made the build server (as seen on IRC) fail. 
So I quickly reverted. I'm gonna fix the code tonight - I had to "make clean" 
my entire llvm project yesterday, so that took a pretty long compiling time 
before I could start testing again.


http://reviews.llvm.org/D20365



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

Re: [clang-tools-extra] r270472 - Commiting for http://reviews.llvm.org/D20365

[Mads Ravn via cfe-commits]

@Nico, Yes, I will. I'm sorry about that. I had mistakenly read that it
would take the title and commit message from phabricator if I linked to
that in my svn commit message.

@Piotr, A test failed to build on the build server (as shown on IRC), so I
quickly reverted the commit. I will remember better commit messages (with
both titles and messages) from now on. Sorry.

Not the best start to this endeavour, but lesson learned.

Best regards,
Mads Ravn

On Mon, May 23, 2016 at 11:01 PM Piotr Padlewski <piotr.padlew...@gmail.com>
wrote:

> BTW why did you revert this change? And why the commit message doesn't
> have "revert" in name?
>
> 2016-05-23 20:51 GMT+02:00 Nico Weber via cfe-commits <
> cfe-commits@lists.llvm.org>:
>
>> Next time, please use real commit messages: Describe what the change
>> does, and why it's being done. Include a link to the review link at the end
>> of the commit message. If every change just had a phab link as commit
>> message, people bisecting changes would have to click through for every
>> change in `svn log` output.
>>
>> On Mon, May 23, 2016 at 2:15 PM, Mads Ravn via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: madsravn
>>> Date: Mon May 23 13:15:40 2016
>>> New Revision: 270472
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=270472=rev
>>> Log:
>>> Commiting for http://reviews.llvm.org/D20365
>>>
>>> Modified:
>>> clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp
>>> clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp
>>>
>>> Modified:
>>> clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp?rev=270472=270471=270472=diff
>>>
>>> ==
>>> --- clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp
>>> (original)
>>> +++ clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp
>>> Mon May 23 13:15:40 2016
>>> @@ -181,6 +181,12 @@ void PassByValueCheck::check(const Match
>>>if (!paramReferredExactlyOnce(Ctor, ParamDecl))
>>>  return;
>>>
>>> +
>>> +  // If the parameter is trivial to copy, don't move it. Moving a
>>> trivivally
>>> +  // copyable type will cause a problem with modernize-pass-by-value
>>> +  if (ParamDecl->getType().isTriviallyCopyableType(*Result.Context))
>>> +return;
>>> +
>>>auto Diag = diag(ParamDecl->getLocStart(), "pass by value and use
>>> std::move");
>>>
>>>// Iterate over all declarations of the constructor.
>>>
>>> Modified:
>>> clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp?rev=270472=270471=270472=diff
>>>
>>> ==
>>> --- clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp
>>> (original)
>>> +++ clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp
>>> Mon May 23 13:15:40 2016
>>> @@ -194,3 +194,9 @@ struct S {
>>>Movable M;
>>>  };
>>>
>>> +// Test that types that are trivially copyable will not use std::move.
>>> This will
>>> +// cause problems with misc-move-const-arg, as it will revert it.
>>> +struct T {
>>> +  std::array<int, 10> a_;
>>> +  T(std::array<int, 10> a) : a_(a) {}
>>> +};
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r270473 - Commiting for http://reviews.llvm.org/D20365

[Mads Ravn via cfe-commits]

Author: madsravn
Date: Mon May 23 13:27:05 2016
New Revision: 270473

URL: http://llvm.org/viewvc/llvm-project?rev=270473=rev
Log:
Commiting for http://reviews.llvm.org/D20365

Modified:
clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp

Modified: clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp?rev=270473=270472=270473=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp Mon May 
23 13:27:05 2016
@@ -181,12 +181,6 @@ void PassByValueCheck::check(const Match
   if (!paramReferredExactlyOnce(Ctor, ParamDecl))
 return;
 
-
-  // If the parameter is trivial to copy, don't move it. Moving a trivivally
-  // copyable type will cause a problem with modernize-pass-by-value
-  if (ParamDecl->getType().isTriviallyCopyableType(*Result.Context)) 
-return;
-
   auto Diag = diag(ParamDecl->getLocStart(), "pass by value and use 
std::move");
 
   // Iterate over all declarations of the constructor.

Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp?rev=270473=270472=270473=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp Mon May 
23 13:27:05 2016
@@ -193,10 +193,3 @@ struct S {
   // CHECK-FIXES: S(Movable &) : M(M) {}
   Movable M;
 };
-
-// Test that types that are trivially copyable will not use std::move. This 
will
-// cause problems with misc-move-const-arg, as it will revert it.
-struct T {
-  std::array a_;
-  T(std::array a) : a_(a) {}
-};


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

Re: [PATCH] D20365: [PATCH] clang-tidy: Bug 27731 - modernize-pass-by-value suggest using std::move for types that perform copies on move

[Mads Ravn via cfe-commits]

madsravn closed this revision.
madsravn added a comment.

Code committed.


http://reviews.llvm.org/D20365



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

[clang-tools-extra] r270472 - Commiting for http://reviews.llvm.org/D20365

[Mads Ravn via cfe-commits]

Author: madsravn
Date: Mon May 23 13:15:40 2016
New Revision: 270472

URL: http://llvm.org/viewvc/llvm-project?rev=270472=rev
Log:
Commiting for http://reviews.llvm.org/D20365

Modified:
clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp

Modified: clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp?rev=270472=270471=270472=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/PassByValueCheck.cpp Mon May 
23 13:15:40 2016
@@ -181,6 +181,12 @@ void PassByValueCheck::check(const Match
   if (!paramReferredExactlyOnce(Ctor, ParamDecl))
 return;
 
+
+  // If the parameter is trivial to copy, don't move it. Moving a trivivally
+  // copyable type will cause a problem with modernize-pass-by-value
+  if (ParamDecl->getType().isTriviallyCopyableType(*Result.Context)) 
+return;
+
   auto Diag = diag(ParamDecl->getLocStart(), "pass by value and use 
std::move");
 
   // Iterate over all declarations of the constructor.

Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp?rev=270472=270471=270472=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-pass-by-value.cpp Mon May 
23 13:15:40 2016
@@ -194,3 +194,9 @@ struct S {
   Movable M;
 };
 
+// Test that types that are trivially copyable will not use std::move. This 
will
+// cause problems with misc-move-const-arg, as it will revert it.
+struct T {
+  std::array a_;
+  T(std::array a) : a_(a) {}
+};


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

[clang-tools-extra] r270470 - Commiting for http://reviews.llvm.org/D20365

[Mads Ravn via cfe-commits]

Author: madsravn
Date: Mon May 23 13:06:29 2016
New Revision: 270470

URL: http://llvm.org/viewvc/llvm-project?rev=270470=rev
Log:
Commiting for http://reviews.llvm.org/D20365

Modified:
clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-macro-parentheses.cpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.cpp?rev=270470=270469=270470=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.cpp Mon May 
23 13:06:29 2016
@@ -188,6 +188,10 @@ void MacroParenthesesPPCallbacks::argume
 if (Prev.is(tok::kw_namespace))
   continue;
 
+// Variadic templates
+if (MI->isVariadic())
+  continue;
+
 Check->diag(Tok.getLocation(), "macro argument should be enclosed in "
"parentheses")
 << FixItHint::CreateInsertion(Tok.getLocation(), "(")

Modified: clang-tools-extra/trunk/test/clang-tidy/misc-macro-parentheses.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-macro-parentheses.cpp?rev=270470=270469=270470=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/misc-macro-parentheses.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-macro-parentheses.cpp Mon May 
23 13:06:29 2016
@@ -37,6 +37,8 @@
 #define GOOD26(x) (a->*x)
 #define GOOD27(x) (a.*x)
 #define GOOD28(x) namespace x {int b;}
+#define GOOD29(...)   std::cout << __VA_ARGS__;
+#define GOOD30(args...)   std::cout << args;
 
 // These are allowed for now..
 #define MAYBE1*12.34


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

Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files

[Mads Ravn via cfe-commits]

madsravn added inline comments.


Comment at: clang-tidy/llvm/HeaderGuardCheck.h:19
@@ -18,3 +18,3 @@
 
 /// Finds and fixes header guards that do not adhere to LLVM style.
 class LLVMHeaderGuardCheck : public utils::HeaderGuardCheck {

hokein wrote:
> You should add a document for the option `HeaderFileExtensions` here.
I'm not sure what you mean here. What is "a document" in this context?


http://reviews.llvm.org/D20512



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

[PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files

[Mads Ravn via cfe-commits]

madsravn created this revision.
madsravn added reviewers: alexfh, flx.
madsravn added a subscriber: cfe-commits.

Patch for bug 27475: https://llvm.org/bugs/show_bug.cgi?id=27475

I used the suggestion of alexfh to check the header extension.

http://reviews.llvm.org/D20512

Files:
  clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tidy/llvm/HeaderGuardCheck.h
  clang-tidy/utils/HeaderGuard.cpp
  clang-tidy/utils/HeaderGuard.h

Index: clang-tidy/utils/HeaderGuard.h
===
--- clang-tidy/utils/HeaderGuard.h
+++ clang-tidy/utils/HeaderGuard.h
@@ -11,6 +11,8 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADERGUARD_H
 
 #include "../ClangTidy.h"
+#include "../utils/HeaderFileExtensionsUtils.h"
+#include "llvm/Support/Path.h"
 
 namespace clang {
 namespace tidy {
@@ -39,6 +41,8 @@
   /// \brief Get the canonical header guard for a file.
   virtual std::string getHeaderGuard(StringRef Filename,
  StringRef OldGuard = StringRef()) = 0;
+private:
+  utils::HeaderFileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace utils
Index: clang-tidy/utils/HeaderGuard.cpp
===
--- clang-tidy/utils/HeaderGuard.cpp
+++ clang-tidy/utils/HeaderGuard.cpp
@@ -274,13 +274,37 @@
 }
 
 bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
-  return FileName.endswith(".h");
+  const std::string RawStringHeaderFileExtensions(
+Options.getLocalOrGlobal("HeaderFileExtensions", ",h,hh,hpp,hxx"));
+  utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+HeaderFileExtensions,
+',');
+  StringRef extension = ::llvm::sys::path::extension(FileName);
+  if (extension.size() > 0 && extension.front() == '.') {
+extension = extension.substr(1);
+  }
+  if (HeaderFileExtensions.count(extension))
+return true;
+
+  return false;
 }
 
 bool HeaderGuardCheck::shouldFixHeaderGuard(StringRef FileName) { return true; }
 
 bool HeaderGuardCheck::shouldSuggestToAddHeaderGuard(StringRef FileName) {
-  return FileName.endswith(".h");
+  const std::string RawStringHeaderFileExtensions(
+Options.getLocalOrGlobal("HeaderFileExtensions", ",h,hh,hpp,hxx"));
+  utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+HeaderFileExtensions,
+',');
+  StringRef extension = ::llvm::sys::path::extension(FileName);
+  if (extension.size() > 0 && extension.front() == '.') {
+extension = extension.substr(1);
+  }
+  if (HeaderFileExtensions.count(extension))
+return true;
+
+  return false;
 }
 
 std::string HeaderGuardCheck::formatEndIf(StringRef HeaderGuard) {
Index: clang-tidy/llvm/HeaderGuardCheck.h
===
--- clang-tidy/llvm/HeaderGuardCheck.h
+++ clang-tidy/llvm/HeaderGuardCheck.h
@@ -24,6 +24,8 @@
   bool shouldSuggestEndifComment(StringRef Filename) override { return false; }
   bool shouldFixHeaderGuard(StringRef Filename) override;
   std::string getHeaderGuard(StringRef Filename, StringRef OldGuard) override;
+private:
+  utils::HeaderFileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace llvm
Index: clang-tidy/llvm/HeaderGuardCheck.cpp
===
--- clang-tidy/llvm/HeaderGuardCheck.cpp
+++ clang-tidy/llvm/HeaderGuardCheck.cpp
@@ -14,7 +14,20 @@
 namespace llvm {
 
 bool LLVMHeaderGuardCheck::shouldFixHeaderGuard(StringRef Filename) {
-  return Filename.endswith(".h");
+  const std::string RawStringHeaderFileExtensions(
+  Options.getLocalOrGlobal("HeaderFileExtensions", ",h,hh,hpp,hxx"));
+  utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+  HeaderFileExtensions,
+  ',');
+  StringRef extension = ::llvm::sys::path::extension(Filename);
+  if (extension.size() > 0 && extension.front() == '.') {
+extension = extension.substr(1);
+  }
+
+  if (HeaderFileExtensions.count(extension))
+return true;
+
+  return false;
 }
 
 std::string LLVMHeaderGuardCheck::getHeaderGuard(StringRef Filename,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D20510: [PATCH] Fix for bug 27802 misc-macro-parentheses breaks variadic macros

[Mads Ravn via cfe-commits]

madsravn created this revision.
madsravn added reviewers: alexfh, flx.
madsravn added a subscriber: cfe-commits.

This is fix for bug 27802: https://llvm.org/bugs/show_bug.cgi?id=27802

I have added a check to clang-tidy which refrains from putting parantheses 
around macros which are variadic.

http://reviews.llvm.org/D20510

Files:
  clang-tidy/misc/MacroParenthesesCheck.cpp
  test/clang-tidy/misc-macro-parentheses.cpp

Index: test/clang-tidy/misc-macro-parentheses.cpp
===
--- test/clang-tidy/misc-macro-parentheses.cpp
+++ test/clang-tidy/misc-macro-parentheses.cpp
@@ -37,6 +37,8 @@
 #define GOOD26(x) (a->*x)
 #define GOOD27(x) (a.*x)
 #define GOOD28(x) namespace x {int b;}
+#define GOOD29(...)   std::cout << __VA_ARGS__;
+#define GOOD30(args...)   std::cout << args;
 
 // These are allowed for now..
 #define MAYBE1*12.34
Index: clang-tidy/misc/MacroParenthesesCheck.cpp
===
--- clang-tidy/misc/MacroParenthesesCheck.cpp
+++ clang-tidy/misc/MacroParenthesesCheck.cpp
@@ -188,6 +188,10 @@
 if (Prev.is(tok::kw_namespace))
   continue;
 
+// Variadic templates
+if (MI->isVariadic())
+  continue;
+
 Check->diag(Tok.getLocation(), "macro argument should be enclosed in "
"parentheses")
 << FixItHint::CreateInsertion(Tok.getLocation(), "(")


Index: test/clang-tidy/misc-macro-parentheses.cpp
===
--- test/clang-tidy/misc-macro-parentheses.cpp
+++ test/clang-tidy/misc-macro-parentheses.cpp
@@ -37,6 +37,8 @@
 #define GOOD26(x) (a->*x)
 #define GOOD27(x) (a.*x)
 #define GOOD28(x) namespace x {int b;}
+#define GOOD29(...)   std::cout << __VA_ARGS__;
+#define GOOD30(args...)   std::cout << args;
 
 // These are allowed for now..
 #define MAYBE1*12.34
Index: clang-tidy/misc/MacroParenthesesCheck.cpp
===
--- clang-tidy/misc/MacroParenthesesCheck.cpp
+++ clang-tidy/misc/MacroParenthesesCheck.cpp
@@ -188,6 +188,10 @@
 if (Prev.is(tok::kw_namespace))
   continue;
 
+// Variadic templates
+if (MI->isVariadic())
+  continue;
+
 Check->diag(Tok.getLocation(), "macro argument should be enclosed in "
"parentheses")
 << FixItHint::CreateInsertion(Tok.getLocation(), "(")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20365: [PATCH] clang-tidy: Bug 27731 - modernize-pass-by-value suggest using std::move for types that perform copies on move

[Mads Ravn via cfe-commits]

madsravn added a comment.

@Prazek thanks. I will look into it :)


http://reviews.llvm.org/D20365



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

Re: [PATCH] D20365: [PATCH] clang-tidy: Bug 27731 - modernize-pass-by-value suggest using std::move for types that perform copies on move

[Mads Ravn via cfe-commits]

madsravn added a comment.

Just curious, as I'm sort of new to this. How long will it take before its 
merged in?


http://reviews.llvm.org/D20365



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

[PATCH] D20365: [PATCH] clang-tidy: Bug 27731 - modernize-pass-by-value suggest using std::move for types that perform copies on move

[Mads Ravn via cfe-commits]

madsravn created this revision.
madsravn added reviewers: alexfh, vsk, djasper, klimek.
madsravn added a subscriber: cfe-commits.

This is a patch for bug: https://llvm.org/bugs/show_bug.cgi?id=27731

I have excluded types which are trivially copyable from being called with 
std::move in the modernizer. In the current state another modernizer will catch 
them and change them back, thus creating a cyclic behaviour.

http://reviews.llvm.org/D20365

Files:
  clang-tidy/modernize/PassByValueCheck.cpp
  test/clang-tidy/modernize-pass-by-value.cpp

Index: test/clang-tidy/modernize-pass-by-value.cpp
===
--- test/clang-tidy/modernize-pass-by-value.cpp
+++ test/clang-tidy/modernize-pass-by-value.cpp
@@ -194,3 +194,9 @@
   Movable M;
 };
 
+// Test that types that are trivially copyable will not use std::move. This 
will
+// cause problems with misc-move-const-arg, as it will revert it.
+struct T {
+  std::array a_;
+  T(std::array a) : a_(a) {}
+};
Index: clang-tidy/modernize/PassByValueCheck.cpp
===
--- clang-tidy/modernize/PassByValueCheck.cpp
+++ clang-tidy/modernize/PassByValueCheck.cpp
@@ -181,6 +181,11 @@
   if (!paramReferredExactlyOnce(Ctor, ParamDecl))
 return;
 
+  // If the parameter is trivial to copy, don't move it. Moving a trivivally
+  // copyable type will cause a problem with modernize-pass-by-value
+  if (ParamDecl->getType().isTriviallyCopyableType(*Result.Context)) 
+return;
+
   auto Diag = diag(ParamDecl->getLocStart(), "pass by value and use 
std::move");
 
   // Iterate over all declarations of the constructor.


Index: test/clang-tidy/modernize-pass-by-value.cpp
===
--- test/clang-tidy/modernize-pass-by-value.cpp
+++ test/clang-tidy/modernize-pass-by-value.cpp
@@ -194,3 +194,9 @@
   Movable M;
 };
 
+// Test that types that are trivially copyable will not use std::move. This will
+// cause problems with misc-move-const-arg, as it will revert it.
+struct T {
+  std::array a_;
+  T(std::array a) : a_(a) {}
+};
Index: clang-tidy/modernize/PassByValueCheck.cpp
===
--- clang-tidy/modernize/PassByValueCheck.cpp
+++ clang-tidy/modernize/PassByValueCheck.cpp
@@ -181,6 +181,11 @@
   if (!paramReferredExactlyOnce(Ctor, ParamDecl))
 return;
 
+  // If the parameter is trivial to copy, don't move it. Moving a trivivally
+  // copyable type will cause a problem with modernize-pass-by-value
+  if (ParamDecl->getType().isTriviallyCopyableType(*Result.Context)) 
+return;
+
   auto Diag = diag(ParamDecl->getLocStart(), "pass by value and use std::move");
 
   // Iterate over all declarations of the constructor.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: Patch submission for bug 27400

[Mads Ravn via cfe-commits]

Cool :) don't the sweat the time. I was just a little excited. Small patch
but it's nice to get started somewhere.

Best regards,
Mads Ravn

> On May 17, 2016, at 2:59 AM, Mads Ravn <madsr...@gmail.com> wrote:
>
> Hi guys,
>
> I just wanted to check up on this patch. I heard I could just reply to
this mail and see if I could 'ping' anyone in this regard. Hope it's OK.

Sorry for the delay! This looks good. Committed as r269786.

thanks,
vedant

>
> Best regards,
> Mads Ravn
>
> On Thu, May 12, 2016 at 6:11 PM Mads Ravn <madsr...@gmail.com> wrote:
> Hi,
>
> I have fixed the things you mentioned now. I have attached the new patch
to this email.
>
> Best regards,
> Mads Ravn
>
> On Wed, May 11, 2016 at 11:54 PM Vedant Kumar <v...@apple.com> wrote:
> Hi,
>
> Thanks for the patch!
>
> This patch is missing a small, lit-style test case. You can find examples
of test cases here:
>
>   extra/test/clang-tidy/
>
> Apart from that, my only other nit-pick is that llvm uses 2-space
indents, and spaces between "if" and "(".
>
> If you reply to this list with an updated patch, someone would be happy
to commit it for you.
>
> best
> vedant
>
> > On May 11, 2016, at 10:01 AM, Mads Ravn via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> >
> > Hi,
> >
> > I would like to submit a patch for
https://llvm.org/bugs/show_bug.cgi?id=27400 .
> >
> > Beside attaching the patch, is there anything I should be aware of? I
have not submitted a patch before.
> >
> > You can find the patch attached to this mail.
> >
> > Kind regards,
> > Mads Ravn
> >
___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: Patch submission for bug 27400

[Mads Ravn via cfe-commits]

Hi guys,

I just wanted to check up on this patch. I heard I could just reply to this
mail and see if I could 'ping' anyone in this regard. Hope it's OK.

Best regards,
Mads Ravn

On Thu, May 12, 2016 at 6:11 PM Mads Ravn <madsr...@gmail.com> wrote:

> Hi,
>
> I have fixed the things you mentioned now. I have attached the new patch
> to this email.
>
> Best regards,
> Mads Ravn
>
> On Wed, May 11, 2016 at 11:54 PM Vedant Kumar <v...@apple.com> wrote:
>
>> Hi,
>>
>> Thanks for the patch!
>>
>> This patch is missing a small, lit-style test case. You can find examples
>> of test cases here:
>>
>>   extra/test/clang-tidy/
>>
>> Apart from that, my only other nit-pick is that llvm uses 2-space
>> indents, and spaces between "if" and "(".
>>
>> If you reply to this list with an updated patch, someone would be happy
>> to commit it for you.
>>
>> best
>> vedant
>>
>> > On May 11, 2016, at 10:01 AM, Mads Ravn via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>> >
>> > Hi,
>> >
>> > I would like to submit a patch for
>> https://llvm.org/bugs/show_bug.cgi?id=27400 .
>> >
>> > Beside attaching the patch, is there anything I should be aware of? I
>> have not submitted a patch before.
>> >
>> > You can find the patch attached to this mail.
>> >
>> > Kind regards,
>> > Mads Ravn
>> >
>> ___
>> > cfe-commits mailing list
>> > cfe-commits@lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] clang-tidy: Bug 27731 - modernize-pass-by-value suggest using std::move for types that perform copies on move

[Mads Ravn via cfe-commits]

Hi,

I hereby attach a patch to fix bug no. 27731:
https://llvm.org/bugs/show_bug.cgi?id=27731

Best regards,
Mads Ravn
Index: clang-tidy/modernize/PassByValueCheck.cpp
===
--- clang-tidy/modernize/PassByValueCheck.cpp	(revision 269603)
+++ clang-tidy/modernize/PassByValueCheck.cpp	(working copy)
@@ -181,6 +181,11 @@
   if (!paramReferredExactlyOnce(Ctor, ParamDecl))
 return;
 
+  // If the parameter is trivial to copy, don't move it. Moving a trivivally
+  // copyable type will cause a problem with modernize-pass-by-value
+  if (ParamDecl->getType().isTriviallyCopyableType(*Result.Context)) 
+return;
+
   auto Diag = diag(ParamDecl->getLocStart(), "pass by value and use std::move");
 
   // Iterate over all declarations of the constructor.
Index: test/clang-tidy/modernize-pass-by-value.cpp
===
--- test/clang-tidy/modernize-pass-by-value.cpp	(revision 269603)
+++ test/clang-tidy/modernize-pass-by-value.cpp	(working copy)
@@ -194,3 +194,9 @@
   Movable M;
 };
 
+// Test that types that are trivially copyable will not use std::move. This will
+// cause problems with misc-move-const-arg, as it will revert it.
+struct T {
+  std::array a_;
+  T(std::array a) : a_(a) {}
+};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: Patch submission for bug 27400

[Mads Ravn via cfe-commits]

Hi,

I have fixed the things you mentioned now. I have attached the new patch to
this email.

Best regards,
Mads Ravn

On Wed, May 11, 2016 at 11:54 PM Vedant Kumar <v...@apple.com> wrote:

> Hi,
>
> Thanks for the patch!
>
> This patch is missing a small, lit-style test case. You can find examples
> of test cases here:
>
>   extra/test/clang-tidy/
>
> Apart from that, my only other nit-pick is that llvm uses 2-space indents,
> and spaces between "if" and "(".
>
> If you reply to this list with an updated patch, someone would be happy to
> commit it for you.
>
> best
> vedant
>
> > On May 11, 2016, at 10:01 AM, Mads Ravn via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >
> > Hi,
> >
> > I would like to submit a patch for
> https://llvm.org/bugs/show_bug.cgi?id=27400 .
> >
> > Beside attaching the patch, is there anything I should be aware of? I
> have not submitted a patch before.
> >
> > You can find the patch attached to this mail.
> >
> > Kind regards,
> > Mads Ravn
> >
> ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
Index: clang-tidy/misc/MacroParenthesesCheck.cpp
===
--- clang-tidy/misc/MacroParenthesesCheck.cpp	(revision 268956)
+++ clang-tidy/misc/MacroParenthesesCheck.cpp	(working copy)
@@ -184,6 +184,9 @@
 Next.isOneOf(tok::comma, tok::greater))
   continue;
 
+if (Prev.is(tok::kw_namespace))
+  continue;
+
 Check->diag(Tok.getLocation(), "macro argument should be enclosed in "
"parentheses")
 << FixItHint::CreateInsertion(Tok.getLocation(), "(")
Index: test/clang-tidy/misc-macro-parentheses.cpp
===
--- test/clang-tidy/misc-macro-parentheses.cpp	(revision 268956)
+++ test/clang-tidy/misc-macro-parentheses.cpp	(working copy)
@@ -36,6 +36,7 @@
 #define GOOD25(t) std::set<t,t,t> s
 #define GOOD26(x) (a->*x)
 #define GOOD27(x) (a.*x)
+#define GOOD28(x) namespace x {int b;}
 
 // These are allowed for now..
 #define MAYBE1*12.34
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits