diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt index be8528593250..13c15bc9d227 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt @@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyCppCoreGuidelinesModule AvoidGotoCheck.cpp CppCoreGuidelinesTidyModule.cpp + InitVariablesCheck.cpp InterfacesGlobalInitCheck.cpp MacroUsageCheck.cpp NarrowingConversionsCheck.cpp diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp index 7c7fd1b72cb7..8886eb833795 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -15,6 +15,7 @@ #include "../modernize/UseOverrideCheck.h" #include "../readability/MagicNumbersCheck.h" #include "AvoidGotoCheck.h" +#include "InitVariablesCheck.h" #include "InterfacesGlobalInitCheck.h" #include "MacroUsageCheck.h" #include "NarrowingConversionsCheck.h" @@ -49,6 +50,8 @@ public: "cppcoreguidelines-avoid-magic-numbers"); CheckFactories.registerCheck( "cppcoreguidelines-explicit-virtual-functions"); + CheckFactories.registerCheck( + "cppcoreguidelines-init-variables"); CheckFactories.registerCheck( "cppcoreguidelines-interfaces-global-init"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp new file mode 100644 index 000000000000..8a628317a302 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp @@ -0,0 +1,105 @@ +//===--- InitVariablesCheck.cpp - clang-tidy ------------------------------===// +// +// 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 "InitVariablesCheck.h" + +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace cppcoreguidelines { + +namespace { +AST_MATCHER(VarDecl, isLocalVarDecl) { return Node.isLocalVarDecl(); } +} // namespace + +InitVariablesCheck::InitVariablesCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IncludeStyle(utils::IncludeSorter::parseIncludeStyle( + Options.getLocalOrGlobal("IncludeStyle", "llvm"))), + MathHeader(Options.get("MathHeader", "math.h")) {} + +void InitVariablesCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(varDecl(unless(hasInitializer(anything())), + unless(isInstantiated()), isLocalVarDecl(), + unless(isStaticLocal()), isDefinition()) + .bind("vardecl"), + this); +} + +void InitVariablesCheck::registerPPCallbacks(const SourceManager &SM, + Preprocessor *PP, + Preprocessor *ModuleExpanderPP) { + IncludeInserter = + std::make_unique(SM, getLangOpts(), IncludeStyle); + PP->addPPCallbacks(IncludeInserter->CreatePPCallbacks()); +} + +void InitVariablesCheck::check(const MatchFinder::MatchResult &Result) { + const auto *MatchedDecl = Result.Nodes.getNodeAs("vardecl"); + const ASTContext &Context = *Result.Context; + const SourceManager &Source = Context.getSourceManager(); + + // We want to warn about cases where the type name + // comes from a macro like this: + // + // TYPENAME_FROM_MACRO var; + // + // but not if the entire declaration comes from + // one: + // + // DEFINE_SOME_VARIABLE(); + // + // or if the definition comes from a macro like SWAP + // that uses an internal temporary variable. + // + // Thus check that the variable name does + // not come from a macro expansion. + if (MatchedDecl->getEndLoc().isMacroID()) + return; + + QualType TypePtr = MatchedDecl->getType(); + const char *InitializationString = nullptr; + bool AddMathInclude = false; + + if (TypePtr->isIntegerType()) + InitializationString = " = 0"; + else if (TypePtr->isFloatingType()) { + InitializationString = " = NAN"; + AddMathInclude = true; + } else if (TypePtr->isAnyPointerType()) { + if (getLangOpts().CPlusPlus11) + InitializationString = " = nullptr"; + else + InitializationString = " = NULL"; + } + + if (InitializationString) { + auto Diagnostic = + diag(MatchedDecl->getLocation(), "variable %0 is not initialized") + << MatchedDecl + << FixItHint::CreateInsertion( + MatchedDecl->getLocation().getLocWithOffset( + MatchedDecl->getName().size()), + InitializationString); + if (AddMathInclude) { + auto IncludeHint = IncludeInserter->CreateIncludeInsertion( + Source.getFileID(MatchedDecl->getBeginLoc()), MathHeader, false); + if (IncludeHint) + Diagnostic << *IncludeHint; + } + } +} + +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.h new file mode 100644 index 000000000000..0cacf9e533cf --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.h @@ -0,0 +1,42 @@ +//===--- InitVariablesCheck.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_INITVARIABLESCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_INITVARIABLESCHECK_H + +#include "../ClangTidyCheck.h" +#include "../utils/IncludeInserter.h" +#include "../utils/OptionsUtils.h" + +namespace clang { +namespace tidy { +namespace cppcoreguidelines { + +/// Find uninitialized local variables. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-init-variables.html +class InitVariablesCheck : public ClangTidyCheck { +public: + InitVariablesCheck(StringRef Name, ClangTidyContext *Context); + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + std::unique_ptr IncludeInserter; + const utils::IncludeSorter::IncludeStyle IncludeStyle; + const std::string MathHeader; +}; + +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_INITVARIABLESCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 2ba944bc7d0e..e5be18202c9a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -79,24 +79,27 @@ Improvements to clang-tidy Finds obvious infinite loops (loops where the condition variable is not changed at all). +- New :doc:`cppcoreguidelines-init-variables + ` check. + - New :doc:`darwin-dispatch-once-nonstatic ` check. Finds declarations of ``dispatch_once_t`` variables without static or global storage. -- New :doc:`linuxkernel-must-use-errs - ` check. - - Checks Linux kernel code to see if it uses the results from the functions in - ``linux/err.h``. - - New :doc:`google-upgrade-googletest-case ` check. Finds uses of deprecated Googletest APIs with names containing ``case`` and replaces them with equivalent APIs with ``suite``. +- New :doc:`linuxkernel-must-use-errs + ` check. + + Checks Linux kernel code to see if it uses the results from the functions in + ``linux/err.h``. + - New :doc:`llvm-prefer-register-over-unsigned ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst new file mode 100644 index 000000000000..e1b0fe323000 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst @@ -0,0 +1,51 @@ +.. title:: clang-tidy - cppcoreguidelines-init-variables + +cppcoreguidelines-init-variables +================================ + +Checks whether there are local variables that are declared without an +initial value. These may lead to unexpected behaviour if there is a +code path that reads the variable before assigning to it. + +Only integers, booleans, floats, doubles and pointers are checked. The +fix option initializes all detected values with the value of zero. An +exception is float and double types, which are initialized to NaN. + +As an example a function that looks like this: + +.. code-block:: c++ + + void function() { + int x; + char *txt; + double d; + + // Rest of the function. + } + +Would be rewritten to look like this: + +.. code-block:: c++ + + #include + + void function() { + int x = 0; + char *txt = nullptr; + double d = NAN; + + // Rest of the function. + } + +Options +------- + +.. option:: IncludeStyle + + A string specifying which include-style is used, `llvm` or + `google`. Default is `llvm`. + +.. option:: MathHeader + + A string specifying the header to include to get the definition of + `NAN`. Default is `math.h`. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index ae7ca492b674..f0f35bd9e621 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -194,6 +194,7 @@ Clang-Tidy Checks cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) cppcoreguidelines-explicit-virtual-functions (redirects to modernize-use-override) + cppcoreguidelines-init-variables cppcoreguidelines-interfaces-global-init cppcoreguidelines-macro-usage cppcoreguidelines-narrowing-conversions diff --git a/clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp b/clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp new file mode 100644 index 000000000000..893c1d287798 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp @@ -0,0 +1,80 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t + +// Ensure that function declarations are not changed. +void some_func(int x, double d, bool b, const char *p); + +// Ensure that function arguments are not changed +int identity_function(int x) { + return x; +} + +int do_not_modify_me; + +static int should_not_be_initialized; +extern int should_not_be_initialized2; + +typedef struct { + int unaltered1; + int unaltered2; +} UnusedStruct; + +typedef int my_int_type; +#define MACRO_INT int +#define FULL_DECLARATION() int macrodecl; + +template +void template_test_function() { + T t; + int uninitialized; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'uninitialized' is not initialized [cppcoreguidelines-init-variables] + // CHECK-FIXES: {{^}} int uninitialized = 0;{{$}} +} + +void init_unit_tests() { + int x; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'x' is not initialized [cppcoreguidelines-init-variables] + // CHECK-FIXES: {{^}} int x = 0;{{$}} + my_int_type myint; + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'myint' is not initialized [cppcoreguidelines-init-variables] + // CHECK-FIXES: {{^}} my_int_type myint = 0;{{$}} + + MACRO_INT macroint; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable 'macroint' is not initialized [cppcoreguidelines-init-variables] + // CHECK-FIXES: {{^}} MACRO_INT macroint = 0;{{$}} + FULL_DECLARATION(); + + int x0 = 1, x1, x2 = 2; + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'x1' is not initialized [cppcoreguidelines-init-variables] + // CHECK-FIXES: {{^}} int x0 = 1, x1 = 0, x2 = 2;{{$}} + int y0, y1 = 1, y2; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'y0' is not initialized [cppcoreguidelines-init-variables] + // CHECK-MESSAGES: :[[@LINE-2]]:19: warning: variable 'y2' is not initialized [cppcoreguidelines-init-variables] + // CHECK-FIXES: {{^}} int y0 = 0, y1 = 1, y2 = 0;{{$}} + int hasval = 42; + + float f; + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: variable 'f' is not initialized [cppcoreguidelines-init-variables] + // CHECK-FIXES: {{^}} float f = NAN;{{$}} + float fval = 85.0; + double d; + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: variable 'd' is not initialized [cppcoreguidelines-init-variables] + // CHECK-FIXES: {{^}} double d = NAN;{{$}} + double dval = 99.0; + + bool b; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: variable 'b' is not initialized [cppcoreguidelines-init-variables] + // CHECK-FIXES: {{^}} bool b = 0;{{$}} + bool bval = true; + + const char *ptr; + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'ptr' is not initialized [cppcoreguidelines-init-variables] + // CHECK-FIXES: {{^}} const char *ptr = nullptr;{{$}} + const char *ptrval = "a string"; + + UnusedStruct u; + + static int does_not_need_an_initializer; + extern int does_not_need_an_initializer2; + int parens(42); + int braces{42}; +}