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;
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;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits