MaskRay created this revision.
MaskRay added reviewers: alexfh, aaron.ballman, njames93, whisperity.
Herald added subscribers: carlosgalvezp, StephenFan, rnkovacs, xazax.hun, 
mgorny.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Find undefined container usage due to std::allocator<const T>. This is
unsupported in libstdc++ and may be unsupported in future libc++ (D120996 
<https://reviews.llvm.org/D120996>). See
docs/clang-tidy/checks/portability-std-allocator-const-t.rst for detail.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123655

Files:
  clang-tools-extra/clang-tidy/portability/CMakeLists.txt
  clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/portability/StdAllocatorConstTCheck.cpp
  clang-tools-extra/clang-tidy/portability/StdAllocatorConstTCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/portability-std-allocator-const-t.rst
  
clang-tools-extra/test/clang-tidy/checkers/portability-std-allocator-const-t.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/portability-std-allocator-const-t.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/portability-std-allocator-const-t.cpp
@@ -0,0 +1,72 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s portability-std-allocator-const-t %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 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(std::vector<const char> a) {
+  // CHECK-MESSAGES: [[#@LINE-1]]:13: warning: container using std::allocator<const T> is undefined; remove const to work with libstdc++ and future libc++ [portability-std-allocator-const-t]
+  std::deque<const short> d;
+  // CHECK-MESSAGES: [[#@LINE-1]]:3: warning: container
+  std::list<const long> l;
+  // CHECK-MESSAGES: [[#@LINE-1]]:3: warning: container
+
+  std::multiset<int *const> ms;
+  // CHECK-MESSAGES: [[#@LINE-1]]:3: warning: container
+  std::set<const std::hash<int>> s;
+  // CHECK-MESSAGES: [[#@LINE-1]]:3: warning: container
+
+  absl::flat_hash_set<const int> fhs;
+  // CHECK-MESSAGES: [[#@LINE-1]]:3: warning: container
+
+  std::vector<int> neg0;
+  std::vector<const int *> neg1;                     // not const T
+  std::vector<const int, allocator<const int>> neg2; // not use std::allocator<const T>
+  std::forward_list<const int> forward;              // rare, don't bother
+  std::stack<const int> stack;                       // not caught, but rare
+}
+
+template <class T>
+void temp() {
+  std::vector<const T> v;
+  // CHECK-MESSAGES: [[#@LINE-1]]:3: warning: container
+
+  std::vector<T> neg;
+}
+void use_temp() {
+  temp<int>();
+}
Index: clang-tools-extra/docs/clang-tidy/checks/portability-std-allocator-const-t.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/portability-std-allocator-const-t.rst
@@ -0,0 +1,22 @@
+.. title:: clang-tidy - portability-std-allocator-const-t
+
+portability-std-allocator-const-t
+=================================
+
+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.
+
+libstdc++ never supports ``std::allocator<const T>`` and containers using them.
+Since GCC 8.0 (`PR48101 <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48101>`),
+there is a better diagnostic for some containers.
+
+.. code:: c++
+
+  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
+
+However, code bases not compiled with libstdc++ may accrue such undefined usage.
+This check finds such code and prevents backsliding while clean-up is ongoing.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -289,6 +289,7 @@
    `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-t <portability-std-allocator-const-t.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"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -117,6 +117,10 @@
 
   Replaces groups of adjacent macros with an unscoped anonymous enum.
 
+- New :doc:`portability-std-allocator-t <clang-tidy/checks/portability-std-allocator-const-t>` check.
+
+  Find undefined container usage due to ``std::allocator<const T>``.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
Index: clang-tools-extra/clang-tidy/portability/StdAllocatorConstTCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/portability/StdAllocatorConstTCheck.h
@@ -0,0 +1,36 @@
+//===--- 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_STDALLOCATORCONSTTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PORTABILITY_STDALLOCATORCONSTTCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace portability {
+
+/// Find undefined container usage due to std::allocator<const T>. This is
+/// unsupported in libstdc++ (all versions) and future libc++.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/portability-std-allocator-const-t.html
+class StdAllocatorConstTCheck : public ClangTidyCheck {
+public:
+  StdAllocatorConstTCheck(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_STDALLOCATORCONSTTCHECK_H
Index: clang-tools-extra/clang-tidy/portability/StdAllocatorConstTCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/portability/StdAllocatorConstTCheck.cpp
@@ -0,0 +1,55 @@
+//===-- StdAllocatorConstTCheck.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 "StdAllocatorConstTCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace portability {
+
+void StdAllocatorConstTCheck::registerMatchers(MatchFinder *Finder) {
+  // Match std::allocator<const T>.
+  internal::BindableMatcher<Type> AllocatorConstT =
+      recordType(hasDeclaration(classTemplateSpecializationDecl(
+          hasName("::std::allocator"),
+          hasTemplateArgument(0, refersToType(qualType(isConstQualified()))))));
+
+  // Match `std::vector<const T> var;` and other common containers like deque,
+  // list, and absl::flat_hash_set. Some containers like forward_list, queue,
+  // and stack are not caught. Don't bother with std::unordered_set<const T>
+  // since libc++ does not allow it.
+  Finder->addMatcher(
+      varDecl(
+          hasType(hasUnqualifiedDesugaredType(
+              recordType(hasDeclaration(classTemplateSpecializationDecl(
+                  hasAnyName("::std::vector", "::std::deque", "::std::list",
+                             "::std::multiset", "::std::set",
+                             "::absl::flat_hash_set"),
+                  anyOf(hasTemplateArgument(1, refersToType(AllocatorConstT)),
+                        hasTemplateArgument(2, refersToType(AllocatorConstT)),
+                        hasTemplateArgument(
+                            3, refersToType(AllocatorConstT)))))))))
+          .bind("var"),
+      this);
+}
+
+void StdAllocatorConstTCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var");
+  if (!Var)
+    return;
+  diag(Var->getBeginLoc(),
+       "container using std::allocator<const T> is "
+       "undefined; remove const to work with libstdc++ and future libc++");
+}
+
+} // namespace portability
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp
+++ clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp
@@ -11,6 +11,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "RestrictSystemIncludesCheck.h"
 #include "SIMDIntrinsicsCheck.h"
+#include "StdAllocatorConstTCheck.h"
 
 namespace clang {
 namespace tidy {
@@ -23,6 +24,8 @@
         "portability-restrict-system-includes");
     CheckFactories.registerCheck<SIMDIntrinsicsCheck>(
         "portability-simd-intrinsics");
+    CheckFactories.registerCheck<StdAllocatorConstTCheck>(
+        "portability-std-allocator-const-t");
   }
 };
 
Index: clang-tools-extra/clang-tidy/portability/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/portability/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/portability/CMakeLists.txt
@@ -7,6 +7,7 @@
   PortabilityTidyModule.cpp
   RestrictSystemIncludesCheck.cpp
   SIMDIntrinsicsCheck.cpp
+  StdAllocatorConstTCheck.cpp
 
   LINK_LIBS
   clangTidy
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to