diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt @@ -6,6 +6,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule AvoidGotoCheck.cpp AvoidNonConstGlobalVariablesCheck.cpp + ConstCorrectnessCheck.cpp CppCoreGuidelinesTidyModule.cpp InitVariablesCheck.cpp InterfacesGlobalInitCheck.cpp @@ -28,6 +29,12 @@ SpecialMemberFunctionsCheck.cpp LINK_LIBS + clangAnalysis + clangAST + clangASTMatchers + clangBasic + clangLex + clangSerialization clangTidy clangTidyMiscModule clangTidyModernizeModule diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h @@ -0,0 +1,59 @@ +//===--- ConstCorrectnessCheck.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_CONSTCORRECTNESSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_CONSTCORRECTNESSCHECK_H + +#include "../ClangTidy.h" +#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h" + +namespace clang { +namespace tidy { + +namespace cppcoreguidelines { + +/// This check warns on variables which could be declared const but are not. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-const.html +class ConstCorrectnessCheck : public ClangTidyCheck { +public: + ConstCorrectnessCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + AnalyzeValues(Options.get("AnalyzeValues", 1)), + AnalyzeReferences(Options.get("AnalyzeReferences", 1)), + WarnPointersAsValues(Options.get("WarnPointersAsValues", 0)), + TransformValues(Options.get("TransformValues", 1)), + TransformReferences(Options.get("TransformReferences", 1)), + TransformPointersAsValues(Options.get("TransformPointersAsValues", 0)) { + } + + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + void registerScope(const CompoundStmt *LocalScope, ASTContext *Context); + + using MutationAnalyzer = std::unique_ptr; + llvm::DenseMap ScopesCache; + + const bool AnalyzeValues; + const bool AnalyzeReferences; + const bool WarnPointersAsValues; + + const bool TransformValues; + const bool TransformReferences; + const bool TransformPointersAsValues; +}; + +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_CONSTCORRECTNESSCHECK_H diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp @@ -0,0 +1,178 @@ +//===--- ConstCorrectnessCheck.cpp - 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 +// +//===----------------------------------------------------------------------===// + +#include "ConstCorrectnessCheck.h" +#include "../utils/FixItHintUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace cppcoreguidelines { + +namespace { +// FIXME: This matcher exists in some other code-review as well. +// It should probably move to ASTMatchers. +AST_MATCHER(VarDecl, isLocal) { return Node.isLocalVarDecl(); } +AST_MATCHER_P(DeclStmt, containsDeclaration2, + ast_matchers::internal::Matcher, InnerMatcher) { + return ast_matchers::internal::matchesFirstInPointerRange( + InnerMatcher, Node.decl_begin(), Node.decl_end(), Finder, Builder); +} +AST_MATCHER(ReferenceType, isSpelledAsLValue) { + return Node.isSpelledAsLValue(); +} +AST_MATCHER(Type, isDependentType) { return Node.isDependentType(); } +} // namespace + +void ConstCorrectnessCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AnalyzeValues", AnalyzeValues); + Options.store(Opts, "AnalyzeReferences", AnalyzeReferences); + Options.store(Opts, "WarnPointersAsValues", WarnPointersAsValues); + + Options.store(Opts, "TransformValues", TransformValues); + Options.store(Opts, "TransformReferences", TransformReferences); + Options.store(Opts, "TransformPointersAsValues", TransformPointersAsValues); +} + +void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) { + const auto ConstType = hasType(isConstQualified()); + const auto ConstReference = hasType(references(isConstQualified())); + const auto RValueReference = hasType( + referenceType(anyOf(rValueReferenceType(), unless(isSpelledAsLValue())))); + + const auto TemplateType = anyOf( + hasType(hasCanonicalType(templateTypeParmType())), + hasType(substTemplateTypeParmType()), hasType(isDependentType()), + // References to template types, their substitutions or typedefs to + // template types need to be considered as well. + hasType(referenceType(pointee(hasCanonicalType(templateTypeParmType())))), + hasType(referenceType(pointee(substTemplateTypeParmType())))); + + const auto AutoTemplateType = varDecl( + anyOf(hasType(autoType()), hasType(referenceType(pointee(autoType()))), + hasType(pointerType(pointee(autoType())))), + hasInitializer(isInstantiationDependent())); + + const auto FunctionPointerRef = + hasType(hasCanonicalType(referenceType(pointee(functionType())))); + + // Match local variables which could be 'const' if not modified later. + // Example: `int i = 10` would match `int i`. + const auto LocalValDecl = varDecl( + allOf(isLocal(), hasInitializer(anything()), + unless(anyOf(ConstType, ConstReference, TemplateType, + AutoTemplateType, RValueReference, FunctionPointerRef, + hasType(cxxRecordDecl(isLambda())), isImplicit())))); + + // Match the function scope for which the analysis of all local variables + // shall be run. + const auto FunctionScope = functionDecl(hasBody( + compoundStmt(findAll(declStmt(allOf(containsDeclaration2( + LocalValDecl.bind("local-value")), + unless(has(decompositionDecl())))) + .bind("decl-stmt"))) + .bind("scope"))); + + Finder->addMatcher(FunctionScope, this); +} + +/// Classify for a variable in what the Const-Check is interested. +enum class VariableCategory { Value, Reference, Pointer }; + +void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) { + const auto *LocalScope = Result.Nodes.getNodeAs("scope"); + assert(LocalScope && "Did not match scope for local variable"); + registerScope(LocalScope, Result.Context); + + const auto *Variable = Result.Nodes.getNodeAs("local-value"); + assert(Variable && "Did not match local variable definition"); + + VariableCategory VC = VariableCategory::Value; + if (Variable->getType()->isReferenceType()) + VC = VariableCategory::Reference; + if (Variable->getType()->isPointerType()) + VC = VariableCategory::Pointer; + + // Each variable can only in one category: Value, Pointer, Reference. + // Analysis can be controlled for every category. + if (VC == VariableCategory::Reference && !AnalyzeReferences) + return; + + if (VC == VariableCategory::Reference && + Variable->getType()->getPointeeType()->isPointerType() && + !WarnPointersAsValues) + return; + + if (VC == VariableCategory::Pointer && !WarnPointersAsValues) + return; + + if (VC == VariableCategory::Value && !AnalyzeValues) + return; + + // Offload const-analysis to utility function. + if (ScopesCache[LocalScope]->isMutated(Variable)) + return; + + auto Diag = diag(Variable->getBeginLoc(), + "variable %0 of type %1 can be declared 'const'") + << Variable << Variable->getType(); + + const auto *VarDeclStmt = Result.Nodes.getNodeAs("decl-stmt"); + + // It can not be guaranteed that the variable is declared isolated, therefore + // a transformation might effect the other variables as well and be incorrect. + if (VarDeclStmt == nullptr || !VarDeclStmt->isSingleDecl()) + return; + + using namespace utils::fixit; + using llvm::Optional; + if (VC == VariableCategory::Value && TransformValues) { + if (Optional Fix = addQualifierToVarDecl( + *Variable, *Result.Context, DeclSpec::TQ_const, + QualifierTarget::Value, QualifierPolicy::Right)) { + Diag << *Fix; + // FIXME: Add '{}' for default initialization if no user-defined default + // constructor exists and there is no initializer. + } + return; + } + + if (VC == VariableCategory::Reference && TransformReferences) { + if (Optional Fix = addQualifierToVarDecl( + *Variable, *Result.Context, DeclSpec::TQ_const, + QualifierTarget::Value, QualifierPolicy::Right)) + Diag << *Fix; + return; + } + + if (VC == VariableCategory::Pointer) { + if (WarnPointersAsValues && TransformPointersAsValues) { + if (Optional Fix = addQualifierToVarDecl( + *Variable, *Result.Context, DeclSpec::TQ_const, + QualifierTarget::Value, QualifierPolicy::Right)) + Diag << *Fix; + } + return; + } +} + +void ConstCorrectnessCheck::registerScope(const CompoundStmt *LocalScope, + ASTContext *Context) { + if (ScopesCache.find(LocalScope) == ScopesCache.end()) + ScopesCache.insert(std::make_pair( + LocalScope, + std::make_unique(*LocalScope, *Context))); +} + +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -16,6 +16,7 @@ #include "../readability/MagicNumbersCheck.h" #include "AvoidGotoCheck.h" #include "AvoidNonConstGlobalVariablesCheck.h" +#include "ConstCorrectnessCheck.h" #include "InitVariablesCheck.h" #include "InterfacesGlobalInitCheck.h" #include "MacroUsageCheck.h" @@ -52,6 +53,8 @@ "cppcoreguidelines-avoid-magic-numbers"); CheckFactories.registerCheck( "cppcoreguidelines-avoid-non-const-global-variables"); + CheckFactories.registerCheck( + "cppcoreguidelines-const-correctness"); CheckFactories.registerCheck( "cppcoreguidelines-explicit-virtual-functions"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -100,6 +100,14 @@ Finds condition variables in nested ``if`` statements that were also checked in the outer ``if`` statement and were not changed. +- New :doc:`cppcoreguidelines-const-correctness + ` check. + + Suggest adding ``const`` to unmodified local variables. + +New check aliases +^^^^^^^^^^^^^^^^^ + Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst @@ -0,0 +1,68 @@ +.. title:: clang-tidy - cppcoreguidelines-const-correctness + +cppcoreguidelines-const-correctness +=================================== + +This check implements detection of local variables which could be declared as +``const``, but are not. Declaring variables as ``const`` is required by many +coding guidelines, such as: +`CppCoreGuidelines ES.25 `_ +and `High Integrity C++ 7.1.2 `_. + +Please note that this analysis is type-based only. Variables that are not modified +but non-const handles might escape out of the scope are not diagnosed as potential +``const``. + +.. code-block:: c++ + + // Declare a variable, which is not ``const`` ... + int i = 42; + // but use it as read-only. This means that `i` can be declared ``const``. + int result = i * i; + +The check analyzes values, pointers and references (if configured that way). +For better understanding some code samples: + +.. code-block:: c++ + + // Normal values like built-ins or objects. + int potential_const_int = 42; + int copy_of_value = potential_const_int; + + MyClass could_be_const; + could_be_const.const_qualified_method(); + + // References can be declared const as well. + int &reference_value = potential_const_int; + int another_copy = reference_value; + + // Similar behaviour for pointers. + int *pointer_variable = &potential_const_int; + int last_copy = *pointer_variable; + + +Options +------- + +.. option:: AnalyzeValues (default = 1) + + Enable or disable the analysis of ordinary value variables, like ``int i = 42;`` + +.. option:: AnalyzeReferences (default = 1) + + Enable or disable the analysis of reference variables, like ``int &ref = i;`` + +.. option:: WarnPointersAsValues (default = 0) + + This option enables the suggestion for ``const`` of the pointer itself. + Pointer values have two possibilities to be ``const``, the pointer itself + and the value pointing to. + + .. code-block:: c++ + + const int value = 42; + const int * const pointer_variable = &value; + + // The following operations are forbidden for `pointer_variable`. + // *pointer_variable = 44; + // pointer_variable = nullptr; diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-cxx17.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-cxx17.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-cxx17.cpp @@ -0,0 +1,49 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-const-correctness %t -- -- -std=c++17 + +template +struct MyPair { + L left; + R right; + MyPair(const L &ll, const R &rr) : left{ll}, right{rr} {} +}; + +void f() { + // FIXME: Decomposition Decls need special treatment, because they require to use 'auto' + // and the 'const' should only be added if all elements can be const. + // The issue is similar to multiple declarations in one statement. + // Simply bail for now. + auto [np_local0, np_local1] = MyPair(42, 42); + np_local0++; + np_local1++; + + auto [np_local2, p_local0] = MyPair(42., 42.); + np_local2++; + + auto [p_local1, np_local3] = MyPair(42., 42.); + np_local3++; + + auto [p_local2, p_local3] = MyPair(42., 42.); +} + +void g() { + int p_local0 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const' +} + +template +struct DoGooder { + DoGooder(void *accessor, SomeValue value) { + } +}; +struct Bingus { + static constexpr auto someRandomConstant = 99; +}; +template +struct HardWorker { + HardWorker() { + const DoGooder anInstanceOf(nullptr, Foo::someRandomConstant); + } +}; +struct TheContainer { + HardWorker m_theOtherInstance; +}; diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-pointer-as-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-pointer-as-values.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-pointer-as-values.cpp @@ -0,0 +1,11 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-const-correctness %t \ +// RUN: -config='{CheckOptions: \ +// RUN: [{key: "cppcoreguidelines-const-correctness.AnalyzeValues", value: 1},\ +// RUN: {key: "cppcoreguidelines-const-correctness.WarnPointersAsValues", value: 1}]}' \ +// RUN: -- + +void potential_const_pointer() { + 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' +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-pointer-as-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-pointer-as-values.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-pointer-as-values.cpp @@ -0,0 +1,13 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-const-correctness %t \ +// RUN: -config='{CheckOptions: \ +// RUN: [{key: "cppcoreguidelines-const-correctness.AnalyzeValues", value: 1},\ +// RUN: {key: "cppcoreguidelines-const-correctness.WarnPointersAsValues", value: 1}, \ +// RUN: {key: "cppcoreguidelines-const-correctness.TransformPointersAsValues", value: 1},\ +// RUN: ]}' -- + +void potential_const_pointer() { + 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' + // CHECK-FIXES: const +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp @@ -0,0 +1,168 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-const-correctness %t -- \ +// RUN: -config="{CheckOptions: [\ +// RUN: {key: 'cppcoreguidelines-const-correctness.TransformValues', value: 1},\ +// RUN: {key: 'cppcoreguidelines-const-correctness.WarnPointersAsValues', value: 0}, \ +// RUN: {key: 'cppcoreguidelines-const-correctness.TransformPointersAsValues', value: 0}, \ +// RUN: ]}" -- + +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 + +// Lambdas should be ignored, because they do not follow the normal variable +// semantic (e.g. the type is only known to the compiler). +void lambdas() { + auto Lambda = [](int i) { return i < 0; }; +} + +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' + // CHECK-FIXES: const +} + +void nested_scopes() { + { + int p_local1 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: variable 'p_local1' of type 'int' can be declared 'const' + // CHECK-FIXES: const + } +} + +template +void define_locals(T np_arg0, T &np_arg1, int np_arg2) { + T np_local0 = 0; + int p_local1 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared 'const' + // CHECK-FIXES: const +} + +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; + + double NonConstMember; + const double ConstMember; + + double &NonConstMemberRef; + const double &ConstMemberRef; + + double *NonConstMemberPtr; + const double *ConstMemberPtr; +}; + +void direct_class_access() { + ConstNonConstClass p_local0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'ConstNonConstClass' can be declared 'const' + // CHECK-FIXES: const + p_local0.constMethod(); +} + +void class_access_array() { + ConstNonConstClass p_local0[2]; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'ConstNonConstClass [2]' can be declared 'const' + // CHECK-FIXES: const + p_local0[0].constMethod(); +} + +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 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' + // CHECK-FIXES: const +} + +void range_for() { + int np_local0[2] = {1, 2}; + int *np_local3[2] = {&np_local0[0], &np_local0[1]}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'np_local3' of type 'int *[2]' can be declared 'const' + // CHECK-FIXES: const + for (int *non_const_ptr : np_local3) { + *non_const_ptr = 45; + } +} + +void casts() { + decltype(sizeof(void *)) p_local0 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'decltype(sizeof(void *))' (aka 'unsigned long') can be declared 'const' + // CHECK-FIXES: const +} + +// taken from http://www.cplusplus.com/reference/type_traits/integral_constant/ +template +struct integral_constant { + static constexpr T value = v; + using value_type = T; + using type = integral_constant; + constexpr operator T() { return v; } +}; + +template +struct is_integral : integral_constant {}; +template <> +struct is_integral : integral_constant {}; + +template +struct not_integral : integral_constant {}; +template <> +struct not_integral : integral_constant {}; + +// taken from http://www.cplusplus.com/reference/type_traits/enable_if/ +template +struct enable_if {}; + +template +struct enable_if { using type = T; }; + +template +struct TMPClass { + T alwaysConst() const { return T{}; } + + template ::value>::type> + T sometimesConst() const { return T{}; } + + template ::value>::type> + T sometimesConst() { return T{}; } +}; + +void meta_type() { + TMPClass p_local0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'TMPClass' can be declared 'const' + // CHECK-FIXES: const + p_local0.alwaysConst(); + p_local0.sometimesConst(); + + TMPClass p_local1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'TMPClass' can be declared 'const' + // CHECK-FIXES: const + p_local1.alwaysConst(); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp @@ -0,0 +1,957 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-const-correctness %t -- \ +// RUN: -config="{CheckOptions: [\ +// RUN: {key: 'cppcoreguidelines-const-correctness.TransformValues', value: 1}, \ +// RUN: {key: 'cppcoreguidelines-const-correctness.WarnPointersAsValues', value: 0}, \ +// RUN: {key: 'cppcoreguidelines-const-correctness.TransformPointersAsValues', value: 0}, \ +// RUN: ]}" -- + +// ------- 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 + +// Lambdas should be ignored, because they do not follow the normal variable +// semantic (e.g. the type is only known to the compiler). +void lambdas() { + auto Lambda = [](int i) { return i < 0; }; +} + +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 nested_scopes() { + int p_local0 = 2; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const' + int np_local0 = 42; + + { + int p_local1 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: variable 'p_local1' of type 'int' can be declared 'const' + np_local0 *= 2; + } +} + +void ignore_reference_to_pointers() { + int *np_local0 = nullptr; + int *&np_local1 = np_local0; +} + +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; + const int *const p0_np_local1 = &np_local1; + int *const p1_np_local1 = &np_local1; + *p1_np_local0 = 43; + + int np_local2 = 42; + function_inout_pointer(&np_local2); + + // Prevents const. + int np_local3 = 42; + out = &np_local3; // This returns and invalid address, its just about the AST + + int p_local1 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared 'const' + const int *const p0_p_local1 = &p_local1; + + int p_local2 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'int' can be declared 'const' + function_in_pointer(&p_local2); +} + +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; + r1_np_local0 = 43; + 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 +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; +} + +template +void more_template_locals() { + const T np_local0 = {}; + auto np_local1 = T{}; + T &np_local2 = np_local1; + T *np_local_ptr = &np_local1; + + const auto np_local3 = T{}; + // FIXME: False positive, the reference points to a template type and needs + // to be excluded from analysis, but somehow isn't (matchers don't work) + auto &np_local4 = np_local3; + + const auto *np_local5 = &np_local3; + auto *np_local6 = &np_local1; + + using TypedefToTemplate = T; + TypedefToTemplate np_local7{}; + // FIXME: False positive, the reference points to a template type and needs + // to be excluded from analysis, but somehow isn't (matchers don't work) + // auto &np_local8 = np_local7; + const auto &np_local9 = np_local7; + auto np_local10 = np_local7; + auto *np_local11 = &np_local10; + const auto *const np_local12 = &np_local10; + + // FIXME: False positive, the reference points to a template type and needs + // to be excluded from analysis, but somehow isn't (matchers don't work) + // TypedefToTemplate &np_local13 = np_local7; + TypedefToTemplate *np_local14 = &np_local7; +} + +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); + more_template_locals(); +} + +struct ConstNonConstClass { + ConstNonConstClass(); + ConstNonConstClass(double &np_local0); + double nonConstMethod() {} + double constMethod() const {} + double modifyingMethod(double &np_arg0) const; + + double NonConstMember; + const double ConstMember; + + double &NonConstMemberRef; + const double &ConstMemberRef; + + double *NonConstMemberPtr; + const double *ConstMemberPtr; +}; + +void direct_class_access() { + 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' + + ConstNonConstClass np_local3; + np_local3.NonConstMember = 42.; + + ConstNonConstClass np_local4; + np_local4.NonConstMemberRef = 42.; + + ConstNonConstClass p_local3; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local3' of type 'ConstNonConstClass' can be declared 'const' + const double val0 = p_local3.NonConstMember; + const double val1 = p_local3.NonConstMemberRef; + const double val2 = *p_local3.NonConstMemberPtr; + + ConstNonConstClass p_local4; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local4' of type 'ConstNonConstClass' can be declared 'const' + *np_local4.NonConstMemberPtr = 42.; +} + +void class_access_array() { + ConstNonConstClass np_local0[2]; + np_local0[0].constMethod(); + np_local0[1].constMethod(); + np_local0[1].nonConstMethod(); + + ConstNonConstClass p_local0[2]; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'ConstNonConstClass [2]' can be declared 'const' + p_local0[0].constMethod(); + np_local0[1].constMethod(); +} + +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]; // Could be `double *const`, but warning deactivated by default + + double np_local1[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.}; + double &non_const_ref = np_local1[1]; + non_const_ref = 42.; + + 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]; // Could be `const double *const`, but warning deactivated by default + + 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) { + non_const_ref = 42; + } + + int np_local1[2] = {1, 2}; + for (auto &non_const_ref : np_local1) { + non_const_ref = 43; + } + + int np_local2[2] = {1, 2}; + for (auto &&non_const_ref : np_local2) { + non_const_ref = 44; + } + + // FIXME the warning message is suboptimal. It could be defined as + // `int *const np_local3[2]` because the pointers are not reseated. + // But this is not easily deducable from the warning. + int *np_local3[2] = {&np_local0[0], &np_local0[1]}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'np_local3' of type 'int *[2]' can be declared 'const' + for (int *non_const_ptr : np_local3) { + *non_const_ptr = 45; + } + + // FIXME same as above, but silenced + int *const np_local4[2] = {&np_local0[0], &np_local0[1]}; + for (auto *non_const_ptr : np_local4) { + *non_const_ptr = 46; + } + + 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) { + } + + 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) { + } +} + +inline void *operator new(decltype(sizeof(void *)), void *p) { return p; } + +struct Value { +}; +void placement_new() { + Value Mem; + Value *V = new (&Mem) Value; +} + +struct ModifyingConversion { + operator int() { return 15; } +}; +struct NonModifyingConversion { + operator int() const { return 15; } +}; +void conversion_operators() { + ModifyingConversion np_local0; + NonModifyingConversion p_local0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'NonModifyingConversion' can be declared 'const' + + int np_local1 = np_local0; + np_local1 = p_local0; +} + +void casts() { + decltype(sizeof(void *)) p_local0 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'decltype(sizeof(void *))' (aka 'unsigned long') can be declared 'const' + auto np_local0 = reinterpret_cast(p_local0); + np_local0 = nullptr; + + int p_local1 = 43; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared 'const' + short p_local2 = static_cast(p_local1); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'short' can be declared 'const' + + int np_local1 = p_local2; + int &np_local2 = static_cast(np_local1); + np_local2 = 5; +} + +void ternary_operator() { + int np_local0 = 1, np_local1 = 2; + int &np_local2 = true ? np_local0 : np_local1; + np_local2 = 2; + + int p_local0 = 3, np_local3 = 5; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const' + const int &np_local4 = true ? p_local0 : ++np_local3; + + int np_local5[3] = {1, 2, 3}; + int &np_local6 = np_local5[1] < np_local5[2] ? np_local5[0] : np_local5[2]; + np_local6 = 42; + + int np_local7[3] = {1, 2, 3}; + int *np_local8 = np_local7[1] < np_local7[2] ? &np_local7[0] : &np_local7[2]; + *np_local8 = 42; +} + +// taken from http://www.cplusplus.com/reference/type_traits/integral_constant/ +template +struct integral_constant { + static constexpr T value = v; + using value_type = T; + using type = integral_constant; + constexpr operator T() { return v; } +}; + +template +struct is_integral : integral_constant {}; +template <> +struct is_integral : integral_constant {}; + +template +struct not_integral : integral_constant {}; +template <> +struct not_integral : integral_constant {}; + +// taken from http://www.cplusplus.com/reference/type_traits/enable_if/ +template +struct enable_if {}; + +template +struct enable_if { using type = T; }; + +template +struct TMPClass { + T alwaysConst() const { return T{}; } + + template ::value>::type> + T sometimesConst() const { return T{}; } + + template ::value>::type> + T sometimesConst() { return T{}; } +}; + +void meta_type() { + TMPClass p_local0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'TMPClass' can be declared 'const' + p_local0.alwaysConst(); + p_local0.sometimesConst(); + + TMPClass p_local1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'TMPClass' can be declared 'const' + p_local1.alwaysConst(); + + TMPClass np_local0; + np_local0.alwaysConst(); + np_local0.sometimesConst(); +} + +// This test is the essence from llvm/lib/Support/MemoryBuffer.cpp at line 450 +template +struct to_construct : T { + to_construct(int &j) {} +}; +template +void placement_new_in_unique_ptr() { + int p_local0 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const' + int np_local0 = p_local0; + new to_construct(np_local0); +} + +struct stream_obj {}; +stream_obj &operator>>(stream_obj &o, unsigned &foo); +void input_operator() { + stream_obj np_local0; + unsigned np_local1 = 42; + np_local0 >> np_local1; +} + +struct stream_obj_template {}; +template +IStream &operator>>(IStream &o, unsigned &foo); + +template +void input_operator_template() { + Stream np_local0; + unsigned np_local1 = 42; + np_local0 >> np_local1; +} + +// Test bit fields +struct HardwareRegister { + unsigned field : 5; + unsigned : 7; + unsigned another : 20; +}; + +void TestRegisters() { + HardwareRegister np_reg0; + np_reg0.field = 3; + + HardwareRegister p_reg1{3, 22}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_reg1' of type 'HardwareRegister' can be declared 'const' + const unsigned p_val = p_reg1.another; +} + +struct IntWrapper { + IntWrapper &operator=(unsigned value) { return *this; } + template + friend Istream &operator>>(Istream &is, IntWrapper &rhs); +}; +struct IntMaker { + friend IntMaker &operator>>(IntMaker &, unsigned &); +}; +template +Istream &operator>>(Istream &is, IntWrapper &rhs) { + unsigned np_local0 = 0; + is >> np_local0; + return is; +} + +struct Actuator { + int actuations; +}; +struct Sensor { + int observations; +}; +struct System : public Actuator, public Sensor { +}; +int some_computation(int arg); +int test_inheritance() { + System np_sys; + np_sys.actuations = 5; + return some_computation(np_sys.actuations); +} +struct AnotherActuator : Actuator { +}; +Actuator &test_return_polymorphic() { + static AnotherActuator np_local0; + return np_local0; +} + +using f_signature = int *(*)(int &); +int *my_alloc(int &size) { return new int[size]; } +struct A { + int f(int &i) { return i + 1; } + int (A::*x)(int &); +}; +void f() { + int p_local0 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const' + int np_local0 = 42; + f_signature action = my_alloc; + action(np_local0); + my_alloc(np_local0); + + int np_local1 = 42; + A a; + a.x = &A::f; + (a.*(a.x))(np_local1); +} + +struct nested_data { + int more_data; +}; +struct repro_assignment_to_reference { + int my_data; + nested_data nested; +}; +void assignment_reference() { + repro_assignment_to_reference np_local0{42}; + int &np_local1 = np_local0.my_data; + np_local1++; + + repro_assignment_to_reference np_local2; + int &np_local3 = np_local2.nested.more_data; + np_local3++; +} + +struct non_const_iterator { + int data[42]; + + int *begin() { return &data[0]; } + int *end() { return &data[41]; } +}; + +// The problem is, that 'begin()' and 'end()' are not const overloaded, so +// they are always a mutation. If 'np_local1' is fixed to const it results in +// a compilation error. +void for_bad_iterators() { + non_const_iterator np_local0; + non_const_iterator &np_local1 = np_local0; + + for (int np_local2 : np_local1) { + np_local2++; + } + + non_const_iterator np_local3; + for (int p_local0 : np_local3) + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'p_local0' of type 'int' can be declared 'const' + ; + + // Horrible code constructs... + { + non_const_iterator np_local4; + np_local4.data[0]++; + non_const_iterator np_local5; + for (int p_local1 : np_local4, np_local5) + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: variable 'p_local1' of type 'int' can be declared 'const' + ; + + non_const_iterator np_local6; + non_const_iterator np_local7; + for (int p_local2 : 1 > 2 ? np_local6 : np_local7) + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: variable 'p_local2' of type 'int' can be declared 'const' + ; + + non_const_iterator np_local8; + non_const_iterator np_local9; + for (int p_local3 : 2 > 1 ? np_local8 : (np_local8, np_local9)) + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: variable 'p_local3' of type 'int' can be declared 'const' + ; + } +} + +struct good_iterator { + int data[3] = {1, 2, 3}; + + int *begin() { return &data[0]; } + int *end() { return &data[2]; } + const int *begin() const { return &data[0]; } + const int *end() const { return &data[2]; } +}; + +void good_iterators() { + good_iterator p_local0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'good_iterator' can be declared 'const' + good_iterator &p_local1 = p_local0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'good_iterator &' can be declared 'const' + + for (int p_local2 : p_local1) { + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'p_local2' of type 'int' can be declared 'const' + (void)p_local2; + } + + good_iterator p_local3; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local3' of type 'good_iterator' can be declared 'const' + for (int p_local4 : p_local3) + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'p_local4' of type 'int' can be declared 'const' + ; + good_iterator np_local1; + for (int &np_local2 : np_local1) + np_local2++; +} + +void for_bad_iterators_array() { + int np_local0[42]; + int(&np_local1)[42] = np_local0; + + for (int &np_local2 : np_local1) { + np_local2++; + } +} +void for_ok_iterators_array() { + int np_local0[42]; + int(&p_local0)[42] = np_local0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int (&)[42]' can be declared 'const' + + for (int p_local1 : p_local0) { + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'p_local1' of type 'int' can be declared 'const' + (void)p_local1; + } +} + +void take_ref(int &); +void ternary_reference() { + int np_local0 = 42; + int np_local1 = 43; + take_ref((np_local0 > np_local1 ? np_local0 : (np_local0, np_local1))); +} + +void complex_usage() { + int np_local0 = 42; + int p_local0 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const' + int np_local1 = 42; + (np_local0 == p_local0 ? np_local0 : (p_local0, np_local1))++; +} + +template +struct SmallVectorBase { + T data[4]; + void push_back(const T &el) {} + int size() const { return 4; } + T *begin() { return data; } + const T *begin() const { return data; } + T *end() { return data + 4; } + const T *end() const { return data + 4; } +}; + +template +struct SmallVector : SmallVectorBase {}; + +template +void EmitProtocolMethodList(T &&Methods) { + // Note: If the template is uninstantiated the analysis does not figure out, + // that p_local0 could be const. Not sure why, but probably bails because + // some expressions are type-dependant. + SmallVector p_local0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'SmallVector' can be declared 'const' + SmallVector np_local0; + for (const auto *I : Methods) { + if (I == nullptr) + np_local0.push_back(I); + } + p_local0.size(); +} +void instantiate() { + int *p_local0[4] = {nullptr, nullptr, nullptr, nullptr}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int *[4]' can be declared 'const' + EmitProtocolMethodList(p_local0); +} +struct base { + int member; +}; +struct derived : base {}; +struct another_struct { + derived member; +}; +void another_struct_f() { + another_struct np_local0{}; + base &np_local1 = np_local0.member; + np_local1.member++; +} +struct list_init { + int &member; +}; +void create_false_positive() { + int np_local0 = 42; + list_init p_local0 = {np_local0}; + // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'p_local0' of type 'list_init' can be declared 'const' +} +struct list_init_derived { + base &member; +}; +void list_init_derived_func() { + derived np_local0; + list_init_derived p_local0 = {np_local0}; + // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'p_local0' of type 'list_init_derived' can be declared 'const' +} +template +struct ref_pair { + L &first; + R &second; +}; +template +void list_init_template() { + T np_local0{}; + ref_pair p_local0 = {np_local0, np_local0}; +} +void cast_in_class_hierarchy() { + derived np_local0; + base p_local1 = static_cast(np_local0); + // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'p_local1' of type 'base' can be declared 'const' +} + +void function_ref_target(int); +using my_function_type = void (&)(int); +void func_references() { + // Could be const, because the reference is not adjusted but adding that + // has no effect and creates a compiler warning. + my_function_type ptr = function_ref_target; +} + +#if 0 +template +T &return_ref() { + static T global; + return global; +} +template +T *return_ptr() { return &return_ref(); } + +template +void auto_usage_variants() { + // FIXME: Currently all 'auto's that deduce to a reference are not ignored + // for the analysis. That results in bad transformations. + auto auto_val0 = T{}; + auto &auto_val1 = auto_val0; // Bad + auto *auto_val2 = &auto_val0; + + auto auto_ref0 = return_ref(); // Bad + auto &auto_ref1 = return_ref(); // Bad + auto *auto_ref2 = return_ptr(); + + auto auto_ptr0 = return_ptr(); + auto &auto_ptr1 = auto_ptr0; + auto *auto_ptr2 = return_ptr(); + + using MyTypedef = T; + auto auto_td0 = MyTypedef{}; + auto &auto_td1 = auto_td0; // Bad + auto *auto_td2 = &auto_td0; +} +void instantiate_auto_cases() { + auto_usage_variants(); + auto_usage_variants(); +} +#endif diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp --- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp +++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp @@ -6,7 +6,10 @@ // //===----------------------------------------------------------------------===// #include "clang/Analysis/Analyses/ExprMutationAnalyzer.h" +#include "clang/AST/Expr.h" +#include "clang/AST/OperationKinds.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" #include "llvm/ADT/STLExtras.h" namespace clang { @@ -24,11 +27,11 @@ return InnerMatcher.matches(*Range, Finder, Builder); } -AST_MATCHER_P(Expr, maybeEvalCommaExpr, - ast_matchers::internal::Matcher, InnerMatcher) { - const Expr* Result = &Node; +AST_MATCHER_P(Expr, maybeEvalCommaExpr, ast_matchers::internal::Matcher, + InnerMatcher) { + const Expr *Result = &Node; while (const auto *BOComma = - dyn_cast_or_null(Result->IgnoreParens())) { + dyn_cast_or_null(Result->IgnoreParens())) { if (!BOComma->isCommaOp()) break; Result = BOComma->getRHS(); @@ -36,6 +39,41 @@ return InnerMatcher.matches(*Result, Finder, Builder); } +AST_MATCHER_P(Expr, canResolveToExpr, ast_matchers::internal::Matcher, + InnerMatcher) { + // Unless the value is a derived class and is assigned to a + // reference to the base class. Other implicit casts should not + // happen. + const auto IgnoreDerivedToBase = ignoringParens( + expr(anyOf(implicitCastExpr(anyOf(hasCastKind(CK_DerivedToBase), + hasCastKind(CK_UncheckedDerivedToBase)), + hasSourceExpression(InnerMatcher)), + InnerMatcher))); + + auto const ComplexMatcher = ignoringParens( + expr(anyOf(maybeEvalCommaExpr(IgnoreDerivedToBase), + conditionalOperator( + anyOf(hasTrueExpression(ignoringParens( + maybeEvalCommaExpr(IgnoreDerivedToBase))), + hasFalseExpression(ignoringParens( + maybeEvalCommaExpr(IgnoreDerivedToBase)))))))); + return ComplexMatcher.matches(Node, Finder, Builder); +} + +// Similar to 'hasAnyArgument', but does not work because 'InitListExpr' does +// not have the 'arguments()' method. +AST_MATCHER_P(InitListExpr, hasAnyArgumentExpr, + ast_matchers::internal::Matcher, InnerMatcher) { + for (const Expr *Arg : Node.inits()) { + ast_matchers::internal::BoundNodesTreeBuilder Result(*Builder); + if (InnerMatcher.matches(*Arg, Finder, &Result)) { + *Builder = std::move(Result); + return true; + } + } + return false; +} + const ast_matchers::internal::VariadicDynCastAllOfMatcher cxxTypeidExpr; @@ -151,7 +189,7 @@ NodeID::value, match( findAll( - expr(equalsNode(Exp), + expr(canResolveToExpr(equalsNode(Exp)), anyOf( // `Exp` is part of the underlying expression of // decltype/typeof if it has an ancestor of @@ -202,29 +240,41 @@ const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) { // LHS of any assignment operators. const auto AsAssignmentLhs = binaryOperator( - isAssignmentOperator(), - hasLHS(maybeEvalCommaExpr(ignoringParenImpCasts(equalsNode(Exp))))); + isAssignmentOperator(), hasLHS(canResolveToExpr(equalsNode(Exp)))); // Operand of increment/decrement operators. const auto AsIncDecOperand = unaryOperator(anyOf(hasOperatorName("++"), hasOperatorName("--")), - hasUnaryOperand(maybeEvalCommaExpr( - ignoringParenImpCasts(equalsNode(Exp))))); + hasUnaryOperand(canResolveToExpr(equalsNode(Exp)))); + const auto NotInstantiated = unless(hasDeclaration(isInstantiated())); // Invoking non-const member function. // A member function is assumed to be non-const when it is unresolved. const auto NonConstMethod = cxxMethodDecl(unless(isConst())); - const auto AsNonConstThis = - expr(anyOf(cxxMemberCallExpr(callee(NonConstMethod), - on(maybeEvalCommaExpr(equalsNode(Exp)))), - cxxOperatorCallExpr(callee(NonConstMethod), - hasArgument(0, - maybeEvalCommaExpr(equalsNode(Exp)))), - callExpr(callee(expr(anyOf( - unresolvedMemberExpr( - hasObjectExpression(maybeEvalCommaExpr(equalsNode(Exp)))), - cxxDependentScopeMemberExpr( - hasObjectExpression(maybeEvalCommaExpr(equalsNode(Exp)))))))))); + + const auto AsNonConstThis = expr(anyOf( + cxxMemberCallExpr( + callee(NonConstMethod), + on(ignoringImpCasts(canResolveToExpr(equalsNode(Exp))))), + cxxOperatorCallExpr( + callee(NonConstMethod), + hasArgument(0, ignoringImpCasts(canResolveToExpr(equalsNode(Exp))))), + // operator call expression might be unresolved as well. If that is + // the case and the operator is called on the 'Exp' itself, this is + // considered a moditication. + cxxOperatorCallExpr( + callee(expr(anyOf(unresolvedLookupExpr(), unresolvedMemberExpr(), + cxxDependentScopeMemberExpr(), + hasType(templateTypeParmType())))), + hasArgument(0, canResolveToExpr(equalsNode(Exp)))), + callExpr(callee(expr( + anyOf(unresolvedMemberExpr(hasObjectExpression( + ignoringImpCasts(canResolveToExpr(equalsNode(Exp))))), + cxxDependentScopeMemberExpr(hasObjectExpression( + ignoringImpCasts(canResolveToExpr(equalsNode(Exp))))))))), + callExpr(allOf(isTypeDependent(), + callee(memberExpr( + hasDeclaration(cxxMethodDecl(unless(isConst()))))))))); // Taking address of 'Exp'. // We're assuming 'Exp' is mutated as soon as its address is taken, though in @@ -234,38 +284,48 @@ unaryOperator(hasOperatorName("&"), // A NoOp implicit cast is adding const. unless(hasParent(implicitCastExpr(hasCastKind(CK_NoOp)))), - hasUnaryOperand(maybeEvalCommaExpr(equalsNode(Exp)))); + hasUnaryOperand(canResolveToExpr(equalsNode(Exp)))); const auto AsPointerFromArrayDecay = castExpr(hasCastKind(CK_ArrayToPointerDecay), unless(hasParent(arraySubscriptExpr())), - has(maybeEvalCommaExpr(equalsNode(Exp)))); + has(canResolveToExpr(equalsNode(Exp)))); // Treat calling `operator->()` of move-only classes as taking address. // These are typically smart pointers with unique ownership so we treat // mutation of pointee as mutation of the smart pointer itself. - const auto AsOperatorArrowThis = - cxxOperatorCallExpr(hasOverloadedOperatorName("->"), - callee(cxxMethodDecl(ofClass(isMoveOnly()), - returns(nonConstPointerType()))), - argumentCountIs(1), - hasArgument(0, maybeEvalCommaExpr(equalsNode(Exp)))); + const auto AsOperatorArrowThis = cxxOperatorCallExpr( + hasOverloadedOperatorName("->"), + callee( + cxxMethodDecl(ofClass(isMoveOnly()), returns(nonConstPointerType()))), + argumentCountIs(1), hasArgument(0, canResolveToExpr(equalsNode(Exp)))); // Used as non-const-ref argument when calling a function. // An argument is assumed to be non-const-ref when the function is unresolved. // Instantiated template functions are not handled here but in // findFunctionArgMutation which has additional smarts for handling forwarding // references. - const auto NonConstRefParam = forEachArgumentWithParam( - maybeEvalCommaExpr(equalsNode(Exp)), - parmVarDecl(hasType(nonConstReferenceType()))); - const auto NotInstantiated = unless(hasDeclaration(isInstantiated())); + const auto NonConstRefParam = forEachArgumentWithParamType( + anyOf(canResolveToExpr(equalsNode(Exp)), + memberExpr(hasObjectExpression(canResolveToExpr(equalsNode(Exp))))), + nonConstReferenceType()); const auto AsNonConstRefArg = anyOf( callExpr(NonConstRefParam, NotInstantiated), cxxConstructExpr(NonConstRefParam, NotInstantiated), callExpr(callee(expr(anyOf(unresolvedLookupExpr(), unresolvedMemberExpr(), cxxDependentScopeMemberExpr(), hasType(templateTypeParmType())))), - hasAnyArgument(maybeEvalCommaExpr(equalsNode(Exp)))), - cxxUnresolvedConstructExpr(hasAnyArgument(maybeEvalCommaExpr(equalsNode(Exp))))); + hasAnyArgument(canResolveToExpr(equalsNode(Exp)))), + cxxUnresolvedConstructExpr( + hasAnyArgument(canResolveToExpr(equalsNode(Exp)))), + // Previous False Positive in the following Code: + // `template void f() { int i = 42; new Type(i); }` + // Where the constructor of `Type` takes its argument as reference. + // The AST does not resolve in a `cxxConstructExpr` because it is + // type-dependent. + parenListExpr(hasDescendant(expr(canResolveToExpr(equalsNode(Exp))))), + // If the initializer is for a reference type, there is no cast for + // the variable. Values are casted to RValue first. + initListExpr( + hasAnyArgumentExpr(expr(canResolveToExpr(equalsNode(Exp)))))); // Captured by a lambda by reference. // If we're initializing a capture with 'Exp' directly then we're initializing @@ -279,7 +339,15 @@ // For returning by const-ref there will be an ImplicitCastExpr (for // adding const.) const auto AsNonConstRefReturn = returnStmt(hasReturnValue( - maybeEvalCommaExpr(equalsNode(Exp)))); + anyOf(canResolveToExpr(equalsNode(Exp)), + castExpr(allOf( + hasCastKind(CK_DerivedToBase), + hasSourceExpression(canResolveToExpr(equalsNode(Exp)))))))); + + // It is used as a non-const-reference for initalizing a range-for loop. + const auto AsNonConstRefRangeInit = cxxForRangeStmt( + hasRangeInit(declRefExpr(allOf(canResolveToExpr(equalsNode(Exp)), + hasType(nonConstReferenceType()))))); const auto Matches = match( traverse( @@ -296,9 +364,10 @@ const Stmt *ExprMutationAnalyzer::findMemberMutation(const Expr *Exp) { // Check whether any member of 'Exp' is mutated. const auto MemberExprs = - match(findAll(expr(anyOf(memberExpr(hasObjectExpression(equalsNode(Exp))), - cxxDependentScopeMemberExpr( - hasObjectExpression(equalsNode(Exp))))) + match(findAll(expr(anyOf(memberExpr(hasObjectExpression( + canResolveToExpr(equalsNode(Exp)))), + cxxDependentScopeMemberExpr(hasObjectExpression( + canResolveToExpr(equalsNode(Exp)))))) .bind(NodeID::value)), Stm, Context); return findExprMutation(MemberExprs); @@ -306,43 +375,110 @@ const Stmt *ExprMutationAnalyzer::findArrayElementMutation(const Expr *Exp) { // Check whether any element of an array is mutated. - const auto SubscriptExprs = match( - findAll(arraySubscriptExpr(hasBase(ignoringImpCasts(equalsNode(Exp)))) - .bind(NodeID::value)), - Stm, Context); + const auto SubscriptExprs = + match(findAll(arraySubscriptExpr( + anyOf(hasBase(canResolveToExpr(equalsNode(Exp))), + hasBase(implicitCastExpr( + allOf(hasCastKind(CK_ArrayToPointerDecay), + hasSourceExpression(canResolveToExpr( + equalsNode(Exp)))))))) + .bind(NodeID::value)), + Stm, Context); return findExprMutation(SubscriptExprs); } const Stmt *ExprMutationAnalyzer::findCastMutation(const Expr *Exp) { + // If the 'Exp' is explicitly casted to a non-const reference type the + // 'Exp' is considered to be modified. + const auto ExplicitCast = match( + findAll( + stmt(castExpr(hasSourceExpression(canResolveToExpr(equalsNode(Exp))), + explicitCastExpr( + hasDestinationType(nonConstReferenceType())))) + .bind("stmt")), + Stm, Context); + + if (const auto *CastStmt = selectFirst("stmt", ExplicitCast)) + return CastStmt; + // If 'Exp' is casted to any non-const reference type, check the castExpr. - const auto Casts = - match(findAll(castExpr(hasSourceExpression(equalsNode(Exp)), - anyOf(explicitCastExpr(hasDestinationType( - nonConstReferenceType())), - implicitCastExpr(hasImplicitDestinationType( - nonConstReferenceType())))) - .bind(NodeID::value)), - Stm, Context); + const auto Casts = match( + findAll( + expr(castExpr(hasSourceExpression(canResolveToExpr(equalsNode(Exp))), + anyOf(explicitCastExpr( + hasDestinationType(nonConstReferenceType())), + implicitCastExpr(hasImplicitDestinationType( + nonConstReferenceType()))))) + .bind(NodeID::value)), + Stm, Context); + if (const Stmt *S = findExprMutation(Casts)) return S; // Treat std::{move,forward} as cast. const auto Calls = match(findAll(callExpr(callee(namedDecl( hasAnyName("::std::move", "::std::forward"))), - hasArgument(0, equalsNode(Exp))) + hasArgument(0, canResolveToExpr(equalsNode(Exp)))) .bind("expr")), Stm, Context); return findExprMutation(Calls); } const Stmt *ExprMutationAnalyzer::findRangeLoopMutation(const Expr *Exp) { + // Keep the ordering for the specific initialization matches to happen first, + // because it is cheaper to match then all potential modifications of the + // loop variable. + + // The range variable is a reference to a builtin array. In that case the + // array is considered modified if the loop-variable is a non-const reference. + const auto DeclStmtToNonRefToArray = declStmt(hasSingleDecl(varDecl(hasType( + hasUnqualifiedDesugaredType(referenceType(pointee(arrayType()))))))); + const auto RefToArrayRefToElements = match( + findAll(stmt(cxxForRangeStmt( + hasLoopVariable(varDecl(hasType(nonConstReferenceType())) + .bind(NodeID::value)), + hasRangeStmt(DeclStmtToNonRefToArray), + hasRangeInit(canResolveToExpr(equalsNode(Exp))))) + .bind("stmt")), + Stm, Context); + + if (const auto *BadRangeInitFromArray = + selectFirst("stmt", RefToArrayRefToElements)) + return BadRangeInitFromArray; + + // It is possible, that containers do not provide a const-overload for their + // iterator accessors. If this is the case, the variable is used non-const + // no matter what happens in the loop. This requires special detection as it + // is faster to find then all mutations of the loop variable. + // It aims at a different modification as well. + const auto HasAnyNonConstIterator = + anyOf(allOf(hasMethod(allOf(hasName("begin"), unless(isConst()))), + unless(hasMethod(allOf(hasName("begin"), isConst())))), + allOf(hasMethod(allOf(hasName("end"), unless(isConst()))), + unless(hasMethod(allOf(hasName("end"), isConst()))))); + + const auto DeclStmtToNonConstIteratorContainer = declStmt( + hasSingleDecl(varDecl(hasType(hasUnqualifiedDesugaredType(referenceType( + pointee(hasDeclaration(cxxRecordDecl(HasAnyNonConstIterator))))))))); + + const auto RefToContainerBadIterators = + match(findAll(stmt(cxxForRangeStmt(allOf( + hasRangeStmt(DeclStmtToNonConstIteratorContainer), + hasRangeInit(canResolveToExpr(equalsNode(Exp)))))) + .bind("stmt")), + Stm, Context); + + if (const auto *BadIteratorsContainer = + selectFirst("stmt", RefToContainerBadIterators)) + return BadIteratorsContainer; + // If range for looping over 'Exp' with a non-const reference loop variable, // check all declRefExpr of the loop variable. const auto LoopVars = match(findAll(cxxForRangeStmt( hasLoopVariable(varDecl(hasType(nonConstReferenceType())) .bind(NodeID::value)), - hasRangeInit(equalsNode(Exp)))), + hasRangeInit(canResolveToExpr(equalsNode(Exp))))), Stm, Context); return findDeclMutation(LoopVars); } @@ -356,7 +492,8 @@ hasOverloadedOperatorName("*"), callee(cxxMethodDecl(ofClass(isMoveOnly()), returns(nonConstReferenceType()))), - argumentCountIs(1), hasArgument(0, equalsNode(Exp))) + argumentCountIs(1), + hasArgument(0, canResolveToExpr(equalsNode(Exp)))) .bind(NodeID::value)), Stm, Context); if (const Stmt *S = findExprMutation(Ref)) @@ -367,13 +504,12 @@ stmt(forEachDescendant( varDecl( hasType(nonConstReferenceType()), - hasInitializer(anyOf(equalsNode(Exp), - conditionalOperator(anyOf( - hasTrueExpression(equalsNode(Exp)), - hasFalseExpression(equalsNode(Exp)))))), + hasInitializer(anyOf(canResolveToExpr(equalsNode(Exp)), + memberExpr(hasObjectExpression( + canResolveToExpr(equalsNode(Exp)))))), hasParent(declStmt().bind("stmt")), - // Don't follow the reference in range statement, we've handled - // that separately. + // Don't follow the reference in range statement, we've + // handled that separately. unless(hasParent(declStmt(hasParent( cxxForRangeStmt(hasRangeStmt(equalsBoundNode("stmt")))))))) .bind(NodeID::value))), @@ -383,7 +519,7 @@ const Stmt *ExprMutationAnalyzer::findFunctionArgMutation(const Expr *Exp) { const auto NonConstRefParam = forEachArgumentWithParam( - equalsNode(Exp), + canResolveToExpr(equalsNode(Exp)), parmVarDecl(hasType(nonConstReferenceType())).bind("parm")); const auto IsInstantiated = hasDeclaration(isInstantiated()); const auto FuncDecl = hasDeclaration(functionDecl().bind("func")); diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp --- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp +++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp @@ -19,9 +19,7 @@ using namespace clang::ast_matchers; using ::testing::ElementsAre; -using ::testing::IsEmpty; using ::testing::ResultOf; -using ::testing::StartsWith; using ::testing::Values; namespace { @@ -63,13 +61,18 @@ const auto *const S = selectFirst("stmt", Results); SmallVector Chain; ExprMutationAnalyzer Analyzer(*S, AST->getASTContext()); + + std::string buffer; for (const auto *E = selectFirst("expr", Results); E != nullptr;) { const Stmt *By = Analyzer.findMutation(E); - std::string buffer; + if (!By) + break; + llvm::raw_string_ostream stream(buffer); By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy()); - Chain.push_back(StringRef(stream.str()).trim().str()); + Chain.emplace_back(StringRef(stream.str()).trim().str()); E = dyn_cast(By); + buffer.clear(); } return Chain; } @@ -394,22 +397,26 @@ "void g(const int&&); void f() { int x; g(static_cast(x)); }"); auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); - EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_THAT(mutatedBy(Results, AST.get()), + ElementsAre("static_cast(x)")); AST = buildASTFromCode("struct A {}; A operator+(const A&&, int);" "void f() { A x; static_cast(x) + 1; }"); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); - EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_THAT(mutatedBy(Results, AST.get()), + ElementsAre("static_cast(x)")); AST = buildASTFromCode("void f() { struct A { A(const int&&); }; " "int x; A y(static_cast(x)); }"); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); - EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_THAT(mutatedBy(Results, AST.get()), + ElementsAre("static_cast(x)")); AST = buildASTFromCode("void f() { struct A { A(); A(const A&&); }; " "A x; A y(static_cast(x)); }"); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); - EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_THAT(mutatedBy(Results, AST.get()), + ElementsAre("static_cast(x)")); } TEST(ExprMutationAnalyzerTest, Move) { @@ -579,7 +586,7 @@ const auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_THAT(mutatedBy(Results, AST.get()), - ElementsAre("return static_cast(x);")); + ElementsAre("static_cast(x)")); } TEST(ExprMutationAnalyzerTest, ReturnAsConstRRef) { @@ -587,7 +594,8 @@ "const int&& f() { int x; return static_cast(x); }"); const auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); - EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_THAT(mutatedBy(Results, AST.get()), + ElementsAre("static_cast(x)")); } TEST(ExprMutationAnalyzerTest, TakeAddress) { @@ -863,13 +871,13 @@ auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_THAT(mutatedBy(Results, AST.get()), - ElementsAre("static_cast(x) = 10")); + ElementsAre("static_cast(x)")); AST = buildASTFromCode("typedef int& IntRef;" "void f() { int x; static_cast(x) = 10; }"); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_THAT(mutatedBy(Results, AST.get()), - ElementsAre("static_cast(x) = 10")); + ElementsAre("static_cast(x)")); } TEST(ExprMutationAnalyzerTest, CastToRefNotModified) { @@ -877,7 +885,8 @@ buildASTFromCode("void f() { int x; static_cast(x); }"); const auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); - EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_THAT(mutatedBy(Results, AST.get()), + ElementsAre("static_cast(x)")); } TEST(ExprMutationAnalyzerTest, CastToConstRef) { @@ -1054,20 +1063,22 @@ buildASTFromCode("void f() { int x[2]; for (int& e : x) e = 10; }"); auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); - EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("e", "e = 10")); + EXPECT_THAT(mutatedBy(Results, AST.get()), + ElementsAre("for (int &e : x)\n e = 10;")); AST = buildASTFromCode("typedef int& IntRef;" "void f() { int x[2]; for (IntRef e : x) e = 10; }"); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); - EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("e", "e = 10")); + EXPECT_THAT(mutatedBy(Results, AST.get()), + ElementsAre("for (IntRef e : x)\n e = 10;")); } -TEST(ExprMutationAnalyzerTest, RangeForArrayByRefNotModified) { +TEST(ExprMutationAnalyzerTest, RangeForArrayByRefModifiedByImplicitInit) { const auto AST = buildASTFromCode("void f() { int x[2]; for (int& e : x) e; }"); const auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); - EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_TRUE(isMutated(Results, AST.get())); } TEST(ExprMutationAnalyzerTest, RangeForArrayByValue) { @@ -1107,7 +1118,8 @@ "void f() { V x; for (int& e : x) e = 10; }"); const auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); - EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("e", "e = 10")); + EXPECT_THAT(mutatedBy(Results, AST.get()), + ElementsAre("for (int &e : x)\n e = 10;")); } TEST(ExprMutationAnalyzerTest, RangeForNonArrayByRefNotModified) { @@ -1115,7 +1127,7 @@ "void f() { V x; for (int& e : x) e; }"); const auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); - EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_TRUE(isMutated(Results, AST.get())); } TEST(ExprMutationAnalyzerTest, RangeForNonArrayByValue) { @@ -1246,6 +1258,22 @@ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x->mf()")); } +TEST(ExprMutationAnalyzerTest, UnevaluatedContext) { + const std::string Example = + "template " + "struct to_construct : T { to_construct(int &j) {} };" + "template " + "void placement_new_in_unique_ptr() { int x = 0;" + " new to_construct(x);" + "}"; + auto AST = + buildASTFromCodeWithArgs(Example, {"-fno-delayed-template-parsing"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isMutated(Results, AST.get())); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("(x)")); +} + TEST(ExprMutationAnalyzerTest, ReproduceFailureMinimal) { const std::string Reproducer = "namespace std {"