[clang-tools-extra] 73da7ee - [clang-tidy] Add portability-std-allocator-const check
Fangrui Song via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 13 22:35:15 PDT 2022
Author: Fangrui Song
Date: 2022-04-13T22:35:11-07:00
New Revision: 73da7eed8fac84c9005518740f12d58389998d95
URL: https://github.com/llvm/llvm-project/commit/73da7eed8fac84c9005518740f12d58389998d95
DIFF: https://github.com/llvm/llvm-project/commit/73da7eed8fac84c9005518740f12d58389998d95.diff
LOG: [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
Added:
clang-tools-extra/clang-tidy/portability/StdAllocatorConstCheck.cpp
clang-tools-extra/clang-tidy/portability/StdAllocatorConstCheck.h
clang-tools-extra/docs/clang-tidy/checks/portability-std-allocator-const.rst
clang-tools-extra/test/clang-tidy/checkers/portability-std-allocator-const.cpp
Modified:
clang-tools-extra/clang-tidy/portability/CMakeLists.txt
clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/portability/CMakeLists.txt b/clang-tools-extra/clang-tidy/portability/CMakeLists.txt
index a0de5871e036a..579e459bc72bc 100644
--- a/clang-tools-extra/clang-tidy/portability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/portability/CMakeLists.txt
@@ -7,6 +7,7 @@ add_clang_library(clangTidyPortabilityModule
PortabilityTidyModule.cpp
RestrictSystemIncludesCheck.cpp
SIMDIntrinsicsCheck.cpp
+ StdAllocatorConstCheck.cpp
LINK_LIBS
clangTidy
diff --git a/clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp
index c87a119aa81fc..1e4f1d5de1d97 100644
--- a/clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp
@@ -11,6 +11,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "RestrictSystemIncludesCheck.h"
#include "SIMDIntrinsicsCheck.h"
+#include "StdAllocatorConstCheck.h"
namespace clang {
namespace tidy {
@@ -23,6 +24,8 @@ class PortabilityModule : public ClangTidyModule {
"portability-restrict-system-includes");
CheckFactories.registerCheck<SIMDIntrinsicsCheck>(
"portability-simd-intrinsics");
+ CheckFactories.registerCheck<StdAllocatorConstCheck>(
+ "portability-std-allocator-const");
}
};
diff --git a/clang-tools-extra/clang-tidy/portability/StdAllocatorConstCheck.cpp b/clang-tools-extra/clang-tidy/portability/StdAllocatorConstCheck.cpp
new file mode 100644
index 0000000000000..a95048d71ef92
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/portability/StdAllocatorConstCheck.cpp
@@ -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
diff --git a/clang-tools-extra/clang-tidy/portability/StdAllocatorConstCheck.h b/clang-tools-extra/clang-tidy/portability/StdAllocatorConstCheck.h
new file mode 100644
index 0000000000000..98a7b473e17e1
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/portability/StdAllocatorConstCheck.h
@@ -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
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 870a043c6b813..5392ab5758f59 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -117,6 +117,14 @@ New checks
Replaces groups of adjacent macros with an unscoped anonymous enum.
+- New :doc:`portability-std-allocator-const <clang-tidy/checks/portability-std-allocator-const>` check.
+
+ 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.
+ Future libc++ will remove the extension (`D120996
+ <https://reviews.llvm.org/D120996>`).
+
New check aliases
^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index e9367216ac515..6b3a6d83a361c 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -289,6 +289,7 @@ Clang-Tidy Checks
`performance-unnecessary-value-param <performance-unnecessary-value-param.html>`_, "Yes"
`portability-restrict-system-includes <portability-restrict-system-includes.html>`_, "Yes"
`portability-simd-intrinsics <portability-simd-intrinsics.html>`_,
+ `portability-std-allocator-const <portability-std-allocator-const.html>`_,
`readability-avoid-const-params-in-decls <readability-avoid-const-params-in-decls.html>`_, "Yes"
`readability-braces-around-statements <readability-braces-around-statements.html>`_, "Yes"
`readability-const-return-type <readability-const-return-type.html>`_, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/portability-std-allocator-const.rst b/clang-tools-extra/docs/clang-tidy/checks/portability-std-allocator-const.rst
new file mode 100644
index 0000000000000..31463c2b65dd3
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/portability-std-allocator-const.rst
@@ -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.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/portability-std-allocator-const.cpp b/clang-tools-extra/test/clang-tidy/checkers/portability-std-allocator-const.cpp
new file mode 100644
index 0000000000000..e6155592f33bc
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/portability-std-allocator-const.cpp
@@ -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;
+}
More information about the cfe-commits
mailing list