[clang-tools-extra] r361735 - DeleteNullPointerCheck now deletes until the end brace of the condition.
Author: madsravn
Date: Sun May 26 10:00:38 2019
New Revision: 361735
URL: http://llvm.org/viewvc/llvm-project?rev=361735&view=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&r1=361734&r2=361735&view=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&r1=361734&r2=361735&view=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
Re: [PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy
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
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
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
[clang-tools-extra] r290747 - [clang-tidy] Add check 'misc-string-compare'.
Author: madsravn
Date: Fri Dec 30 04:09:46 2016
New Revision: 290747
URL: http://llvm.org/viewvc/llvm-project?rev=290747&view=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&r1=290746&r2=290747&view=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&r1=290746&r2=290747&view=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&view=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))
+
r290766 - [clang] Minor fix to libASTMatcherTutorial
Author: madsravn
Date: Fri Dec 30 14:49:44 2016
New Revision: 290766
URL: http://llvm.org/viewvc/llvm-project?rev=290766&view=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&r1=290765&r2=290766&view=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 &Result) {
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
Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files
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 &Compiler) 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 &HeaderFileExtensions,
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 LLVMHeade
Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files
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 &Compiler) 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 &HeaderFileExtensions,
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 chec
Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files
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 &Compiler) 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 &HeaderFileExtensions,
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.c
Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files
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 &Compiler) 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 &HeaderFileExtensions,
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/Hea
Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files
madsravn added a comment. I have commit access. I can commit it myself :) thanks though. Is the documentation correct? I wasn't sure about the "\n ..." part. Comment at: clang-tidy/utils/HeaderGuard.h:15 @@ -14,1 +14,3 @@ +#include "../utils/HeaderFileExtensionsUtils.h" +#include "llvm/Support/Path.h" hokein wrote: > seems not needed? Needed for Options. 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
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 &Compiler) 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 &HeaderFileExtensions,
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;
}
Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files
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
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 &Compiler) 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 &HeaderFileExtensions,
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-tid
Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files
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&id=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 &HeaderFileExtensions,
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 &Compiler) 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::isHeaderFileExtensi
[clang-tools-extra] r279803 - [clang-tidy] Added hh, hxx and hpp to header guard checks.
Author: madsravn
Date: Fri Aug 26 00:59:53 2016
New Revision: 279803
URL: http://llvm.org/viewvc/llvm-project?rev=279803&view=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&r1=279802&r2=279803&view=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&r1=279802&r2=279803&view=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&r1=279802&r2=279803&view=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&r1=279802&r2=279803&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/utils/HeaderFileExten
[clang-tools-extra] r294912 - [clang-tidy] Fix for bug 31838: readability-delete-null-pointer does not work for class members
Author: madsravn
Date: Sun Feb 12 14:09:59 2017
New Revision: 294912
URL: http://llvm.org/viewvc/llvm-project?rev=294912&view=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&r1=294911&r2=294912&view=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&r1=294911&r2=294912&view=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&r1=294911&r2=294912&view=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
[clang-tools-extra] r294913 - [clang-tidy] Fix for bug 31838: readability-delete-null-pointer does not work for class members
Author: madsravn
Date: Sun Feb 12 14:35:42 2017
New Revision: 294913
URL: http://llvm.org/viewvc/llvm-project?rev=294913&view=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&r1=294912&r2=294913&view=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
Re: [PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy
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
Re: [PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy
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
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
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
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
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
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
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
[clang-tools-extra] r301167 - [clang-tidy] New check: modernize-replace-random-shuffle.
Author: madsravn
Date: Mon Apr 24 04:27:20 2017
New Revision: 301167
URL: http://llvm.org/viewvc/llvm-project?rev=301167&view=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&r1=301166&r2=301167&view=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&r1=301166&r2=301167&view=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&view=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, argumentC
Patch submission for bug 27400
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
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(), "(")
___
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
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 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 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
[PATCH] clang-tidy: Bug 27731 - modernize-pass-by-value suggest using std::move for types that perform copies on move
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
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 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 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
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 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 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 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] D20365: [PATCH] clang-tidy: Bug 27731 - modernize-pass-by-value suggest using std::move for types that perform copies on move
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] D20365: [PATCH] clang-tidy: Bug 27731 - modernize-pass-by-value suggest using std::move for types that perform copies on move
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
Re: [PATCH] D20365: [PATCH] clang-tidy: Bug 27731 - modernize-pass-by-value suggest using std::move for types that perform copies on move
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
[PATCH] D20510: [PATCH] Fix for bug 27802 misc-macro-parentheses breaks variadic macros
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
[PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files
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
Re: [PATCH] D20512: [PATCH] Bug 27475 - Request header guard check processes .hpp files as well as .h files
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
[clang-tools-extra] r270470 - Commiting for http://reviews.llvm.org/D20365
Author: madsravn
Date: Mon May 23 13:06:29 2016
New Revision: 270470
URL: http://llvm.org/viewvc/llvm-project?rev=270470&view=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&r1=270469&r2=270470&view=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&r1=270469&r2=270470&view=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
[clang-tools-extra] r270472 - Commiting for http://reviews.llvm.org/D20365
Author: madsravn
Date: Mon May 23 13:15:40 2016
New Revision: 270472
URL: http://llvm.org/viewvc/llvm-project?rev=270472&view=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&r1=270471&r2=270472&view=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&r1=270471&r2=270472&view=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
Re: [PATCH] D20365: [PATCH] clang-tidy: Bug 27731 - modernize-pass-by-value suggest using std::move for types that perform copies on move
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] r270473 - Commiting for http://reviews.llvm.org/D20365
Author: madsravn
Date: Mon May 23 13:27:05 2016
New Revision: 270473
URL: http://llvm.org/viewvc/llvm-project?rev=270473&view=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&r1=270472&r2=270473&view=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&r1=270472&r2=270473&view=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(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: [clang-tools-extra] r270472 - Commiting for http://reviews.llvm.org/D20365
@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
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&view=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&r1=270471&r2=270472&view=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&r1=270471&r2=270472&view=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
>>>
>>
>>
>> ___
>> 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] D20365: [PATCH] clang-tidy: Bug 27731 - modernize-pass-by-value suggest using std::move for types that perform copies on move
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
[clang-tools-extra] r270565 - [clang-tidy] modernize-pass-by-value bugfix
Author: madsravn
Date: Tue May 24 10:00:16 2016
New Revision: 270565
URL: http://llvm.org/viewvc/llvm-project?rev=270565&view=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&r1=270564&r2=270565&view=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&r1=270564&r2=270565&view=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(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 &M);
+ D(Movable M);
// CHECK-FIXES: D(Movable M);
Movable M;
};
-D::D(const Movable &M) : 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 &M = 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(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 &M);
+ 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(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(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(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(M) {}
+ N(Movable M) : M(std::move(M)) {}
// CHECK-MESSAGES: :[[@LINE-1]]
[clang-tools-extra] r270567 - [clang-tidy] modernize-pass-by-value bugfix. Reverting lit-style test
Author: madsravn
Date: Tue May 24 10:13:44 2016
New Revision: 270567
URL: http://llvm.org/viewvc/llvm-project?rev=270567&view=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&r1=270566&r2=270567&view=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(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 &M);
// CHECK-FIXES: D(Movable M);
Movable M;
};
-D::D(Movable M) : M(std::move(M)) {}
+D::D(const Movable &M) : 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 &M = 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(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 &M);
// 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(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(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(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(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 &A, Movable B, Movable C, double D)
- : A(A), B(std::move(B)), C(std::move(C)), D(D) {}
+ Q(const Movable &A, const Movable &B, const Movable &C, 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 &A, 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::R
[clang-tools-extra] r270575 - [clang-tidy] modernize-pass-by-value bugfix. Reverting lit-style test
Author: madsravn
Date: Tue May 24 11:09:24 2016
New Revision: 270575
URL: http://llvm.org/viewvc/llvm-project?rev=270575&view=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&r1=270574&r2=270575&view=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
Re: [clang-tools-extra] r270575 - [clang-tidy] modernize-pass-by-value bugfix. Reverting lit-style test
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 wrote: > On 24 May 2016 at 17:09, Mads Ravn via cfe-commits > wrote: > > Author: madsravn > > Date: Tue May 24 11:09:24 2016 > > New Revision: 270575 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=270575&view=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
Re: [PATCH] D20510: [PATCH] Fix for bug 27802 misc-macro-parentheses breaks variadic macros
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
