carlosgalvezp updated this revision to Diff 444633.
carlosgalvezp added a comment.

- Rebase onto latest main.
- Remove references to std::reference_wrapper as alternative suggestions, as 
per: https://github.com/isocpp/CppCoreGuidelines/issues/1919


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126880/new/

https://reviews.llvm.org/D126880

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp
@@ -0,0 +1,157 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-const-or-ref-data-members %t
+namespace std {
+template <typename T>
+struct unique_ptr {};
+
+template <typename T>
+struct shared_ptr {};
+} // namespace std
+
+namespace gsl {
+template <typename T>
+struct not_null {};
+} // namespace gsl
+
+struct Ok {
+  int i;
+  int *p;
+  const int *pc;
+  std::unique_ptr<int> up;
+  std::shared_ptr<int> sp;
+  gsl::not_null<int> n;
+};
+
+struct ConstMember {
+  const int c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified [cppcoreguidelines-avoid-const-or-ref-data-members]
+};
+
+struct LvalueRefMember {
+  int &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' is a reference
+};
+
+struct ConstRefMember {
+  const int &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' is a reference
+};
+
+struct RvalueRefMember {
+  int &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' is a reference
+};
+
+struct ConstAndRefMembers {
+  int const c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified
+  int &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' is a reference
+  const int &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' is a reference
+  int &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' is a reference
+};
+
+struct Foo {};
+
+struct Ok2 {
+  Foo i;
+  Foo *p;
+  const Foo *pc;
+  std::unique_ptr<Foo> up;
+  std::shared_ptr<Foo> sp;
+  gsl::not_null<Foo> n;
+};
+
+struct ConstMember2 {
+  const Foo c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified
+};
+
+struct LvalueRefMember2 {
+  Foo &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' is a reference
+};
+
+struct ConstRefMember2 {
+  const Foo &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' is a reference
+};
+
+struct RvalueRefMember2 {
+  Foo &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' is a reference
+};
+
+struct ConstAndRefMembers2 {
+  const Foo c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified
+  Foo &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' is a reference
+  const Foo &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' is a reference
+  Foo &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' is a reference
+};
+
+using ConstType = const int;
+using RefType = int &;
+using ConstRefType = const int &;
+using RefRefType = int &&;
+
+struct WithAlias {
+  ConstType c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified
+  RefType lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: member 'lr' is a reference
+  ConstRefType cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: member 'cr' is a reference
+  RefRefType rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'rr' is a reference
+};
+
+template <int N>
+using Array = int[N];
+
+struct ConstArrayMember {
+  const Array<1> c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: member 'c' is const qualified
+};
+
+struct LvalueRefArrayMember {
+  Array<2> &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'lr' is a reference
+};
+
+struct ConstLvalueRefArrayMember {
+  Array<3> const &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: member 'cr' is a reference
+};
+
+struct RvalueRefArrayMember {
+  Array<4> &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'rr' is a reference
+};
+
+template <typename T>
+struct TemplatedOk {
+  T t;
+};
+
+template <typename T>
+struct TemplatedConst {
+  T t;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: member 't' is const qualified
+};
+
+template <typename T>
+struct TemplatedRef {
+  T t;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: member 't' is a reference
+};
+
+TemplatedOk<int> t1{};
+TemplatedConst<const int> t2{123};
+TemplatedRef<const int &> t3{123};
+TemplatedRef<int &&> t4{123};
+TemplatedRef<int &> t5{t1.t};
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
@@ -176,6 +176,7 @@
    `clang-analyzer-valist.Unterminated <clang-analyzer/valist.Unterminated.html>`_,
    `concurrency-mt-unsafe <concurrency/mt-unsafe.html>`_,
    `concurrency-thread-canceltype-asynchronous <concurrency/thread-canceltype-asynchronous.html>`_,
+   `cppcoreguidelines-avoid-const-or-ref-data-members <cppcoreguidelines/avoid-const-or-ref-data-members.html>`_,
    `cppcoreguidelines-avoid-goto <cppcoreguidelines/avoid-goto.html>`_,
    `cppcoreguidelines-avoid-non-const-global-variables <cppcoreguidelines/avoid-non-const-global-variables.html>`_,
    `cppcoreguidelines-init-variables <cppcoreguidelines/init-variables.html>`_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.rst
@@ -0,0 +1,49 @@
+.. title:: clang-tidy - cppcoreguidelines-avoid-const-or-ref-data-members
+
+cppcoreguidelines-avoid-const-or-ref-data-members
+=================================================
+
+This check warns when structs or classes have const-qualified or reference
+(lvalue or rvalue) data members. Having such members is rarely useful, and
+makes the class only copy-constructible but not copy-assignable.
+
+Examples:
+
+.. code-block:: c++
+
+  // Bad, const-qualified member
+  struct Const {
+    const int x;
+  }
+
+  // Good:
+  class Foo {
+   public:
+    int get() const { return x; }
+   private:
+    int x;
+  };
+
+  // Bad, lvalue reference member
+  struct Ref {
+    int& x;
+  };
+
+  // Good:
+  struct Foo {
+    int* x;
+    std::unique_ptr<int> x;
+    std::shared_ptr<int> x;
+    gsl::not_null<int> x;
+  };
+
+  // Bad, rvalue reference member
+  struct RefRef {
+    int&& x;
+  };
+
+The check implements
+`rule C.12 of C++ Core Guidelines <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c12-dont-make-data-members-const-or-references>`_.
+
+Further reading:
+`Data members: Never const <https://quuxplusone.github.io/blog/2022/01/23/dont-const-all-the-things/#data-members-never-const>`_.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -128,6 +128,11 @@
   Warns when the code is unwrapping a `std::optional<T>`, `absl::optional<T>`,
   or `base::Optional<T>` object without assuring that it contains a value.
 
+- New :doc:`cppcoreguidelines-avoid-const-or-ref-data-members
+  <clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members>` check.
+
+  Warns when a struct or class uses const or reference (lvalue or rvalue) data members.
+
 - New :doc:`misc-confusable-identifiers <clang-tidy/checks/misc/confusable-identifiers>` check.
 
   Detects confusable Unicode identifiers.
@@ -221,7 +226,7 @@
 
 - Fixed a false positive in :doc:`misc-unused-parameters
   <clang-tidy/checks/misc/unused-parameters>`
-  where invalid parameters were implicitly being treated as being unused. 
+  where invalid parameters were implicitly being treated as being unused.
   This fixes `Issue 56152 <https://github.com/llvm/llvm-project/issues/56152>`_.
 
 - Fixed a false positive in :doc:`modernize-deprecated-headers
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -14,6 +14,7 @@
 #include "../modernize/AvoidCArraysCheck.h"
 #include "../modernize/UseOverrideCheck.h"
 #include "../readability/MagicNumbersCheck.h"
+#include "AvoidConstOrRefDataMembersCheck.h"
 #include "AvoidGotoCheck.h"
 #include "AvoidNonConstGlobalVariablesCheck.h"
 #include "InitVariablesCheck.h"
@@ -47,6 +48,8 @@
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<modernize::AvoidCArraysCheck>(
         "cppcoreguidelines-avoid-c-arrays");
+    CheckFactories.registerCheck<AvoidConstOrRefDataMembersCheck>(
+        "cppcoreguidelines-avoid-const-or-ref-data-members");
     CheckFactories.registerCheck<AvoidGotoCheck>(
         "cppcoreguidelines-avoid-goto");
     CheckFactories.registerCheck<readability::MagicNumbersCheck>(
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -4,6 +4,7 @@
   )
 
 add_clang_library(clangTidyCppCoreGuidelinesModule
+  AvoidConstOrRefDataMembersCheck.cpp
   AvoidGotoCheck.cpp
   AvoidNonConstGlobalVariablesCheck.cpp
   CppCoreGuidelinesTidyModule.cpp
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h
@@ -0,0 +1,38 @@
+//===--- AvoidConstOrRefDataMembersCheck.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_CPPCOREGUIDELINES_AVOIDCONSTORREFDATAMEMBERSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDCONSTORREFDATAMEMBERSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Const-qualified or reference data members in classes should be avoided, as
+/// they make the class non-copy-assignable.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.html
+class AvoidConstOrRefDataMembersCheck : public ClangTidyCheck {
+public:
+  AvoidConstOrRefDataMembersCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDCONSTORREFDATAMEMBERSCHECK_H
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp
@@ -0,0 +1,37 @@
+//===--- AvoidConstOrRefDataMembersCheck.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 "AvoidConstOrRefDataMembersCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+void AvoidConstOrRefDataMembersCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      fieldDecl(hasType(hasCanonicalType(referenceType()))).bind("ref"), this);
+  Finder->addMatcher(
+      fieldDecl(hasType(qualType(isConstQualified()))).bind("const"), this);
+}
+
+void AvoidConstOrRefDataMembersCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  if (const auto *MatchedDecl = Result.Nodes.getNodeAs<FieldDecl>("ref"))
+    diag(MatchedDecl->getLocation(), "member %0 is a reference") << MatchedDecl;
+  if (const auto *MatchedDecl = Result.Nodes.getNodeAs<FieldDecl>("const"))
+    diag(MatchedDecl->getLocation(), "member %0 is const qualified")
+        << MatchedDecl;
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to