diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt @@ -22,6 +22,7 @@ add_custom_target(genconfusable DEPENDS Confusables.inc) add_clang_library(clangTidyMiscModule + ConstCorrectnessCheck.cpp DefinitionsInHeadersCheck.cpp ConfusableIdentifierCheck.cpp MiscTidyModule.cpp @@ -42,6 +43,7 @@ UnusedUsingDeclsCheck.cpp LINK_LIBS + clangAnalysis clangTidy clangTidyUtils diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h @@ -0,0 +1,57 @@ +//===--- 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_MISC_CONSTCORRECTNESSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_CONSTCORRECTNESSCHECK_H + +#include "../ClangTidyCheck.h" +#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h" +#include "llvm/ADT/DenseSet.h" + +namespace clang { +namespace tidy { + +namespace misc { + +/// 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/misc-const-correctness.html +class ConstCorrectnessCheck : public ClangTidyCheck { +public: + ConstCorrectnessCheck(StringRef Name, ClangTidyContext *Context); + + // The rules for C and 'const' are different and incompatible for this check. + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } + 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; + llvm::DenseSet TemplateDiagnosticsCache; + + const bool AnalyzeValues; + const bool AnalyzeReferences; + const bool WarnPointersAsValues; + + const bool TransformValues; + const bool TransformReferences; + const bool TransformPointersAsValues; +}; + +} // namespace misc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_CONSTCORRECTNESSCHECK_H diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp @@ -0,0 +1,208 @@ +//===--- 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" + +#include + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace misc { + +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, containsAnyDeclaration, + ast_matchers::internal::Matcher, InnerMatcher) { + return ast_matchers::internal::matchesFirstInPointerRange( + InnerMatcher, Node.decl_begin(), Node.decl_end(), Finder, + Builder) != Node.decl_end(); +} +AST_MATCHER(ReferenceType, isSpelledAsLValue) { + return Node.isSpelledAsLValue(); +} +AST_MATCHER(Type, isDependentType) { return Node.isDependentType(); } +} // namespace + +ConstCorrectnessCheck::ConstCorrectnessCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + AnalyzeValues(Options.get("AnalyzeValues", true)), + AnalyzeReferences(Options.get("AnalyzeReferences", true)), + WarnPointersAsValues(Options.get("WarnPointersAsValues", false)), + TransformValues(Options.get("TransformValues", true)), + TransformReferences(Options.get("TransformReferences", true)), + TransformPointersAsValues( + Options.get("TransformPointersAsValues", false)) { + if (AnalyzeValues == false && AnalyzeReferences == false) + this->configurationDiag( + "The check 'misc-const-correctness' will not " + "perform any analysis because both 'AnalyzeValues' and " + "'AnalyzeReferences' are false."); +} + +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()))))); + + 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, + hasInitializer(isInstantiationDependent()), + 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(forEachDescendant( + declStmt(containsAnyDeclaration( + LocalValDecl.bind("local-value")), + unless(has(decompositionDecl()))) + .bind("decl-stmt"))) + .bind("scope"))) + .bind("function-decl"); + + 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"); + const auto *Variable = Result.Nodes.getNodeAs("local-value"); + const auto *Function = Result.Nodes.getNodeAs("function-decl"); + + /// If the variable was declared in a template it might be analyzed multiple + /// times. Only one of those instantiations shall emit a warning. NOTE: This + /// shall only deduplicate warnings for variables that are not instantiation + /// dependent. Variables like 'int x = 42;' in a template that can become + /// const emit multiple warnings otherwise. + bool IsNormalVariableInTemplate = Function->isTemplateInstantiation(); + if (IsNormalVariableInTemplate && + TemplateDiagnosticsCache.contains(Variable->getBeginLoc())) + return; + + VariableCategory VC = VariableCategory::Value; + if (Variable->getType()->isReferenceType()) + VC = VariableCategory::Reference; + if (Variable->getType()->isPointerType()) + VC = VariableCategory::Pointer; + + // Each variable can only be 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; + + // 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; + + auto Diag = diag(Variable->getBeginLoc(), + "variable %0 of type %1 can be declared 'const'") + << Variable << Variable->getType(); + if (IsNormalVariableInTemplate) + TemplateDiagnosticsCache.insert(Variable->getBeginLoc()); + + 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; + if (VC == VariableCategory::Value && TransformValues) { + Diag << addQualifierToVarDecl(*Variable, *Result.Context, + DeclSpec::TQ_const, QualifierTarget::Value, + QualifierPolicy::Right); + // FIXME: Add '{}' for default initialization if no user-defined default + // constructor exists and there is no initializer. + return; + } + + if (VC == VariableCategory::Reference && TransformReferences) { + Diag << addQualifierToVarDecl(*Variable, *Result.Context, + DeclSpec::TQ_const, QualifierTarget::Value, + QualifierPolicy::Right); + return; + } + + if (VC == VariableCategory::Pointer) { + if (WarnPointersAsValues && TransformPointersAsValues) { + Diag << addQualifierToVarDecl(*Variable, *Result.Context, + DeclSpec::TQ_const, QualifierTarget::Value, + QualifierPolicy::Right); + } + return; + } +} + +void ConstCorrectnessCheck::registerScope(const CompoundStmt *LocalScope, + ASTContext *Context) { + auto &Analyzer = ScopesCache[LocalScope]; + if (!Analyzer) + Analyzer = std::make_unique(*LocalScope, *Context); +} + +} // namespace misc +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp --- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" #include "ConfusableIdentifierCheck.h" +#include "ConstCorrectnessCheck.h" #include "DefinitionsInHeadersCheck.h" #include "MisleadingBidirectional.h" #include "MisleadingIdentifier.h" @@ -36,6 +37,8 @@ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck( "misc-confusable-identifiers"); + CheckFactories.registerCheck( + "misc-const-correctness"); CheckFactories.registerCheck( "misc-definitions-in-headers"); 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 @@ -137,6 +137,11 @@ Warns when there is an assignment within an if statement condition expression. +- New :doc:`misc-const-correctness + ` check. + + Detects unmodified local variables and suggest adding ``const`` if the transformation is possible. + - New :doc:`modernize-macro-to-enum ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -239,6 +239,7 @@ `llvmlibc-implementation-in-namespace `_, `llvmlibc-restrict-system-libc-headers `_, "Yes" `misc-confusable-identifiers `_, + `misc-const-correctness `_, "Yes" `misc-definitions-in-headers `_, "Yes" `misc-misleading-bidirectional `_, `misc-misleading-identifier `_, diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst @@ -0,0 +1,150 @@ +.. title:: clang-tidy - misc-const-correctness + +misc-const-correctness +====================== + +This check implements detection of local variables which could be declared as +``const``, but are not. Declaring variables as ``const`` is required or recommended by many +coding guidelines, such as: +`CppCoreGuidelines ES.25 `_ +and `AUTOSAR C++14 Rule A7-1-1 (6.7.1 Specifiers) `_. + +Please note that this analysis is type-based only. Variables that are not modified +but used to create a non-const handle that might escape 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 can analyzes values, pointers and references but not (yet) pointees: + +.. code-block:: c++ + + // Normal values like built-ins or objects. + int potential_const_int = 42; // 'const int potential_const_int = 42' suggestion. + int copy_of_value = potential_const_int; + + MyClass could_be_const; // 'const MyClass could_be_const' suggestion; + could_be_const.const_qualified_method(); + + // References can be declared const as well. + int &reference_value = potential_const_int; // 'const int &reference_value' suggestion. + int another_copy = reference_value; + + // The similar semantics of pointers are not (yet) analyzed. + int *pointer_variable = &potential_const_int; // Not 'const int *pointer_variable' suggestion. + int last_copy = *pointer_variable; + +The automatic code transformation is only applied to variables that are declared in single +declarations. You may want to prepare your code base with +`readability-isolate-declaration `_ first. + +Note that there is the check +`cppcoreguidelines-avoid-non-const-global-variables `_ +to enforce ``const`` correctness on all globals. + +Known Limitations +----------------- + +The check will not analyze templated variables or variables that are instantiation dependent. +Different instantiations can result in different ``const`` correctness properties and in general it +is not possible to find all instantiations of a template. It might be used differently in an +independent translation unit. + +Pointees can not be analyzed for constness yet. The following code is shows this limitation. + +.. code-block:: c++ + + // Declare a variable that will not be modified. + int constant_value = 42; + + // Declare a pointer to that variable, that does not modify either, but misses 'const'. + // Could be 'const int *pointer_to_constant = &constant_value;' + int *pointer_to_constant = &constant_value; + + // Usage: + int result = 520 * 120 * (*pointer_to_constant); + +This limitation affects the capability to add ``const`` to methods which is not possible, too. + +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 + 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; + +.. option:: TransformValues (default = 1) + + Provides fixit-hints for value types that automatically adds ``const`` if its a single declaration. + + .. code-block:: c++ + + // Emits a hint for 'value' to become 'const int value = 42;'. + int value = 42; + // Result is modified later in its life-time. No diagnostic and fixit hint will be emitted. + int result = value * 3; + result -= 10; + +.. option:: TransformReferences (default = 1) + + Provides fixit-hints for reference types that automatically adds ``const`` if its a single + declaration. + + .. code-block:: c++ + + // This variable could still be a constant. But because there is a non-const reference to + // it, it can not be transformed (yet). + int value = 42; + // The reference 'ref_value' is not modified and can be made 'const int &ref_value = value;' + int &ref_value = value; + + // Result is modified later in its life-time. No diagnostic and fixit hint will be emitted. + int result = ref_value * 3; + result -= 10; + +.. option:: TransformPointersAsValues (default = 0) + + Provides fixit-hints for pointers if their pointee is not changed. This does not analyze if the + value-pointed-to is unchanged! + + Requires 'WarnPointersAsValues' to be 1. + + .. code-block:: c++ + + int value = 42; + // Emits a hint that 'ptr_value' may become 'int *const ptr_value = &value' because its pointee + // is not changed. + int *ptr_value = &value; + + int result = 100 * (*ptr_value); + // This modification of the pointee is still allowed and not analyzed/diagnosed. + *ptr_value = 0; + + // The following pointer may not become a 'int *const'. + int *changing_pointee = &value; + changing_pointee = &result; + diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-cxx17.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-cxx17.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-cxx17.cpp @@ -0,0 +1,55 @@ +// RUN: %check_clang_tidy %s misc-const-correctness %t -- -- -std=c++17 -fno-delayed-template-parsing + +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++; + // CHECK-FIXES-NOT: auto const [np_local0, np_local1] + + auto [np_local2, p_local0] = MyPair(42., 42.); + np_local2++; + // CHECK-FIXES-NOT: auto const [np_local2, p_local0] + + auto [p_local1, np_local3] = MyPair(42., 42.); + np_local3++; + // CHECK-FIXES-NOT: auto const [p_local1, np_local3] + + auto [p_local2, p_local3] = MyPair(42., 42.); + // CHECK-FIXES-NOT: auto const [p_local2, p_local3] +} + +void g() { + int p_local0 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const' + // CHECK-FIXES: int const p_local0 = 42; +} + +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; + // CHECK-FIXES-NOT: HardWorker const m_theOtherInstance +}; diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp @@ -0,0 +1,13 @@ +// RUN: %check_clang_tidy %s misc-const-correctness %t \ +// RUN: -config='{CheckOptions: \ +// RUN: [{key: "misc-const-correctness.AnalyzeValues", value: true},\ +// RUN: {key: "misc-const-correctness.WarnPointersAsValues", value: true},\ +// RUN: {key: "misc-const-correctness.TransformPointersAsValues", value: true}]}' \ +// RUN: -- -fno-delayed-template-parsing + +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: double *const p_local0 +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp @@ -0,0 +1,22 @@ +// RUN: %check_clang_tidy %s misc-const-correctness %t -- \ +// RUN: -config="{CheckOptions: [\ +// RUN: {key: 'misc-const-correctness.TransformValues', value: true}, \ +// RUN: {key: 'misc-const-correctness.TransformReferences', value: true}, \ +// RUN: {key: 'misc-const-correctness.WarnPointersAsValues', value: false}, \ +// RUN: {key: 'misc-const-correctness.TransformPointersAsValues', value: false}, \ +// RUN: ]}" -- -fno-delayed-template-parsing + +template +void type_dependent_variables() { + T value = 42; + auto &ref = value; + T &templateRef = value; + + int value_int = 42; + // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'value_int' of type 'int' can be declared 'const' + // CHECK-FIXES: int const value_int +} +void instantiate_template_cases() { + type_dependent_variables(); + type_dependent_variables(); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-transform-pointer-as-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-transform-pointer-as-values.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-transform-pointer-as-values.cpp @@ -0,0 +1,13 @@ +// RUN: %check_clang_tidy %s misc-const-correctness %t \ +// RUN: -config='{CheckOptions: \ +// RUN: [{key: "misc-const-correctness.AnalyzeValues", value: true},\ +// RUN: {key: "misc-const-correctness.WarnPointersAsValues", value: true}, \ +// RUN: {key: "misc-const-correctness.TransformPointersAsValues", value: true},\ +// RUN: ]}' -- -fno-delayed-template-parsing + +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: double *const p_local0 = &np_local0[1]; +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-transform-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-transform-values.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-transform-values.cpp @@ -0,0 +1,175 @@ +// RUN: %check_clang_tidy %s misc-const-correctness %t -- \ +// RUN: -config="{CheckOptions: [\ +// RUN: {key: 'misc-const-correctness.TransformValues', value: true},\ +// RUN: {key: 'misc-const-correctness.WarnPointersAsValues', value: false}, \ +// RUN: {key: 'misc-const-correctness.TransformPointersAsValues', value: false}, \ +// RUN: ]}" -- -fno-delayed-template-parsing + +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: int const p_local0 = 2; +} + +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: int const p_local1 = 42; + } +} + +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: int const p_local1 = 42; +} + +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: ConstNonConstClass const p_local0; + 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: ConstNonConstClass const p_local0[2]; + 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: double const p_local0[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.}; +} + +void range_for() { + int np_local0[2] = {1, 2}; + // The transformation is not possible because the range-for-loop mutates the array content. + int *const np_local1[2] = {&np_local0[0], &np_local0[1]}; + for (int *non_const_ptr : np_local1) { + *non_const_ptr = 45; + } + + int *np_local2[2] = {&np_local0[0], &np_local0[1]}; + for (int *non_const_ptr : np_local2) { + *non_const_ptr = 45; + } +} + +void decltype_declaration() { + decltype(sizeof(void *)) p_local0 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'decltype(sizeof(void *))' + // CHECK-FIXES: decltype(sizeof(void *)) const p_local0 = 42; +} + +// Taken from libcxx/include/type_traits and improved readability. +template +struct integral_constant { + static constexpr const Tp value = v; + using value_type = Tp; + using type = integral_constant; + constexpr operator value_type() const noexcept { return value; } + constexpr value_type operator()() const noexcept { return value; } +}; + +template +struct is_integral : integral_constant {}; +template <> +struct is_integral : integral_constant {}; + +template +struct not_integral : integral_constant {}; +template <> +struct not_integral : integral_constant {}; + +template +struct enable_if {}; + +template +struct enable_if { using type = Tp; }; + +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: TMPClass const p_local0; + 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: TMPClass const p_local1; + p_local1.alwaysConst(); + + TMPClass p_local2; // Don't attempt to make this const + p_local2.sometimesConst(); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-unaligned.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-unaligned.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-unaligned.cpp @@ -0,0 +1,19 @@ +// RUN: %check_clang_tidy %s misc-const-correctness %t -- \ +// RUN: -config="{CheckOptions: [\ +// RUN: {key: 'misc-const-correctness.TransformValues', value: true}, \ +// RUN: {key: 'misc-const-correctness.WarnPointersAsValues', value: false}, \ +// RUN: {key: 'misc-const-correctness.TransformPointersAsValues', value: false}, \ +// RUN: ]}" -- -fno-delayed-template-parsing -fms-extensions + +struct S {}; + +void f(__unaligned S *); + +void scope() { + // FIXME: This is a bug in the analysis, that is confused by '__unaligned'. + // https://bugs.llvm.org/show_bug.cgi?id=51756 + S s; + // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 's' of type 'S' can be declared 'const' + // CHECK-FIXES: S const s; + f(&s); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp @@ -0,0 +1,1030 @@ +// RUN: %check_clang_tidy %s misc-const-correctness %t -- \ +// RUN: -config="{CheckOptions: [\ +// RUN: {key: 'misc-const-correctness.TransformValues', value: true}, \ +// RUN: {key: 'misc-const-correctness.WarnPointersAsValues', value: false}, \ +// RUN: {key: 'misc-const-correctness.TransformPointersAsValues', value: false}, \ +// RUN: ]}" -- -fno-delayed-template-parsing + +// ------- 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 + +// FIXME: 'static' globals are not matched right now. They could be analyzed but aren't right now. +static int p_static_global = 42; + +namespace foo { +int scoped; +float np_scoped = 1; // namespace variables are like globals +} // namespace foo + +// FIXME: Similary to 'static' globals, anonymous globals are not matched and analyzed. +namespace { +int np_anonymous_global; +int p_anonymous_global = 43; +} // namespace + +// 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: int const p_local0 + + 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' + // CHECK-FIXES: int const p_local0 + int np_local0 = 42; + + { + int p_local1 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: variable 'p_local1' of type 'int' can be declared 'const' + // CHECK-FIXES: int const p_local1 + 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' + // CHECK-FIXES: int const p_local0 + + 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' + // CHECK-FIXES: int const p_local1 +} + +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' + // CHECK-FIXES: int const p_local0 + + 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' + // CHECK-FIXES: int const p_local1 + 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' + // CHECK-FIXES: int const p_local2 + 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' + // CHECK-FIXES: int const p_local0 + 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' + // CHECK-FIXES: int const p_local1 + 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' + // CHECK-FIXES: double const p_local0 + 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' + // CHECK-FIXES: double const p_local0 + double p_local1 = 24.4; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'double' can be declared 'const' + // CHECK-FIXES: double const p_local1 + 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' + // CHECK-FIXES: double const p_local0 + 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' + // CHECK-FIXES: double const p_local0 + double p_local1 = 24.4; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'double' can be declared 'const' + // CHECK-FIXES: double const p_local1 + 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' + // CHECK-FIXES: int const p_local0 + 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' + // CHECK-FIXES: ConstNonConstClass const p_local0 + p_local0.constMethod(); + + ConstNonConstClass p_local1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'ConstNonConstClass' can be declared 'const' + // CHECK-FIXES: ConstNonConstClass const p_local1 + 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' + // CHECK-FIXES: ConstNonConstClass const p_local2(np_local2) + + 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' + // CHECK-FIXES: ConstNonConstClass const p_local3 + 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' + // CHECK-FIXES: ConstNonConstClass const p_local4 + *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' + // CHECK-FIXES: ConstNonConstClass const p_local0[2] + 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' + // CHECK-FIXES: OperatorsAsConstAsPossible const p_local0 + OperatorsAsConstAsPossible p_local1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'OperatorsAsConstAsPossible' can be declared 'const' + // CHECK-FIXES: OperatorsAsConstAsPossible const p_local1 + + 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' + // CHECK-FIXES: NonConstOperators const p_local2 + NonConstOperators p_local3 = p_local2 - p_local2; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local3' of type 'NonConstOperators' can be declared 'const' + // CHECK-FIXES: NonConstOperators const p_local3 +} + +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' + // CHECK-FIXES: double const p_local0[10] + double p_local1 = p_local0[5]; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'double' can be declared 'const' + // CHECK-FIXES: double const p_local1 + + // 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' + // CHECK-FIXES: double const p_local2 + + 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' + // CHECK-FIXES: double const p_local3 +} + +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' + // CHECK-FIXES: double const p_local1[10] + 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' + // CHECK-FIXES: double const p_local3[10] + 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' + // CHECK-FIXES: double const p_local4[10] + 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' + // CHECK-FIXES: double const p_local5[10] + 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' + // CHECK-FIXES: double const p_local6[10] + 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; + } + + int *np_local3[2] = {&np_local0[0], &np_local0[1]}; + 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' + // CHECK-FIXES: int const p_local0[2] + for (int value : p_local0) { + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'value' of type 'int' can be declared 'const' + // CHECK-FIXES: int const value + } + + int p_local1[2] = {1, 2}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int[2]' can be declared 'const' + // CHECK-FIXES: int const p_local1[2] + 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' + // CHECK-FIXES: int *const p_local2[2] + 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' + // CHECK-FIXES: int *const p_local3[2] + 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' + // CHECK-FIXES: NonModifyingConversion const p_local0 + + 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 *))' + // CHECK-FIXES: decltype(sizeof(void *)) const p_local0 + 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' + // CHECK-FIXES: int const p_local1 + short p_local2 = static_cast(p_local1); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'short' can be declared 'const' + // CHECK-FIXES: short const p_local2 + + 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' + // CHECK-NOT-FIXES: int const p_local0 = 3 + 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 libcxx/include/type_traits and improved readability. +template +struct integral_constant { + static constexpr const Tp value = v; + using value_type = Tp; + using type = integral_constant; + constexpr operator value_type() const noexcept { return value; } + constexpr value_type operator()() const noexcept { return value; } +}; + +template +struct is_integral : integral_constant {}; +template <> +struct is_integral : integral_constant {}; + +template +struct not_integral : integral_constant {}; +template <> +struct not_integral : integral_constant {}; + +template +struct enable_if {}; + +template +struct enable_if { using type = Tp; }; + +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: TMPClass const p_local0 + 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: TMPClass const p_local1 + 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' + // CHECK-FIXES: int const p_local0 + 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' + // CHECK-FIXES: HardwareRegister const p_reg1 + 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' + // CHECK-FIXES: int const p_local0 + 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' + // CHECK-FIXES: int const p_local0 + ; + + // 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' + // CHECK-FIXES: int const p_local1 + ; + + 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' + // CHECK-FIXES: int const p_local2 + ; + + 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' + // CHECK-FIXES: int const p_local3 + ; + } +} + +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' + // CHECK-FIXES: good_iterator const p_local0 + good_iterator &p_local1 = p_local0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'good_iterator &' can be declared 'const' + // CHECK-FIXES: good_iterator const&p_local1 + + for (int p_local2 : p_local1) { + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'p_local2' of type 'int' can be declared 'const' + // CHECK-FIXES: int const p_local2 + (void)p_local2; + } + + good_iterator p_local3; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local3' of type 'good_iterator' can be declared 'const' + // CHECK-FIXES: good_iterator const p_local3 + for (int p_local4 : p_local3) + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'p_local4' of type 'int' can be declared 'const' + // CHECK-FIXES: int const p_local4 + ; + 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' + // CHECK-FIXES: int const(&p_local0)[42] + + for (int p_local1 : p_local0) { + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'p_local1' of type 'int' can be declared 'const' + // CHECK-FIXES: int const p_local1 + (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' + // CHECK-FIXES: int const p_local0 + int np_local1 = 42; + (np_local0 == p_local0 ? np_local0 : (p_local0, np_local1))++; +} + +void vlas() { + int N = 1; // Can't make N 'const' because VLAs make everything awful + sizeof(int[++N]); +} + +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-dependent. + SmallVector p_local0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'SmallVector' can be declared 'const' + // CHECK-FIXES: SmallVector const p_local0 + 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' + // CHECK-FIXES: int *const p_local0[4] + 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' + // CHECK-FIXES: list_init const p_local0 +} +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' + // CHECK-FIXES: list_init_derived const p_local0 +} +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' + // CHECK-FIXES: base const p_local1 +} + +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; +} + +template +T &return_ref() { + static T global; + return global; +} +template +T *return_ptr() { return &return_ref(); } + +void auto_usage_variants() { + auto auto_val0 = int{}; + // CHECK-FIXES-NOT: auto const auto_val0 + auto &auto_val1 = auto_val0; + auto *auto_val2 = &auto_val0; + + auto auto_ref0 = return_ref(); + // CHECK-FIXES-NOT: auto const auto_ref0 + auto &auto_ref1 = return_ref(); // Bad + auto *auto_ref2 = return_ptr(); + + auto auto_ptr0 = return_ptr(); + // CHECK-FIXES-NOT: auto const auto_ptr0 + auto &auto_ptr1 = auto_ptr0; + auto *auto_ptr2 = return_ptr(); + + using MyTypedef = int; + auto auto_td0 = MyTypedef{}; + // CHECK-FIXES-NOT: auto const auto_td0 + auto &auto_td1 = auto_td0; + auto *auto_td2 = &auto_td0; +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-wrong-config.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-wrong-config.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-wrong-config.cpp @@ -0,0 +1,12 @@ +// RUN: %check_clang_tidy %s misc-const-correctness %t \ +// RUN: -config='{CheckOptions: \ +// RUN: [{key: "misc-const-correctness.AnalyzeValues", value: false},\ +// RUN: {key: "misc-const-correctness.AnalyzeReferences", value: false},\ +// RUN: ]}' -- -fno-delayed-template-parsing + +// CHECK-MESSAGES: warning: The check 'misc-const-correctness' will not perform any analysis because both 'AnalyzeValues' and 'AnalyzeReferences' are false. [clang-tidy-config] + +void g() { + int p_local0 = 42; + // CHECK-FIXES-NOT: int const p_local0 = 42; +} 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 @@ -455,14 +455,16 @@ // 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); + const auto RefToArrayRefToElements = + match(findAll(stmt(cxxForRangeStmt( + hasLoopVariable( + varDecl(anyOf(hasType(nonConstReferenceType()), + hasType(nonConstPointerType()))) + .bind(NodeID::value)), + hasRangeStmt(DeclStmtToNonRefToArray), + hasRangeInit(canResolveToExpr(equalsNode(Exp))))) + .bind("stmt")), + Stm, Context); if (const auto *BadRangeInitFromArray = selectFirst("stmt", RefToArrayRefToElements)) 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 @@ -1251,13 +1251,13 @@ AST = buildASTFromCode("void f() { int* x[2]; for (int* e : x) e = nullptr; }"); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); - EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_TRUE(isMutated(Results, AST.get())); AST = buildASTFromCode( "typedef int* IntPtr;" "void f() { int* x[2]; for (IntPtr e : x) e = nullptr; }"); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); - EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_TRUE(isMutated(Results, AST.get())); } TEST(ExprMutationAnalyzerTest, RangeForArrayByConstRef) {