JonasToth updated this revision to Diff 142521.
JonasToth added a comment.

- [Feature] implement array and method access
- [Feature] finalize value semantic


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/ConstCheck.cpp
  clang-tidy/cppcoreguidelines/ConstCheck.h
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-const.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-const-primtive-builtin.cpp

Index: test/clang-tidy/cppcoreguidelines-const-primtive-builtin.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-const-primtive-builtin.cpp
@@ -0,0 +1,387 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-const %t
+
+// ------- Provide test samples for primitive builtins ---------
+// - every 'p_*' variable is a 'potential_const_*' variable
+// - every 'np_*' variable is a 'non_potential_const_*' variable
+
+bool global;
+char np_global = 0; // globals can't be known to be const
+
+namespace foo {
+int scoped;
+float np_scoped = 1; // namespace variables are like globals
+} // namespace foo
+
+void some_function(double, wchar_t);
+
+void some_function(double np_arg0, wchar_t np_arg1) {
+  int p_local0 = 2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const
+
+  int np_local0;
+  const int np_local1 = 42;
+
+  unsigned int np_local2 = 3;
+  np_local2 <<= 4;
+
+  int np_local3 = 4;
+  ++np_local3;
+  int np_local4 = 4;
+  np_local4++;
+
+  int np_local5 = 4;
+  --np_local5;
+  int np_local6 = 4;
+  np_local6--;
+}
+
+void some_lambda_environment_capture_all_by_reference(double np_arg0) {
+  int np_local0 = 0;
+  int p_local0 = 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const
+
+  int np_local2;
+  const int np_local3 = 2;
+
+  // Capturing all variables by reference prohibits making them const.
+  [&]() { ++np_local0; };
+
+  int p_local1 = 0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const
+}
+
+void some_lambda_environment_capture_all_by_value(double np_arg0) {
+  int np_local0 = 0;
+  int p_local0 = 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const
+
+  int np_local1;
+  const int np_local2 = 2;
+
+  // Capturing by value has no influence on them.
+  [=]() { (void)p_local0; };
+
+  np_local0 += 10;
+}
+
+void function_inout_pointer(int *inout);
+void function_in_pointer(const int *in);
+
+void some_pointer_taking(int *out) {
+  int np_local0 = 42;
+  const int *const p0_np_local0 = &np_local0;
+  int *const p1_np_local0 = &np_local0;
+
+  int np_local1 = 42;
+  function_inout_pointer(&np_local1);
+
+  // Prevents const.
+  int np_local2 = 42;
+  out = &np_local2; // This returns and invalid address, its just about the AST
+
+  int p_local0 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const
+  const int *const p0_p_local0 = &p_local0;
+
+  int p_local1 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const
+  function_in_pointer(&p_local1);
+}
+
+void function_inout_ref(int &inout);
+void function_in_ref(const int &in);
+
+void some_reference_taking() {
+  int np_local0 = 42;
+  const int &r0_np_local0 = np_local0;
+  int &r1_np_local0 = np_local0;
+  const int &r2_np_local0 = r1_np_local0;
+
+  int np_local1 = 42;
+  function_inout_ref(np_local1);
+
+  int p_local0 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const
+  const int &r0_p_local0 = p_local0;
+
+  int p_local1 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const
+  function_in_ref(p_local1);
+}
+
+double *non_const_pointer_return() {
+  double p_local0 = 0.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared const
+  double np_local0 = 24.4;
+
+  return &np_local0;
+}
+
+const double *const_pointer_return() {
+  double p_local0 = 0.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared const
+  double p_local1 = 24.4;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'double' can be declared const
+  return &p_local1;
+}
+
+double &non_const_ref_return() {
+  double p_local0 = 0.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared const
+  double np_local0 = 42.42;
+  return np_local0;
+}
+
+const double &const_ref_return() {
+  double p_local0 = 0.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared const
+  double p_local1 = 24.4;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'double' can be declared const
+  return p_local1;
+}
+
+double *&return_non_const_pointer_ref() {
+  double *np_local0 = nullptr;
+  return np_local0;
+}
+
+void overloaded_arguments(const int &in);
+void overloaded_arguments(int &inout);
+void overloaded_arguments(const int *in);
+void overloaded_arguments(int *inout);
+
+void function_calling() {
+  int np_local0 = 42;
+  overloaded_arguments(np_local0);
+
+  const int np_local1 = 42;
+  overloaded_arguments(np_local1);
+
+  int np_local2 = 42;
+  overloaded_arguments(&np_local2);
+
+  const int np_local3 = 42;
+  overloaded_arguments(&np_local3);
+}
+
+template <typename T>
+void define_locals(T np_arg0, T &np_arg1, int np_arg2) {
+  T np_local0 = 0;
+  np_local0 += np_arg0 * np_arg1;
+
+  T np_local1 = 42;
+  np_local0 += np_local1;
+
+  // Used as argument to an overloaded function with const and non-const.
+  T np_local2 = 42;
+  overloaded_arguments(np_local2);
+
+  int np_local4 = 42;
+  // non-template values are ok still.
+  int p_local0 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const
+  np_local4 += p_local0;
+}
+
+void template_instantiation() {
+  const int np_local0 = 42;
+  int np_local1 = 42;
+
+  define_locals(np_local0, np_local1, np_local0);
+  define_locals(np_local1, np_local1, np_local1);
+}
+
+struct ConstNonConstClass {
+  ConstNonConstClass();
+  ConstNonConstClass(double &np_local0);
+  double nonConstMethod() {}
+  double constMethod() const {}
+  double modifyingMethod(double &np_arg0) const;
+};
+
+void direct_method_calls() {
+  ConstNonConstClass np_local0;
+
+  np_local0.constMethod();
+  np_local0.nonConstMethod();
+
+  ConstNonConstClass p_local0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'ConstNonConstClass' can be declared const
+  p_local0.constMethod();
+
+  ConstNonConstClass p_local1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'ConstNonConstClass' can be declared const
+  double np_local1;
+  p_local1.modifyingMethod(np_local1);
+
+  double np_local2;
+  ConstNonConstClass p_local2(np_local2);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'ConstNonConstClass' can be declared const
+}
+
+struct OperatorsAsConstAsPossible {
+  OperatorsAsConstAsPossible &operator+=(const OperatorsAsConstAsPossible &rhs);
+  OperatorsAsConstAsPossible operator+(const OperatorsAsConstAsPossible &rhs) const;
+};
+
+struct NonConstOperators {
+};
+NonConstOperators operator+(NonConstOperators &lhs, NonConstOperators &rhs);
+NonConstOperators operator-(NonConstOperators lhs, NonConstOperators rhs);
+
+void internal_operator_calls() {
+  OperatorsAsConstAsPossible np_local0;
+  OperatorsAsConstAsPossible np_local1;
+  OperatorsAsConstAsPossible p_local0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'OperatorsAsConstAsPossible' can be declared const
+  OperatorsAsConstAsPossible p_local1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'OperatorsAsConstAsPossible' can be declared const
+
+  np_local0 += p_local0;
+  np_local1 = p_local0 + p_local1;
+
+  NonConstOperators np_local2;
+  NonConstOperators np_local3;
+  NonConstOperators np_local4;
+
+  np_local2 = np_local3 + np_local4;
+
+  NonConstOperators p_local2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'NonConstOperators' can be declared const
+  NonConstOperators p_local3 = p_local2 - p_local2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local3' of type 'NonConstOperators' can be declared const
+}
+
+struct MyVector {
+  double *begin();
+  const double *begin() const;
+
+  double *end();
+  const double *end() const;
+
+  double &operator[](int index);
+  double operator[](int index) const;
+
+  double values[100];
+};
+
+void vector_usage() {
+  double np_local0[10];
+  np_local0[5] = 42.;
+
+  MyVector np_local1;
+  np_local1[5] = 42.;
+
+  double p_local0[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double [10]' can be declared const
+  double p_local1 = p_local0[5];
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'double' can be declared const
+
+  // The following subscript calls suprisingly choose the non-const operator
+  // version.
+  MyVector np_local2;
+  double p_local2 = np_local2[42];
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'double' can be declared const
+
+  MyVector np_local3;
+  const double np_local4 = np_local3[42];
+
+  // This subscript results in const overloaded operator.
+  const MyVector np_local5{};
+  double p_local3 = np_local5[42];
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local3' of type 'double' can be declared const
+}
+
+void const_handle(const double &np_local0);
+void const_handle(const double *np_local0);
+
+void non_const_handle(double &np_local0);
+void non_const_handle(double *np_local0);
+
+void handle_from_array() {
+  // Non-const handle from non-const array forbids declaring the array as const
+  double np_local0[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.};
+  double *p_local0 = &np_local0[1];
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double *' can be declared const
+
+  double np_local1[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.};
+  double &non_const_ref = np_local1[1];
+
+  double np_local2[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.};
+  double *np_local3;
+  np_local3 = &np_local2[5];
+
+  double np_local4[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.};
+  non_const_handle(np_local4[2]);
+  double np_local5[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.};
+  non_const_handle(&np_local5[2]);
+
+  // Constant handles are ok
+  double p_local1[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'double [10]' can be declared const
+  const double *p_local2 = &p_local1[2];
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'const double *' can be declared const
+
+  double p_local3[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local3' of type 'double [10]' can be declared const
+  const double &const_ref = p_local3[2];
+
+  double p_local4[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local4' of type 'double [10]' can be declared const
+  const double *const_ptr;
+  const_ptr = &p_local4[2];
+
+  double p_local5[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local5' of type 'double [10]' can be declared const
+  const_handle(p_local5[2]);
+  double p_local6[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local6' of type 'double [10]' can be declared const
+  const_handle(&p_local6[2]);
+}
+
+void range_for() {
+  int np_local0[2] = {1, 2};
+  for (int &non_const_ref : np_local0) {
+  }
+
+  int np_local1[2] = {1, 2};
+  for (auto &non_const_ref : np_local1) {
+  }
+
+  int np_local2[2] = {1, 2};
+  for (auto &&non_const_ref : np_local2) {
+  }
+
+  int *np_local3[2] = {&np_local0[0], &np_local0[1]};
+  for (int *non_const_ptr : np_local3) {
+    // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'non_const_ptr' of type 'int *' can be declared const
+  }
+
+  int *np_local4[2] = {&np_local0[0], &np_local0[1]};
+  for (auto *non_const_ptr : np_local4) {
+    // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'non_const_ptr' of type 'int *' can be declared const
+  }
+
+  int p_local0[2] = {1, 2};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int [2]' can be declared const
+  for (int value : p_local0) {
+    // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'value' of type 'int' can be declared const
+  }
+
+  int p_local1[2] = {1, 2};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int [2]' can be declared const
+  for (const int &const_ref : p_local1) {
+  }
+
+  int *p_local2[2] = {&np_local0[0], &np_local0[1]};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'int *[2]' can be declared const
+  for (const int *con_ptr : p_local2) {
+    // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'con_ptr' of type 'const int *' can be declared const
+  }
+
+  int *p_local3[2] = {nullptr, nullptr};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local3' of type 'int *[2]' can be declared const
+  for (const auto *con_ptr : p_local3) {
+    // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'con_ptr' of type 'const int *' can be declared const
+  }
+}
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -76,6 +76,7 @@
    cert-oop11-cpp (redirects to performance-move-constructor-init) <cert-oop11-cpp>
    cppcoreguidelines-avoid-goto
    cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) <cppcoreguidelines-c-copy-assignment-signature>
+   cppcoreguidelines-const
    cppcoreguidelines-interfaces-global-init
    cppcoreguidelines-no-malloc
    cppcoreguidelines-owning-memory
Index: docs/clang-tidy/checks/cppcoreguidelines-const.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-const.rst
@@ -0,0 +1,6 @@
+.. title:: clang-tidy - cppcoreguidelines-const
+
+cppcoreguidelines-const
+=======================
+
+FIXME: Describe what patterns does the check detect and why. Give examples.
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -98,6 +98,11 @@
   The usage of ``goto`` for control flow is error prone and should be replaced
   with looping constructs. Every backward jump is rejected. Forward jumps are
   only allowed in nested loops.
+  
+- New :doc:`cppcoreguidelines-const
+  <clang-tidy/checks/cppcoreguidelines-const>` check
+
+  FIXME: add release notes.
 
 - New alias :doc:`fuchsia-header-anon-namespaces
   <clang-tidy/checks/fuchsia-header-anon-namespaces>` to :doc:`google-build-namespaces
Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===================================================================
--- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -12,6 +12,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "AvoidGotoCheck.h"
+#include "ConstCheck.h"
 #include "InterfacesGlobalInitCheck.h"
 #include "NoMallocCheck.h"
 #include "OwningMemoryCheck.h"
@@ -38,6 +39,8 @@
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<AvoidGotoCheck>(
         "cppcoreguidelines-avoid-goto");
+    CheckFactories.registerCheck<ConstCheck>(
+        "cppcoreguidelines-const");
     CheckFactories.registerCheck<InterfacesGlobalInitCheck>(
         "cppcoreguidelines-interfaces-global-init");
     CheckFactories.registerCheck<NoMallocCheck>("cppcoreguidelines-no-malloc");
Index: clang-tidy/cppcoreguidelines/ConstCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/cppcoreguidelines/ConstCheck.h
@@ -0,0 +1,68 @@
+//===--- ConstCheck.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_CPPCOREGUIDELINES_CONSTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_CONSTCHECK_H
+
+#include "../ClangTidy.h"
+#include <iostream>
+#include <unordered_map>
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// This check warns for every variable, that could be declared as const, but
+/// isn't.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-const.html
+class ConstCheck : public ClangTidyCheck {
+public:
+  ConstCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void onEndOfTranslationUnit() override;
+
+private:
+  void variableRegistering(ast_matchers::MatchFinder *Finder);
+  void handleRegistration(const ast_matchers::MatchFinder::MatchResult &Result);
+
+  void valueTypeMatchers(ast_matchers::MatchFinder *Finder);
+  void checkValueType(const ast_matchers::MatchFinder::MatchResult &Result);
+  void invalidateRefCaptured(const LambdaExpr *Lambda);
+
+  template <typename ASTElement>
+  bool notConstVal(const ast_matchers::MatchFinder::MatchResult &Result,
+                   StringRef matcher_bind) {
+    if (Result.Nodes.getNodeAs<ASTElement>(matcher_bind)) {
+      // std::cout << "Prohibting through " << std::string(matcher_bind)
+                // << std::endl;
+      const auto *Variable = Result.Nodes.getNodeAs<VarDecl>("value-decl");
+      ValueCanBeConst[Variable] = false;
+      return true;
+    }
+    return false;
+  }
+
+  void handleTypeMatchers(ast_matchers::MatchFinder *Finder);
+  void checkHandleType(const ast_matchers::MatchFinder::MatchResult &Result);
+
+  void diagnosePotentialConst();
+
+  llvm::DenseMap<const VarDecl *, bool> ValueCanBeConst;
+  llvm::DenseMap<const VarDecl *, bool> HandleCanBeConst;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_CONSTCHECK_H
Index: clang-tidy/cppcoreguidelines/ConstCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/cppcoreguidelines/ConstCheck.cpp
@@ -0,0 +1,392 @@
+//===--- ConstCheck.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 "ConstCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+#include <iostream>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/*
+ * General Thoughts
+ * ================
+ *
+ * For now: Only local variables are considered. Globals/namespace variables,
+ * paramters and class members are not analyzed.
+ * Parameters have a check already: readability-non-const-parameter
+ *
+ *
+ * Handle = either a pointer or reference
+ * Value  = everything else (Type variable_name;)
+ *
+ * Value Semantic
+ * ==============
+ *  - it is neither global nor namespace level                        + CHECK
+ *  - it never gets assigned to after initialization                  + CHECK
+ *    -> every uninitialized variable can not be const                + CHECK
+ *  - no non-const handle is created with it                          + CHECK
+ *    - no non-const pointer from it                                  + CHECK
+ *    - no non-const pointer argument                                 + CHECK
+ *    - no non-const reference from it                                + CHECK
+ *    - no non-const reference argument                               + CHECK
+ *    - no non-const capture by reference in a lambda                 + CHECK
+ *  - it is not returned as non-const handle from a function          + CHECK
+ *  - it address is not assigned to an out pointer parameter          + CHECK
+ *
+ * primitive Builtins
+ * ----------------
+ *  - it is not modified with an operator (++i,i++,--i,i--)           + CHECK
+ *  - it is not modified with an operator-assignment                  + CHECK
+ *
+ * objects
+ * -------
+ *  - there is no call to a non-const method                          + CHECK
+ *  - there is no call to an non-const overloaded operator            + CHECK
+ *  - there is no non-const iterator created from this type           + CHECK
+ *    (std::begin and friends)
+ *
+ * arrays
+ * ------
+ *  - there is no non-const operator[] access                         + CHECK
+ *  - there is no non-const handle creation of one of the elements    + CHECK
+ *  - there is no non-const iterator created from this type           + CHECK
+ *    (std::begin and friends)
+ *
+ * templated variables
+ * -------------------
+ *  - one can not reason about templated variables, because every sensible
+ *    operation is overloadable and different instantiations will result
+ *    in types with different const-properties.
+ *  - Example: operator+(T& lhs, T& rhs) -> modification might occur for this
+ * type
+ *    -> this fordbids `val = val1 + val2` val1 and val2 to be const
+ *  - only concepts give possibility to infer constness of templated variables
+ *
+ * Handle Semantic
+ * ===============
+ *  - modification of the pointee prohibit constness
+ *
+ * pointers
+ * --------
+ *  - match both for value and handle semantic
+ *
+ * references
+ * ----------
+ *  - only handle semantic applies
+ *  - references to templated types suffer from the same problems as templated
+ *    values
+ *
+ * forwarding reference
+ * --------------------
+ *  - same as references?
+ *
+ * Implementation strategy
+ * =======================
+ *
+ *  - Register every declared local variable/constant with value semantic.
+ *    (pointers, values)
+ *    Store if they can be made const.
+ *    (const int i = 10 : no,
+ *     int *const = &i  : no,
+ *     int i = 10       : yes, -> const int i = 10
+ *     int *p_i = &i    : yes, -> int *const p_i = &i)
+ *  - Register every declared local variable/constant with handle semantic.
+ *    (pointers, references)
+ *    Store if they can be made const, meaning if they can be a const target
+ *    (const int *cp_i = &i : no,
+ *     const int &cr_i = i  : no,
+ *     int *p_i = &i        : yes, -> const int *p_i = &i
+ *     int &r_i = i         : yes, -> const int &r_i = i)
+ *  - Keep 2 dictionaries for values and handles
+ *
+ *  - Match operations/events that forbid values to be const -> mark then 'no'
+ *  - Match operations/events that forbid handles to be const -> mark then 'no'
+ *
+ *  - once the translation unit is finished, determine what can be const, by
+ *    just iterating over all keys and check if they map to 'true'.
+ *    - values that can be const -> emit warning for their type and name
+ *    - handles that can be const -> emit warning for the pointee type and name
+ *    - ignore the rest
+ *
+ * Open Questions
+ * ==============
+ *
+ *  - type conversions:
+ *    - one can overload the type conversion operation and modify a value of a
+ *      class -> implications?
+ *  - what about the 'mutable' keyword -> not considered now, because it applies
+ *    only to class members
+ */
+
+void ConstCheck::registerMatchers(MatchFinder *Finder) {
+  // Ensure that all intersting variables are registered in our mapping.
+  variableRegistering(Finder);
+  valueTypeMatchers(Finder);
+}
+
+void ConstCheck::check(const MatchFinder::MatchResult &Result) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  handleRegistration(Result);
+  checkValueType(Result);
+  // checkHandleType(Result);
+}
+
+// The decision which variable might be made const can only be made at the
+// end of each translation unit.
+void ConstCheck::onEndOfTranslationUnit() { diagnosePotentialConst(); }
+
+void ConstCheck::variableRegistering(MatchFinder *Finder) {
+  const auto HandleType =
+      anyOf(hasType(referenceType()), hasType(pointerType()));
+  const auto ValueType = unless(hasType(referenceType()));
+  const auto ConstType = hasType(isConstQualified());
+  const auto TemplateType = anyOf(hasType(templateTypeParmType()),
+                                  hasType(substTemplateTypeParmType()));
+  const auto LocalVariable =
+      anyOf(hasAncestor(functionDecl(hasBody(compoundStmt()))),
+            hasAncestor(lambdaExpr()));
+
+  // Match local variables, that could be const.
+  // Example: `int i = 10`, `int i` (will be used if program is correct)
+  Finder->addMatcher(varDecl(allOf(LocalVariable, hasInitializer(anything()),
+                                   unless(ConstType), unless(TemplateType),
+                                   unless(isImplicit()), ValueType))
+                         .bind("new-local-value"),
+                     this);
+
+  // Match local handle types, that are not const.
+  // Example: `int &ri`, `int * pi`.
+  Finder->addMatcher(varDecl(allOf(LocalVariable, HandleType,
+                                   unless(isImplicit()), unless(ConstType)))
+                         .bind("new-local-handle"),
+                     this);
+}
+
+void ConstCheck::handleRegistration(const MatchFinder::MatchResult &Result) {
+  // Local variables can be declared as consts.
+  if (const auto *Variable =
+          Result.Nodes.getNodeAs<VarDecl>("new-local-value")) {
+    // std::cout << "Added new local value" << std::endl;
+    if (ValueCanBeConst.find(Variable) == ValueCanBeConst.end()) {
+      ValueCanBeConst[Variable] = true;
+      return;
+    }
+  }
+
+  if (const auto *Variable =
+          Result.Nodes.getNodeAs<VarDecl>("new-local-handle")) {
+    // std::cout << "Added new local handle" << std::endl;
+    if (HandleCanBeConst.find(Variable) == HandleCanBeConst.end()) {
+      HandleCanBeConst[Variable] = true;
+      return;
+    }
+  }
+}
+
+void ConstCheck::valueTypeMatchers(MatchFinder *Finder) {
+  // Matchers for the basic differntiation groups of types.
+  const auto IsHandleType =
+      anyOf(hasType(referenceType()), hasType(pointerType()));
+  const auto IsValueType = unless(hasType(referenceType()));
+
+  // Matchers for non-const handles.
+  const auto IsRefenceToNonConstType =
+      hasType(references(qualType(unless(isConstQualified()))));
+  const auto IsPointerToNonConstType =
+      hasType(pointerType(pointee(unless(isConstQualified()))));
+  const auto IsNonConstHandleType =
+      anyOf(IsRefenceToNonConstType, IsPointerToNonConstType);
+
+  // Match value, array and pointer access.
+  // Pointer do have value and reference semantic.
+  const auto ValueDeclRef = declRefExpr(
+      allOf(hasDeclaration(varDecl(unless(isImplicit())).bind("value-decl")),
+            IsValueType));
+  const auto ArrayAccess =
+      arraySubscriptExpr(hasBase(ignoringImpCasts(ValueDeclRef)));
+  const auto IsValueDeclRefExpr = anyOf(ArrayAccess, ValueDeclRef);
+
+  // Classical assignment of any form (=, +=, <<=, ...) modifies the LHS
+  // and prohibts it from being const.
+  Finder->addMatcher(
+      binaryOperator(allOf(isAssignmentOperator(), hasLHS(IsValueDeclRefExpr)))
+          .bind("value-assignment"),
+      this);
+
+  // Usage of the '++' or '--' operator modifies a variable.
+  Finder->addMatcher(
+      unaryOperator(allOf(anyOf(hasOperatorName("++"), hasOperatorName("--")),
+                          hasUnaryOperand(IsValueDeclRefExpr)))
+          .bind("value-unary-modification"),
+      this);
+
+  // Check the address operator.
+  Finder->addMatcher(
+      unaryOperator(allOf(hasOperatorName("&"),
+                          // Checking for the ImplicitCastExpr is enough,
+                          // because a pointer can get casted only in the 'add
+                          // const' direction implicitly.
+                          unless(hasParent(implicitCastExpr())),
+                          hasUnaryOperand(IsValueDeclRefExpr)))
+          .bind("value-address-to-non-const"),
+      this);
+
+  // Check creation of references to this value.
+  Finder->addMatcher(
+      varDecl(allOf(IsNonConstHandleType, hasInitializer(IsValueDeclRefExpr),
+                    unless(isImplicit())))
+          .bind("value-non-const-reference"),
+      this);
+
+  // Check function calls that bind by reference.
+  Finder->addMatcher(
+      callExpr(forEachArgumentWithParam(
+          IsValueDeclRefExpr, parmVarDecl(IsNonConstHandleType)
+                                  .bind("value-non-const-ref-call-param"))),
+      this);
+
+  // Check return values that reference a value.
+  Finder->addMatcher(
+      functionDecl(
+          allOf(hasDescendant(returnStmt(hasReturnValue(IsValueDeclRefExpr))),
+                returns(qualType(
+                    references(qualType(unless(isConstQualified())))))))
+          .bind("returns-non-const-ref"),
+      this);
+
+  // Check for direct method calls, that modify its object by declaration.
+  Finder->addMatcher(
+      cxxMemberCallExpr(allOf(on(IsValueDeclRefExpr),
+                              unless(callee(cxxMethodDecl(isConst())))))
+          .bind("non-const-member-call"),
+      this);
+
+  // Check for operator calls, that are non-const. E.g. operator=
+  Finder->addMatcher(
+      cxxOperatorCallExpr(allOf(hasArgument(0, IsValueDeclRefExpr),
+                                unless(callee(cxxMethodDecl(isConst())))))
+          .bind("non-const-operator-call"),
+      this);
+
+  // Check for range-for loops that declare non-const handles as loop variable.
+  Finder->addMatcher(
+      cxxForRangeStmt(allOf(hasLoopVariable(IsNonConstHandleType),
+                            hasRangeInit(IsValueDeclRefExpr)))
+          .bind("non-const-range-for"),
+      this);
+
+  // Lambda expression can capture variables by reference which invalidates
+  // the captured variables. Lambdas capture only the variables they actually
+  // use!
+  Finder->addMatcher(lambdaExpr().bind("value-lambda"), this);
+}
+
+void ConstCheck::checkValueType(const MatchFinder::MatchResult &Res) {
+  bool Prohibited = false;
+
+  // Assignment of any form prohibits the LHS to be const.
+  Prohibited = notConstVal<BinaryOperator>(Res, "value-assignment");
+  // Usage of the '++' or '--' operator modifies a value.
+  Prohibited |= notConstVal<UnaryOperator>(Res, "value-unary-modification");
+  // The address of the values has been taken and did not result in a
+  // pointer to const.
+  Prohibited |= notConstVal<UnaryOperator>(Res, "value-address-to-non-const");
+  // A reference is initialized with a value.
+  Prohibited |= notConstVal<VarDecl>(Res, "value-non-const-reference");
+  // A call where a parameter is a non-const reference.
+  Prohibited |= notConstVal<ParmVarDecl>(Res, "value-non-const-ref-call-param");
+  // A function returning a non-const reference prohibts its return value
+  // to be const.
+  Prohibited |= notConstVal<FunctionDecl>(Res, "returns-non-const-ref");
+  // Calling a member function, that is not declared as const prohibts
+  // constness of the value.
+  Prohibited |= notConstVal<CXXMemberCallExpr>(Res, "non-const-member-call");
+  // Calling an overloaded operator that is not declared as const probits
+  // constness similar to member calls.
+  Prohibited |=
+      notConstVal<CXXOperatorCallExpr>(Res, "non-const-operator-call");
+  // Range-For can loop in a modifying way over the range. This is equivalent
+  // to taking a reference/pointer to one of the elements of the range.
+  Prohibited |= notConstVal<CXXForRangeStmt>(Res, "non-const-range-for");
+
+  // If the matchable targets did result in prohibiton, we can stop.
+  // Otherwise analysis that does not function with the same pattern must be
+  // applied.
+  if (Prohibited)
+    return;
+
+  // Analysis of the lambda is more difficult.
+  // Offloaded into a separate function.
+  if (const auto *Lambda = Res.Nodes.getNodeAs<LambdaExpr>("value-lambda")) {
+    invalidateRefCaptured(Lambda);
+    return;
+  }
+}
+
+void ConstCheck::invalidateRefCaptured(const LambdaExpr *Lambda) {
+  for (const auto capture : Lambda->captures()) {
+    if (capture.capturesVariable() &&
+        capture.getCaptureKind() == LambdaCaptureKind::LCK_ByRef) {
+      ValueCanBeConst[capture.getCapturedVar()] = false;
+    }
+  }
+}
+
+void ConstCheck::diagnosePotentialConst() {
+  // Diagnose all potential values.
+  for (const auto it : ValueCanBeConst) {
+    bool VarCanBeConst = it.second;
+
+    if (VarCanBeConst) {
+      const VarDecl *Variable = it.first;
+
+      diag(Variable->getLocStart(),
+           "variable %0 of type %1 can be declared const")
+          << Variable << Variable->getType();
+    }
+  }
+
+  /*
+  for (const auto it : HandleCanBeConst) {
+    bool HandleCanBeConst = it.second;
+
+    // Example: `int& ri` could be `const int& ri`.
+    if (HandleCanBeConst) {
+      const VarDecl *Variable = it.first;
+
+      // Differentiate between pointers and references.
+      QualType HandleType = Variable->getType();
+      if (HandleType->isReferenceType()) {
+
+        diag(Variable->getLocStart(),
+             "reference type %0 of variable %1 can be declared const")
+            << HandleType << Variable;
+      } else if (HandleType->isPointerType()) {
+
+        diag(Variable->getLocStart(),
+             "pointee type %0 of variable %1 can be declared const")
+            << HandleType << Variable;
+      } else
+        llvm_unreachable("Expected handle type");
+    }
+  }
+  */
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/cppcoreguidelines/CMakeLists.txt
===================================================================
--- clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -2,6 +2,7 @@
 
 add_clang_library(clangTidyCppCoreGuidelinesModule
   AvoidGotoCheck.cpp
+  ConstCheck.cpp
   CppCoreGuidelinesTidyModule.cpp
   InterfacesGlobalInitCheck.cpp
   NoMallocCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to