-
Notifications
You must be signed in to change notification settings - Fork 11.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[clang-tidy] Add portability-std-allocator-const check
Report use of ``std::vector<const T>`` (and similar containers of const elements). These are now allowed in standard C++ due to undefined ``std::allocator<const T>``. They do not compile with libstdc++ or MSVC. Future libc++ will remove the extension (D120996). See docs/clang-tidy/checks/portability-std-allocator-const.rst for detail. I have attempted clean-up in a large code base. Here are some statistics: * 98% are related to the container `std::vector`, among `deque/forward_list/list/multiset/queue/set/stack/vector`. * 24% are related to `std::vector<const std::string>`. * Both `std::vector<const absl::string_view>` and `std::vector<const int>` contribute 2%. The other contributors spread over various class types. The check can be useful to other large code bases and may serve as an example for future libc++ strictness improvement. Reviewed By: sammccall Differential Revision: https://reviews.llvm.org/D123655
- Loading branch information
Showing
8 changed files
with
246 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
71 changes: 71 additions & 0 deletions
71
clang-tools-extra/clang-tidy/portability/StdAllocatorConstCheck.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
//===-- StdAllocatorConstCheck.cpp - clang-tidy --------------------------===// | ||
// | ||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
// See https://llvm.org/LICENSE.txt for license information. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#include "StdAllocatorConstCheck.h" | ||
#include "clang/ASTMatchers/ASTMatchFinder.h" | ||
|
||
using namespace clang::ast_matchers; | ||
|
||
namespace clang { | ||
namespace tidy { | ||
namespace portability { | ||
|
||
void StdAllocatorConstCheck::registerMatchers(MatchFinder *Finder) { | ||
// Match std::allocator<const T>. | ||
auto allocatorConst = | ||
recordType(hasDeclaration(classTemplateSpecializationDecl( | ||
hasName("::std::allocator"), | ||
hasTemplateArgument(0, refersToType(qualType(isConstQualified())))))); | ||
|
||
auto hasContainerName = | ||
hasAnyName("::std::vector", "::std::deque", "::std::list", | ||
"::std::multiset", "::std::set", "::std::unordered_multiset", | ||
"::std::unordered_set", "::absl::flat_hash_set"); | ||
|
||
// Match `std::vector<const T> var;` and other common containers like deque, | ||
// list, and absl::flat_hash_set. Containers like queue and stack use deque | ||
// but do not directly use std::allocator as a template argument, so they | ||
// aren't caught. | ||
Finder->addMatcher( | ||
typeLoc( | ||
templateSpecializationTypeLoc(), | ||
loc(hasUnqualifiedDesugaredType(anyOf( | ||
recordType(hasDeclaration(classTemplateSpecializationDecl( | ||
hasContainerName, | ||
anyOf( | ||
hasTemplateArgument(1, refersToType(allocatorConst)), | ||
hasTemplateArgument(2, refersToType(allocatorConst)), | ||
hasTemplateArgument(3, refersToType(allocatorConst)))))), | ||
// Match std::vector<const dependent> | ||
templateSpecializationType( | ||
templateArgumentCountIs(1), | ||
hasTemplateArgument( | ||
0, refersToType(qualType(isConstQualified()))), | ||
hasDeclaration(namedDecl(hasContainerName))))))) | ||
.bind("type_loc"), | ||
this); | ||
} | ||
|
||
void StdAllocatorConstCheck::check(const MatchFinder::MatchResult &Result) { | ||
const auto *T = Result.Nodes.getNodeAs<TypeLoc>("type_loc"); | ||
if (!T) | ||
return; | ||
// Exclude TypeLoc matches in STL headers. | ||
if (isSystem(Result.Context->getSourceManager().getFileCharacteristic( | ||
T->getBeginLoc()))) | ||
return; | ||
|
||
diag(T->getBeginLoc(), | ||
"container using std::allocator<const T> is a deprecated libc++ " | ||
"extension; remove const for compatibility with other standard " | ||
"libraries"); | ||
} | ||
|
||
} // namespace portability | ||
} // namespace tidy | ||
} // namespace clang |
37 changes: 37 additions & 0 deletions
37
clang-tools-extra/clang-tidy/portability/StdAllocatorConstCheck.h
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
//===--- StdAllocatorConstT.h - clang-tidy -----------------------*- C++-*-===// | ||
// | ||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
// See https://llvm.org/LICENSE.txt for license information. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PORTABILITY_STDALLOCATORCONSTCHECK_H | ||
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PORTABILITY_STDALLOCATORCONSTCHECK_H | ||
|
||
#include "../ClangTidyCheck.h" | ||
|
||
namespace clang { | ||
namespace tidy { | ||
namespace portability { | ||
|
||
/// Report use of ``std::vector<const T>`` (and similar containers of const | ||
/// elements). These are not allowed in standard C++ due to undefined | ||
/// ``std::allocator<const T>``. They do not compile with libstdc++ or MSVC. | ||
/// | ||
/// For the user-facing documentation see: | ||
/// http://clang.llvm.org/extra/clang-tidy/checks/portability-std-allocator-const.html | ||
class StdAllocatorConstCheck : public ClangTidyCheck { | ||
public: | ||
StdAllocatorConstCheck(StringRef Name, ClangTidyContext *Context) | ||
: ClangTidyCheck(Name, Context) {} | ||
|
||
void registerMatchers(ast_matchers::MatchFinder *Finder) override; | ||
void check(const ast_matchers::MatchFinder::MatchResult &Result) override; | ||
}; | ||
|
||
} // namespace portability | ||
} // namespace tidy | ||
} // namespace clang | ||
|
||
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PORTABILITY_STDALLOCATORCONSTCHECK_H |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
31 changes: 31 additions & 0 deletions
31
clang-tools-extra/docs/clang-tidy/checks/portability-std-allocator-const.rst
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
.. title:: clang-tidy - portability-std-allocator-const | ||
|
||
portability-std-allocator-const | ||
=============================== | ||
|
||
Report use of ``std::vector<const T>`` (and similar containers of const | ||
elements). These are not allowed in standard C++, and should usually be | ||
``std::vector<T>`` instead." | ||
|
||
Per C++ ``[allocator.requirements.general]``: "T is any cv-unqualified object | ||
type", ``std::allocator<const T>`` is undefined. Many standard containers use | ||
``std::allocator`` by default and therefore their ``const T`` instantiations are | ||
undefined. | ||
|
||
libc++ defines ``std::allocator<const T>`` as an extension which will be removed | ||
in the future. | ||
|
||
libstdc++ and MSVC do not support ``std::allocator<const T>``: | ||
|
||
.. code:: c++ | ||
|
||
// libstdc++ has a better diagnostic since https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48101 | ||
std::deque<const int> deque; // error: static assertion failed: std::deque must have a non-const, non-volatile value_type | ||
std::set<const int> set; // error: static assertion failed: std::set must have a non-const, non-volatile value_type | ||
std::vector<int* const> vector; // error: static assertion failed: std::vector must have a non-const, non-volatile value_type | ||
|
||
// MSVC | ||
// error C2338: static_assert failed: 'The C++ Standard forbids containers of const elements because allocator<const T> is ill-formed.' | ||
|
||
Code bases only compiled with libc++ may accrue such undefined usage. This | ||
check finds such code and prevents backsliding while clean-up is ongoing. |
94 changes: 94 additions & 0 deletions
94
clang-tools-extra/test/clang-tidy/checkers/portability-std-allocator-const.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
// RUN: %check_clang_tidy -std=c++11-or-later %s portability-std-allocator-const %t -- | ||
|
||
namespace std { | ||
typedef unsigned size_t; | ||
|
||
template <class T> | ||
class allocator {}; | ||
template <class T> | ||
class hash {}; | ||
template <class T> | ||
class equal_to {}; | ||
template <class T> | ||
class less {}; | ||
|
||
template <class T, class A = std::allocator<T>> | ||
class deque {}; | ||
template <class T, class A = std::allocator<T>> | ||
class forward_list {}; | ||
template <class T, class A = std::allocator<T>> | ||
class list {}; | ||
template <class T, class A = std::allocator<T>> | ||
class vector {}; | ||
|
||
template <class K, class C = std::less<K>, class A = std::allocator<K>> | ||
class multiset {}; | ||
template <class K, class C = std::less<K>, class A = std::allocator<K>> | ||
class set {}; | ||
template <class K, class H = std::hash<K>, class Eq = std::equal_to<K>, class A = std::allocator<K>> | ||
class unordered_multiset {}; | ||
template <class K, class H = std::hash<K>, class Eq = std::equal_to<K>, class A = std::allocator<K>> | ||
class unordered_set {}; | ||
|
||
template <class T, class C = std::deque<T>> | ||
class stack {}; | ||
} // namespace std | ||
|
||
namespace absl { | ||
template <class K, class H = std::hash<K>, class Eq = std::equal_to<K>, class A = std::allocator<K>> | ||
class flat_hash_set {}; | ||
} // namespace absl | ||
|
||
template <class T> | ||
class allocator {}; | ||
|
||
void simple(const std::vector<const char> &v, std::deque<const short> *d) { | ||
// CHECK-MESSAGES: [[#@LINE-1]]:24: warning: container using std::allocator<const T> is a deprecated libc++ extension; remove const for compatibility with other standard libraries | ||
// CHECK-MESSAGES: [[#@LINE-2]]:52: warning: container | ||
std::list<const long> l; | ||
// CHECK-MESSAGES: [[#@LINE-1]]:8: warning: container | ||
|
||
std::multiset<int *const> ms; | ||
// CHECK-MESSAGES: [[#@LINE-1]]:8: warning: container | ||
std::set<const std::hash<int>> s; | ||
// CHECK-MESSAGES: [[#@LINE-1]]:8: warning: container | ||
std::unordered_multiset<int *const> ums; | ||
// CHECK-MESSAGES: [[#@LINE-1]]:8: warning: container | ||
std::unordered_set<const int> us; | ||
// CHECK-MESSAGES: [[#@LINE-1]]:8: warning: container | ||
|
||
absl::flat_hash_set<const int> fhs; | ||
// CHECK-MESSAGES: [[#@LINE-1]]:9: warning: container | ||
|
||
using my_vector = std::vector<const int>; | ||
// CHECK-MESSAGES: [[#@LINE-1]]:26: warning: container | ||
my_vector v1; | ||
using my_vector2 = my_vector; | ||
|
||
std::vector<int> neg1; | ||
std::vector<const int *> neg2; // not const T | ||
std::vector<const int, allocator<const int>> neg3; // not use std::allocator<const T> | ||
std::allocator<const int> a; // not caught, but rare | ||
std::forward_list<const int> forward; // not caught, but rare | ||
std::stack<const int> stack; // not caught, but rare | ||
} | ||
|
||
template <class T> | ||
void temp1() { | ||
std::vector<const T> v; | ||
// CHECK-MESSAGES: [[#@LINE-1]]:8: warning: container | ||
|
||
std::vector<T> neg1; | ||
std::forward_list<const T> neg2; | ||
} | ||
void use_temp1() { temp1<int>(); } | ||
|
||
template <class T> | ||
void temp2() { | ||
// Match std::vector<const dependent> for the uninstantiated temp2. | ||
std::vector<const T> v; | ||
// CHECK-MESSAGES: [[#@LINE-1]]:8: warning: container | ||
|
||
std::vector<T> neg1; | ||
std::forward_list<const T> neg2; | ||
} |