berenm updated this revision to Diff 35972.
berenm added a comment.
Address the latest comments, as well as useless code removal.
- Only the start of the token range is stored, so there is no need to care
about the end of the range. Remove the code that was trying to match the end of
the range (template, nested name).
- Add some spaces in test cases (destructors, templates) just to be sure that
everything works as expected.
- Add a missing CHECK-FIXES in the test case on the abstract_class destructor.
http://reviews.llvm.org/D13081
Files:
clang-query/tool/CMakeLists.txt
clang-tidy/misc/UnusedRAIICheck.h
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/MakeUniqueCheck.cpp
clang-tidy/modernize/MakeUniqueCheck.h
clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tidy/readability/IdentifierNamingCheck.h
clang-tidy/tool/run-clang-tidy.py
docs/clang-tidy/checks/google-build-explicit-make-pair.rst
docs/clang-tidy/checks/misc-unused-raii.rst
docs/modularize.rst
test/clang-tidy/Inputs/readability-identifier-naming/system/system-header.h
test/clang-tidy/Inputs/readability-identifier-naming/user-header.h
test/clang-tidy/modernize-make-unique.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===================================================================
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -62,23 +62,44 @@
// RUN: {key: readability-identifier-naming.VirtualMethodCase, value: UPPER_CASE}, \
// RUN: {key: readability-identifier-naming.VirtualMethodPrefix, value: 'v_'}, \
// RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \
-// RUN: ]}' -- -std=c++11 -fno-delayed-template-parsing
+// RUN: ]}' -- -std=c++11 -fno-delayed-template-parsing \
+// RUN: -I%S/Inputs/readability-identifier-naming \
+// RUN: -isystem %S/Inputs/readability-identifier-naming/system
-// FIXME: There should be more test cases for checking that references to class
-// FIXME: name, declaration contexts, forward declarations, etc, are correctly
-// FIXME: checked and renamed
+// clang-format off
+
+#include <system-header.h>
+#include "user-header.h"
+// NO warnings or fixes expected from declarations within header files without
+// the -header-filter= option
namespace FOO_NS {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for namespace 'FOO_NS' [readability-identifier-naming]
// CHECK-FIXES: {{^}}namespace foo_ns {{{$}}
inline namespace InlineNamespace {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for inline namespace 'InlineNamespace'
// CHECK-FIXES: {{^}}inline namespace inline_namespace {{{$}}
+SYSTEM_NS::structure g_s1;
+// NO warnings or fixes expected as SYSTEM_NS and structure are declared in a header file
+
+USER_NS::object g_s2;
+// NO warnings or fixes expected as USER_NS and object are declared in a header file
+
+SYSTEM_MACRO(var1);
+// NO warnings or fixes expected as var1 is from macro expansion
+
+USER_MACRO(var2);
+// NO warnings or fixes expected as var2 is declared in a macro expansion
+
+int global;
+#define USE_IN_MACRO(m) auto use_##m = m
+USE_IN_MACRO(global);
+// NO warnings or fixes expected as global is used in a macro expansion
+
#define BLA int FOO_bar
BLA;
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global variable 'FOO_bar'
-// NO fix expected as FOO_bar is from macro expansion
+// NO warnings or fixes expected as FOO_bar is from macro expansion
enum my_enumeration {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for enum 'my_enumeration'
@@ -102,6 +123,13 @@
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_class'
// CHECK-FIXES: {{^}}class CMyClass {{{$}}
my_class();
+// CHECK-FIXES: {{^}} CMyClass();{{$}}
+
+ ~
+ my_class();
+// (space in destructor token test, we could check trigraph but they will be deprecated)
+// CHECK-FIXES: {{^}} ~{{$}}
+// CHECK-FIXES: {{^}} CMyClass();{{$}}
const int MEMBER_one_1 = ConstExpr_variable;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: invalid case style for constant member 'MEMBER_one_1'
@@ -135,15 +163,36 @@
const int my_class::classConstant = 4;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class constant 'classConstant'
-// CHECK-FIXES: {{^}}const int my_class::kClassConstant = 4;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
+// CHECK-FIXES: {{^}}const int CMyClass::kClassConstant = 4;{{$}}
int my_class::ClassMember_2 = 5;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class member 'ClassMember_2'
-// CHECK-FIXES: {{^}}int my_class::ClassMember2 = 5;{{$}}
-// FIXME: The fixup should reflect class name fixups as well:
-// FIXME: {{^}}int CMyClass::ClassMember2 = 5;{{$}}
+// CHECK-FIXES: {{^}}int CMyClass::ClassMember2 = 5;{{$}}
+
+class my_derived_class : public virtual my_class {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_derived_class'
+// CHECK-FIXES: {{^}}class CMyDerivedClass : public virtual CMyClass {};{{$}}
+
+class CMyWellNamedClass {};
+// No warning expected as this class is well named.
+
+template<typename T>
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for type template parameter 'T'
+// CHECK-FIXES: {{^}}template<typename t_t>{{$}}
+class my_templated_class : CMyWellNamedClass {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_templated_class'
+// CHECK-FIXES: {{^}}class CMyTemplatedClass : CMyWellNamedClass {};{{$}}
+
+template<typename T>
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for type template parameter 'T'
+// CHECK-FIXES: {{^}}template<typename t_t>{{$}}
+class my_other_templated_class : my_templated_class< my_class>, private my_derived_class {};
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_other_templated_class'
+// CHECK-FIXES: {{^}}class CMyOtherTemplatedClass : CMyTemplatedClass< CMyClass>, private CMyDerivedClass {};{{$}}
+
+template<typename t_t>
+using MYSUPER_TPL = my_other_templated_class <:: FOO_NS ::my_class>;
+// CHECK-FIXES: {{^}}using MYSUPER_TPL = CMyOtherTemplatedClass <:: foo_ns ::CMyClass>;{{$}}
const int global_Constant = 6;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global constant 'global_Constant'
@@ -184,7 +233,7 @@
void Global_Fun(TYPE_parameters... PARAMETER_PACK) {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global function 'Global_Fun'
// CHECK-MESSAGES: :[[@LINE-2]]:17: warning: invalid case style for parameter pack 'PARAMETER_PACK'
-// CHECK-FIXES: {{^}}void GlobalFun(TYPE_parameters... parameterPack) {{{$}}
+// CHECK-FIXES: {{^}}void GlobalFun(typeParameters_t... parameterPack) {{{$}}
global_function(1, 2);
// CHECK-FIXES: {{^}} GlobalFunction(1, 2);{{$}}
FOO_bar = Global_variable;
@@ -206,12 +255,15 @@
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for abstract class 'abstract_class'
// CHECK-FIXES: {{^}}class AAbstractClass {{{$}}
virtual ~abstract_class() = 0;
+// CHECK-FIXES: {{^}} virtual ~AAbstractClass() = 0;{{$}}
virtual void VIRTUAL_METHOD();
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for virtual method 'VIRTUAL_METHOD'
// CHECK-FIXES: {{^}} virtual void v_VIRTUAL_METHOD();{{$}}
void non_Virtual_METHOD() {}
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for private method 'non_Virtual_METHOD'
// CHECK-FIXES: {{^}} void __non_Virtual_METHOD() {}{{$}}
+
+public:
static void CLASS_METHOD() {}
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for class method 'CLASS_METHOD'
// CHECK-FIXES: {{^}} static void classMethod() {}{{$}}
@@ -242,23 +294,28 @@
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for struct 'THIS___Structure'
// CHECK-FIXES: {{^}}struct this_structure {{{$}}
THIS___Structure();
-// FIXME: There should be a fixup for the constructor as well
-// FIXME: {{^}} this_structure();{{$}}
+// CHECK-FIXES: {{^}} this_structure();{{$}}
union __MyUnion_is_wonderful__ {};
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: invalid case style for union '__MyUnion_is_wonderful__'
// CHECK-FIXES: {{^}} union UMyUnionIsWonderful {};{{$}}
};
typedef THIS___Structure struct_type;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for typedef 'struct_type'
-// CHECK-FIXES: {{^}}typedef THIS___Structure struct_type_t;{{$}}
-// FIXME: The fixup should reflect structure name fixups as well:
-// FIXME: {{^}}typedef this_structure struct_type_t;{{$}}
+// CHECK-FIXES: {{^}}typedef this_structure struct_type_t;{{$}}
static void static_Function() {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for function 'static_Function'
// CHECK-FIXES: {{^}}static void staticFunction() {{{$}}
+
+ ::FOO_NS::InlineNamespace::abstract_class::CLASS_METHOD();
+// CHECK-FIXES: {{^}} ::foo_ns::inline_namespace::AAbstractClass::classMethod();{{$}}
+ ::FOO_NS::InlineNamespace::static_Function();
+// CHECK-FIXES: {{^}} ::foo_ns::inline_namespace::staticFunction();{{$}}
+
+ using ::FOO_NS::InlineNamespace::CE_function;
+// CHECK-FIXES: {{^}} using ::foo_ns::inline_namespace::ce_function;{{$}}
}
}
Index: test/clang-tidy/modernize-make-unique.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-make-unique.cpp
@@ -0,0 +1,122 @@
+// RUN: %python %S/check_clang_tidy.py %s modernize-make-unique %t
+
+namespace std {
+
+template <typename type>
+class unique_ptr {
+public:
+ unique_ptr(type *ptr);
+ unique_ptr(const unique_ptr<type> &t) = delete;
+ unique_ptr(unique_ptr<type> &&t);
+ ~unique_ptr();
+ type &operator*() { return *ptr; }
+ type *operator->() { return ptr; }
+ type *release();
+ void reset();
+ void reset(type *pt);
+
+private:
+ type *ptr;
+};
+
+}
+
+struct Base {
+ Base();
+ Base(int, int);
+};
+
+struct Derived : public Base {
+ Derived();
+ Derived(int, int);
+};
+
+struct Pair {
+ int a, b;
+};
+
+template<class T> using unique_ptr_ = std::unique_ptr<T>;
+
+int g(std::unique_ptr<int> P);
+
+std::unique_ptr<Base> getPointer() {
+ return std::unique_ptr<Base>(new Base);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_unique instead
+ // CHECK-FIXES: return std::make_unique<Base>();
+}
+
+void f() {
+ std::unique_ptr<int> P1 = std::unique_ptr<int>(new int());
+ // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
+ // CHECK-FIXES: std::unique_ptr<int> P1 = std::make_unique<int>();
+
+ // Without parenthesis.
+ std::unique_ptr<int> P2 = std::unique_ptr<int>(new int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
+ // CHECK-FIXES: std::unique_ptr<int> P2 = std::make_unique<int>();
+
+ // With auto.
+ auto P3 = std::unique_ptr<int>(new int());
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
+ // CHECK-FIXES: auto P3 = std::make_unique<int>();
+
+ {
+ // No std.
+ using namespace std;
+ unique_ptr<int> Q = unique_ptr<int>(new int());
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use std::make_unique instead
+ // CHECK-FIXES: unique_ptr<int> Q = std::make_unique<int>();
+ }
+
+ std::unique_ptr<int> R(new int());
+
+ // Create the unique_ptr as a parameter to a function.
+ int T = g(std::unique_ptr<int>(new int()));
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
+ // CHECK-FIXES: int T = g(std::make_unique<int>());
+
+ // Arguments are correctly handled.
+ std::unique_ptr<Base> Pbase = std::unique_ptr<Base>(new Base(5, T));
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: use std::make_unique instead
+ // CHECK-FIXES: std::unique_ptr<Base> Pbase = std::make_unique<Base>(5, T);
+
+ // Works with init lists.
+ std::unique_ptr<Pair> Ppair = std::unique_ptr<Pair>(new Pair{T, 1});
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: use std::make_unique instead
+ // CHECK-FIXES: std::unique_ptr<Pair> Ppair = std::make_unique<Pair>({T, 1});
+
+ // Only replace if the type in the template is the same than the type returned
+ // by the new operator.
+ auto Pderived = std::unique_ptr<Base>(new Derived());
+
+ // The pointer is returned by the function, nothing to do.
+ std::unique_ptr<Base> RetPtr = getPointer();
+
+ // Aliases.
+ typedef std::unique_ptr<int> IntPtr;
+ IntPtr Typedef = IntPtr(new int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use std::make_unique instead
+ // CHECK-FIXES: IntPtr Typedef = std::make_unique<int>();
+
+#define PTR unique_ptr<int>
+ std::unique_ptr<int> Macro = std::PTR(new int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead
+ // CHECK-FIXES: std::unique_ptr<int> Macro = std::make_unique<int>();
+#undef PTR
+
+ std::unique_ptr<int> Using = unique_ptr_<int>(new int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead
+ // CHECK-FIXES: std::unique_ptr<int> Using = std::make_unique<int>();
+
+ // This emulates std::move.
+ std::unique_ptr<int> Move = static_cast<std::unique_ptr<int>&&>(P1);
+
+ // Adding whitespaces.
+ auto Space = std::unique_ptr <int>(new int());
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use std::make_unique instead
+ // CHECK-FIXES: auto Space = std::make_unique<int>();
+
+ auto Spaces = std :: unique_ptr <int>(new int());
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use std::make_unique instead
+ // CHECK-FIXES: auto Spaces = std::make_unique<int>();
+}
Index: test/clang-tidy/Inputs/readability-identifier-naming/user-header.h
===================================================================
--- /dev/null
+++ test/clang-tidy/Inputs/readability-identifier-naming/user-header.h
@@ -0,0 +1,17 @@
+#ifndef USER_HEADER_H
+#define USER_HEADER_H
+
+#define USER_MACRO(m) int m = 0
+
+namespace USER_NS {
+class object {
+ int member;
+};
+
+float global;
+
+void function() {
+}
+}
+
+#endif // USER_HEADER_H
Index: test/clang-tidy/Inputs/readability-identifier-naming/system/system-header.h
===================================================================
--- /dev/null
+++ test/clang-tidy/Inputs/readability-identifier-naming/system/system-header.h
@@ -0,0 +1,17 @@
+#ifndef SYSTEM_HEADER_H
+#define SYSTEM_HEADER_H
+
+#define SYSTEM_MACRO(m) int m = 0
+
+namespace SYSTEM_NS {
+class structure {
+ int member;
+};
+
+float global;
+
+void function() {
+}
+}
+
+#endif // SYSTEM_HEADER_H
Index: docs/modularize.rst
===================================================================
--- docs/modularize.rst
+++ docs/modularize.rst
@@ -210,7 +210,7 @@
An optional ``-root-module=<root-name>`` option can be used to cause a root module
to be created which encloses all the modules.
-An optional ``-problem-files-list=<problem-file-name> can be used to input
+An optional ``-problem-files-list=<problem-file-name>`` can be used to input
a list of files to be excluded, perhaps as a temporary stop-gap measure until
problem headers can be fixed.
Index: docs/clang-tidy/checks/misc-unused-raii.rst
===================================================================
--- docs/clang-tidy/checks/misc-unused-raii.rst
+++ docs/clang-tidy/checks/misc-unused-raii.rst
@@ -19,10 +19,8 @@
We apply a number of heuristics to reduce the false positive count of this
check:
- * Ignore code expanded from macros. Testing frameworks make heavy use of
- this.
- * Ignore types with no user-declared constructor. Those are very unlikely
- to be RAII objects.
- * Ignore objects at the end of a compound statement (doesn't change
- behavior).
+ * Ignore code expanded from macros. Testing frameworks make heavy use of this.
+ * Ignore types with trivial destructors. They are very unlikely to be RAII
+ objects and there's no difference when they are deleted.
+ * Ignore objects at the end of a compound statement (doesn't change behavior).
* Ignore objects returned from a call.
Index: docs/clang-tidy/checks/google-build-explicit-make-pair.rst
===================================================================
--- docs/clang-tidy/checks/google-build-explicit-make-pair.rst
+++ docs/clang-tidy/checks/google-build-explicit-make-pair.rst
@@ -1,7 +1,6 @@
google-build-explicit-make-pair
===============================
-
Check that ``make_pair``'s template arguments are deduced.
G++ 4.6 in C++11 mode fails badly if ``make_pair``'s template arguments are
Index: clang-tidy/tool/run-clang-tidy.py
===================================================================
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -128,22 +128,29 @@
parser.add_argument('-fix', action='store_true', help='apply fix-its')
parser.add_argument('-format', action='store_true', help='Reformat code '
'after applying fixes')
+ parser.add_argument('-p', dest='build_path',
+ help='Path used to read a compile command database.')
args = parser.parse_args()
+ db_path = 'compile_commands.json'
+
+ if args.build_path is not None:
+ build_path = args.build_path
+ else:
+ # Find our database
+ build_path = find_compilation_database(db_path)
+
try:
invocation = [args.clang_tidy_binary, '-list-checks']
+ invocation.append('-p=' + build_path)
if args.checks:
invocation.append('-checks=' + args.checks)
invocation.append('-')
print subprocess.check_output(invocation)
except:
print >>sys.stderr, "Unable to run clang-tidy."
sys.exit(1)
- # Find our database.
- db_path = 'compile_commands.json'
- build_path = find_compilation_database(db_path)
-
# Load the database and extract all files.
database = json.load(open(os.path.join(build_path, db_path)))
files = [entry['file'] for entry in database]
Index: clang-tidy/readability/IdentifierNamingCheck.h
===================================================================
--- clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tidy/readability/IdentifierNamingCheck.h
@@ -62,20 +62,32 @@
}
};
-private:
- std::vector<NamingStyle> NamingStyles;
- bool IgnoreFailedSplit;
-
+ /// \brief Holds an identifier name check failure, tracking the kind of the
+ /// identifer, its possible fixup and the starting locations of all the
+ /// idenfiier usages.
struct NamingCheckFailure {
std::string KindName;
std::string Fixup;
+
+ /// \brief Whether the failure should be fixed or not.
+ ///
+ /// ie: if the identifier was used or declared within a macro we won't offer
+ /// a fixup for safety reasons.
bool ShouldFix;
- std::vector<SourceRange> Usages;
+
+ /// \brief A set of all the identifier usages starting SourceLocation, in
+ /// their encoded form.
+ llvm::DenseSet<unsigned> RawUsageLocs;
NamingCheckFailure() : ShouldFix(true) {}
};
+ typedef llvm::DenseMap<const NamedDecl *, NamingCheckFailure>
+ NamingCheckFailureMap;
- llvm::DenseMap<const NamedDecl *, NamingCheckFailure> NamingCheckFailures;
+private:
+ std::vector<NamingStyle> NamingStyles;
+ bool IgnoreFailedSplit;
+ NamingCheckFailureMap NamingCheckFailures;
};
} // namespace readability
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===================================================================
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -21,6 +21,7 @@
namespace tidy {
namespace readability {
+// clang-format off
#define NAMING_KEYS(m) \
m(Namespace) \
m(InlineNamespace) \
@@ -80,6 +81,7 @@
};
#undef NAMING_KEYS
+// clang-format on
IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
ClangTidyContext *Context)
@@ -134,13 +136,13 @@
}
void IdentifierNamingCheck::registerMatchers(MatchFinder *Finder) {
-// FIXME: For now, only Decl and DeclRefExpr nodes are visited for checking and
-// replacement. There is a lot of missing cases, such as references to a class
-// name (as in 'const int CMyClass::kClassConstant = 4;'), to an enclosing
-// context (namespace, class, etc).
-
Finder->addMatcher(namedDecl().bind("decl"), this);
- Finder->addMatcher(declRefExpr().bind("declref"), this);
+ Finder->addMatcher(usingDecl().bind("using"), this);
+ Finder->addMatcher(declRefExpr().bind("declRef"), this);
+ Finder->addMatcher(cxxConstructorDecl().bind("classRef"), this);
+ Finder->addMatcher(cxxDestructorDecl().bind("classRef"), this);
+ Finder->addMatcher(typeLoc().bind("typeLoc"), this);
+ Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"), this);
}
static bool matchesStyle(StringRef Name,
@@ -499,29 +501,124 @@
return SK_Invalid;
}
+static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
+ const NamedDecl *Decl, SourceRange Range,
+ const SourceManager *SM) {
+ // Do nothin if the provided range is invalid
+ if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid())
+ return;
+
+ // Try to insert the identifier location in the Usages map, and bail out if it
+ // is already in there
+ auto &Failure = Failures[Decl];
+ if (!Failure.RawUsageLocs.insert(Range.getBegin().getRawEncoding()).second)
+ return;
+
+ Failure.ShouldFix = Failure.ShouldFix && !Range.getBegin().isMacroID() &&
+ !Range.getEnd().isMacroID();
+}
+
void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) {
- if (const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>("declref")) {
- auto It = NamingCheckFailures.find(DeclRef->getDecl());
- if (It == NamingCheckFailures.end())
+ if (const auto *Decl =
+ Result.Nodes.getNodeAs<CXXConstructorDecl>("classRef")) {
+ if (Decl->isImplicit())
return;
- NamingCheckFailure &Failure = It->second;
- SourceRange Range = DeclRef->getNameInfo().getSourceRange();
+ addUsage(NamingCheckFailures, Decl->getParent(),
+ Decl->getNameInfo().getSourceRange(), Result.SourceManager);
+ return;
+ }
- Failure.Usages.push_back(Range);
- Failure.ShouldFix = Failure.ShouldFix &&
- Result.SourceManager->isInMainFile(Range.getBegin()) &&
- Result.SourceManager->isInMainFile(Range.getEnd()) &&
- !Range.getBegin().isMacroID() &&
- !Range.getEnd().isMacroID();
+ if (const auto *Decl =
+ Result.Nodes.getNodeAs<CXXDestructorDecl>("classRef")) {
+ if (Decl->isImplicit())
+ return;
+ SourceRange Range = Decl->getNameInfo().getSourceRange();
+ if (Range.getBegin().isInvalid())
+ return;
+ // The first token that will be found is the ~ (or the equivalent trigraph),
+ // we want instead to replace the next token, that will be the identifier.
+ Range.setBegin(CharSourceRange::getTokenRange(Range).getEnd());
+
+ addUsage(NamingCheckFailures, Decl->getParent(), Range,
+ Result.SourceManager);
+ return;
+ }
+
+ if (const auto *Loc = Result.Nodes.getNodeAs<TypeLoc>("typeLoc")) {
+ NamedDecl *Decl = nullptr;
+ if (const auto &Ref = Loc->getAs<TagTypeLoc>()) {
+ Decl = Ref.getDecl();
+ } else if (const auto &Ref = Loc->getAs<InjectedClassNameTypeLoc>()) {
+ Decl = Ref.getDecl();
+ } else if (const auto &Ref = Loc->getAs<UnresolvedUsingTypeLoc>()) {
+ Decl = Ref.getDecl();
+ } else if (const auto &Ref = Loc->getAs<TemplateTypeParmTypeLoc>()) {
+ Decl = Ref.getDecl();
+ }
+
+ if (Decl) {
+ addUsage(NamingCheckFailures, Decl, Loc->getSourceRange(),
+ Result.SourceManager);
+ return;
+ }
+
+ if (const auto &Ref = Loc->getAs<TemplateSpecializationTypeLoc>()) {
+ const auto *Decl =
+ Ref.getTypePtr()->getTemplateName().getAsTemplateDecl();
+
+ SourceRange Range(Ref.getTemplateNameLoc(), Ref.getTemplateNameLoc());
+ if (const auto *ClassDecl = dyn_cast<TemplateDecl>(Decl)) {
+ addUsage(NamingCheckFailures, ClassDecl->getTemplatedDecl(), Range,
+ Result.SourceManager);
+ return;
+ }
+ }
+
+ if (const auto &Ref =
+ Loc->getAs<DependentTemplateSpecializationTypeLoc>()) {
+ addUsage(NamingCheckFailures, Ref.getTypePtr()->getAsTagDecl(),
+ Loc->getSourceRange(), Result.SourceManager);
+ return;
+ }
+ }
+
+ if (const auto *Loc =
+ Result.Nodes.getNodeAs<NestedNameSpecifierLoc>("nestedNameLoc")) {
+ if (NestedNameSpecifier *Spec = Loc->getNestedNameSpecifier()) {
+ if (NamespaceDecl *Decl = Spec->getAsNamespace()) {
+ addUsage(NamingCheckFailures, Decl, Loc->getLocalSourceRange(),
+ Result.SourceManager);
+ return;
+ }
+ }
+ }
+
+ if (const auto *Decl = Result.Nodes.getNodeAs<UsingDecl>("using")) {
+ for (const auto &Shadow : Decl->shadows()) {
+ addUsage(NamingCheckFailures, Shadow->getTargetDecl(),
+ Decl->getNameInfo().getSourceRange(), Result.SourceManager);
+ }
+ return;
+ }
+
+ if (const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>("declRef")) {
+ SourceRange Range = DeclRef->getNameInfo().getSourceRange();
+ addUsage(NamingCheckFailures, DeclRef->getDecl(), Range,
+ Result.SourceManager);
return;
}
if (const auto *Decl = Result.Nodes.getNodeAs<NamedDecl>("decl")) {
if (!Decl->getIdentifier() || Decl->getName().empty() || Decl->isImplicit())
return;
+ // Ignore ClassTemplateSpecializationDecl which are creating duplicate
+ // replacements with CXXRecordDecl
+ if (isa<ClassTemplateSpecializationDecl>(Decl))
+ return;
+
StyleKind SK = findStyleKind(Decl, NamingStyles);
if (SK == SK_Invalid)
return;
@@ -550,11 +647,7 @@
Failure.Fixup = std::move(Fixup);
Failure.KindName = std::move(KindName);
- Failure.ShouldFix =
- Failure.ShouldFix &&
- Result.SourceManager->isInMainFile(Range.getBegin()) &&
- Result.SourceManager->isInMainFile(Range.getEnd()) &&
- !Range.getBegin().isMacroID() && !Range.getEnd().isMacroID();
+ addUsage(NamingCheckFailures, Decl, Range, Result.SourceManager);
}
}
}
@@ -564,19 +657,27 @@
const NamedDecl &Decl = *Pair.first;
const NamingCheckFailure &Failure = Pair.second;
- SourceRange DeclRange =
- DeclarationNameInfo(Decl.getDeclName(), Decl.getLocation())
- .getSourceRange();
- auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
- << Failure.KindName << Decl.getName();
+ if (Failure.KindName.empty())
+ continue;
if (Failure.ShouldFix) {
- Diag << FixItHint::CreateReplacement(
- CharSourceRange::getTokenRange(DeclRange), Failure.Fixup);
-
- for (const auto &Range : Failure.Usages) {
+ auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
+ << Failure.KindName << Decl.getName();
+
+ for (const auto &Loc : Failure.RawUsageLocs) {
+ // We assume that the identifier name is made of one token only. This is
+ // always the case as we ignore usages in macros that could build
+ // identifier names by combining multiple tokens.
+ //
+ // For destructors, we alread take care of it by remembering the
+ // location of the start of the identifier and not the start of the
+ // tilde.
+ //
+ // Other multi-token identifiers, such as operators are not checked at
+ // all.
Diag << FixItHint::CreateReplacement(
- CharSourceRange::getTokenRange(Range), Failure.Fixup);
+ SourceRange(SourceLocation::getFromRawEncoding(Loc)),
+ Failure.Fixup);
}
}
}
Index: clang-tidy/modernize/ModernizeTidyModule.cpp
===================================================================
--- clang-tidy/modernize/ModernizeTidyModule.cpp
+++ clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -11,6 +11,7 @@
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
#include "LoopConvertCheck.h"
+#include "MakeUniqueCheck.h"
#include "PassByValueCheck.h"
#include "ReplaceAutoPtrCheck.h"
#include "ShrinkToFitCheck.h"
@@ -28,6 +29,8 @@
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<LoopConvertCheck>("modernize-loop-convert");
+ CheckFactories.registerCheck<MakeUniqueCheck>(
+ "modernize-make-unique");
CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value");
CheckFactories.registerCheck<ReplaceAutoPtrCheck>(
"modernize-replace-auto-ptr");
Index: clang-tidy/modernize/MakeUniqueCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/modernize/MakeUniqueCheck.h
@@ -0,0 +1,41 @@
+//===--- MakeUniqueCheck.h - clang-tidy--------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MAKE_UNIQUE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MAKE_UNIQUE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replace the pattern:
+/// \code
+/// std::unique_ptr<type>(new type(args...))
+/// \endcode
+///
+/// With the C++14 version:
+/// \code
+/// std::make_unique<type>(args...)
+/// \endcode
+class MakeUniqueCheck : public ClangTidyCheck {
+public:
+ MakeUniqueCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MAKE_UNIQUE_H
+
Index: clang-tidy/modernize/MakeUniqueCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/modernize/MakeUniqueCheck.cpp
@@ -0,0 +1,108 @@
+//===--- MakeUniqueCheck.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 "MakeUniqueCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+const char PointerType[] = "pointerType";
+const char ConstructorCall[] = "constructorCall";
+const char NewExpression[] = "newExpression";
+
+void MakeUniqueCheck::registerMatchers(MatchFinder *Finder) {
+ if (getLangOpts().CPlusPlus11) {
+ Finder->addMatcher(
+ cxxBindTemporaryExpr(has(
+ cxxConstructExpr(
+ hasType(qualType(hasDeclaration(classTemplateSpecializationDecl(
+ matchesName("::std::unique_ptr"),
+ templateArgumentCountIs(1),
+ hasTemplateArgument(
+ 0, templateArgument(
+ refersToType(qualType().bind(PointerType)))))))),
+ argumentCountIs(1),
+ hasArgument(0, cxxNewExpr(hasType(pointsTo(qualType(
+ equalsBoundNode(PointerType)))))
+ .bind(NewExpression)))
+ .bind(ConstructorCall))),
+ this);
+ }
+}
+
+void MakeUniqueCheck::check(const MatchFinder::MatchResult &Result) {
+ SourceManager &SM = *Result.SourceManager;
+ const auto *Construct =
+ Result.Nodes.getNodeAs<CXXConstructExpr>(ConstructorCall);
+ const auto *New = Result.Nodes.getNodeAs<CXXNewExpr>(NewExpression);
+ const auto *Type = Result.Nodes.getNodeAs<QualType>(PointerType);
+
+ SourceLocation ConstructCallStart = Construct->getExprLoc();
+
+ bool Invalid = false;
+ StringRef ExprStr = Lexer::getSourceText(
+ CharSourceRange::getCharRange(
+ ConstructCallStart, Construct->getParenOrBraceRange().getBegin()),
+ SM, LangOptions(), &Invalid);
+ if (Invalid)
+ return;
+
+ auto Diag = diag(ConstructCallStart, "use std::make_unique instead");
+
+ // Find the location of the template's left angle.
+ size_t LAngle = ExprStr.find("<");
+ SourceLocation ConstructCallEnd;
+ if (LAngle == StringRef::npos) {
+ // If the template argument is missing (because it is part of the alias)
+ // we have to add it back.
+ ConstructCallEnd = ConstructCallStart.getLocWithOffset(ExprStr.size());
+ Diag << FixItHint::CreateInsertion(ConstructCallEnd,
+ "<" + Type->getAsString() + ">");
+ } else {
+ ConstructCallEnd = ConstructCallStart.getLocWithOffset(LAngle);
+ }
+
+ Diag << FixItHint::CreateReplacement(
+ CharSourceRange::getCharRange(ConstructCallStart, ConstructCallEnd),
+ "std::make_unique");
+
+ SourceLocation NewStart = New->getSourceRange().getBegin();
+ SourceLocation NewEnd = New->getSourceRange().getEnd();
+ switch (New->getInitializationStyle()) {
+ case CXXNewExpr::NoInit: {
+ Diag << FixItHint::CreateRemoval(SourceRange(NewStart, NewEnd));
+ break;
+ }
+ case CXXNewExpr::CallInit: {
+ SourceRange InitRange = New->getDirectInitRange();
+ Diag << FixItHint::CreateRemoval(
+ SourceRange(NewStart, InitRange.getBegin()));
+ Diag << FixItHint::CreateRemoval(SourceRange(InitRange.getEnd(), NewEnd));
+ break;
+ }
+ case CXXNewExpr::ListInit: {
+ SourceRange InitRange = New->getInitializer()->getSourceRange();
+ Diag << FixItHint::CreateRemoval(
+ SourceRange(NewStart, InitRange.getBegin().getLocWithOffset(-1)));
+ Diag << FixItHint::CreateRemoval(
+ SourceRange(InitRange.getEnd().getLocWithOffset(1), NewEnd));
+ break;
+ }
+ }
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/modernize/CMakeLists.txt
===================================================================
--- clang-tidy/modernize/CMakeLists.txt
+++ clang-tidy/modernize/CMakeLists.txt
@@ -3,6 +3,7 @@
add_clang_library(clangTidyModernizeModule
LoopConvertCheck.cpp
LoopConvertUtils.cpp
+ MakeUniqueCheck.cpp
ModernizeTidyModule.cpp
PassByValueCheck.cpp
ReplaceAutoPtrCheck.cpp
Index: clang-tidy/misc/UnusedRAIICheck.h
===================================================================
--- clang-tidy/misc/UnusedRAIICheck.h
+++ clang-tidy/misc/UnusedRAIICheck.h
@@ -18,28 +18,8 @@
/// Finds temporaries that look like RAII objects.
///
-/// The canonical example for this is a scoped lock.
-///
-/// \code
-/// {
-/// scoped_lock(&global_mutex);
-/// critical_section();
-/// }
-/// \endcode
-///
-/// The destructor of the scoped_lock is called before the `critical_section` is
-/// entered, leaving it unprotected.
-///
-/// We apply a number of heuristics to reduce the false positive count of this
-/// check:
-///
-/// * Ignore code expanded from macros. Testing frameworks make heavy use of
-/// this.
-/// * Ignore types with no user-declared constructor. Those are very unlikely
-/// to be RAII objects.
-/// * Ignore objects at the end of a compound statement (doesn't change
-/// behavior).
-/// * Ignore objects returned from a call.
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-unused-raii.html
class UnusedRAIICheck : public ClangTidyCheck {
public:
UnusedRAIICheck(StringRef Name, ClangTidyContext *Context)
Index: clang-query/tool/CMakeLists.txt
===================================================================
--- clang-query/tool/CMakeLists.txt
+++ clang-query/tool/CMakeLists.txt
@@ -10,3 +10,5 @@
clangQuery
clangTooling
)
+
+install(TARGETS clang-query RUNTIME DESTINATION bin)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
