[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 365252.
JonasToth marked an inline comment as done.
JonasToth added a comment.

- rebase to master
- fix a crash where a scope matched but non of its variables, leading to nullptr dereference (found with the current test-suite)
- remove outcommenting of tests that showed undesirable behavior


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
@@ -920,7 +920,6 @@
   my_function_type ptr = function_ref_target;
 }
 
-#if 0
 template <typename T>
 T &return_ref() {
   static T global;
@@ -935,10 +934,16 @@
   // for the analysis. That results in bad transformations.
   auto auto_val0 = T{};
   auto &auto_val1 = auto_val0; // Bad
+  // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'auto_val1' of type 'System &' can be declared 'const'
+  // CHECK-MESSAGES:[[@LINE-2]]:3: warning: variable 'auto_val1' of type 'int &' can be declared 'const'
   auto *auto_val2 = &auto_val0;
 
-  auto auto_ref0 = return_ref<T>();  // Bad
+  auto auto_ref0 = return_ref<T>();
+  // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'auto_ref0' of type 'System' can be declared 'const'
+  // CHECK-MESSAGES:[[@LINE-2]]:3: warning: variable 'auto_ref0' of type 'int' can be declared 'const'
   auto &auto_ref1 = return_ref<T>(); // Bad
+  // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'auto_ref1' of type 'System &' can be declared 'const'
+  // CHECK-MESSAGES:[[@LINE-2]]:3: warning: variable 'auto_ref1' of type 'int &' can be declared 'const'
   auto *auto_ref2 = return_ptr<T>();
 
   auto auto_ptr0 = return_ptr<T>();
@@ -948,10 +953,11 @@
   using MyTypedef = T;
   auto auto_td0 = MyTypedef{};
   auto &auto_td1 = auto_td0; // Bad
+  // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'auto_td1' of type 'System &' can be declared 'const'
+  // CHECK-MESSAGES:[[@LINE-2]]:3: warning: variable 'auto_td1' of type 'int &' can be declared 'const'
   auto *auto_td2 = &auto_td0;
 }
 void instantiate_auto_cases() {
   auto_usage_variants<int>();
   auto_usage_variants<System>();
 }
-#endif
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
@@ -90,11 +90,12 @@
 
 void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *LocalScope = Result.Nodes.getNodeAs<CompoundStmt>("scope");
-  assert(LocalScope && "Did not match scope for local variable");
-  registerScope(LocalScope, Result.Context);
-
   const auto *Variable = Result.Nodes.getNodeAs<VarDecl>("local-value");
-  assert(Variable && "Did not match local variable definition");
+
+  // There have been crashes on 'Variable == nullptr', even though the matcher
+  // is not conditional. This comes probably from 'findAll'-matching.
+  if (!Variable || !LocalScope)
+    return;
 
   VariableCategory VC = VariableCategory::Value;
   if (Variable->getType()->isReferenceType())
@@ -118,6 +119,9 @@
   if (VC == VariableCategory::Value && !AnalyzeValues)
     return;
 
+  // The scope is only registered if the analysis shall be run.
+  registerScope(LocalScope, Result.Context);
+
   // Offload const-analysis to utility function.
   if (ScopesCache[LocalScope]->isMutated(Variable))
     return;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D54943.365252.patch
Type: text/x-patch
Size: 3341 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210809/4fd836b7/attachment.bin>