[clang-tidy] New check: misc-init-local-variables

[jpakkane]

Summary

This checks finds all primitive type local variables (integers, doubles, pointers) that are declared without an initial value. Includes fixit functionality to initialize said variables with a default value. This is zero for most types and NaN for floating point types. The use of NaNs is copied from the D programming language.

[lebedev.ri]

This certainly needs more tests: macros, -x C, ???

clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp

26–29 ↗

(On Diff #209609)

Can make these proper AST_MATCHER and do this in registerMatchers().

32–35 ↗

(On Diff #209609)

This feels brittle.

41–48 ↗

(On Diff #209609)

should be var = <init>;

43 ↗

(On Diff #209609)

I'm pretty sure there's simple utility function that can insert include.

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp

8 ↗

(On Diff #209609)

Please separate header from include files with empty line.

31 ↗

(On Diff #209609)

Please don't use auto when type could not be deduced from same statement.

41 ↗

(On Diff #209609)

May be spaces should be created around =? Same in other places.

56 ↗

(On Diff #209609)

Please run Clang-format.

clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.h

28 ↗

(On Diff #209609)

Unnecessary empty line.

clang-tools-extra/docs/clang-tidy/checks/misc-init-local-variables.rst

6 ↗

(On Diff #209609)

Please synchronize first statement with description in Release Notes.

31 ↗

(On Diff #209609)

Spaces around =. Same in other places.

[jpakkane]

Fixed issues found in code review.

[jpakkane]

clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp

26–29 ↗

(On Diff #209609)

Init checking is now in the matcher. I did not change isLocalVarDecl because according to the documentation there does not seem to be a builtin matcher for that. Since the isLocalVarDecl is semantically exactly what is needed here it seems a bit silly to reimplement that from scratch from matcher basic blocks.

32–35 ↗

(On Diff #209609)

There's not much else one can do. Also, all variable names that begin with an underscore are reserved for the standard library (I think), so we should not be changing those in any case.

[jpakkane]

Fix missing spaces in documentation.

[alexfh]

clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp

21 ↗

(On Diff #209706)

I believe, this should skip matches within template instantiations. Consider this code:

template<typename T>
void f(T) { T t; }
void g() {
    f(0);
    f(0.0);
}

What will the fix be?

32 ↗

(On Diff #209706)

Should this just disallow all fixes within macros? Maybe warnings as well.

[jpakkane]

clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp

21 ↗

(On Diff #209706)

I tested with the following function:

template<typename T>
void template_test_function() {
  T t;
  int uninitialized;
}

Currently it warns on the "uninitialized" variable regardless of whether the template is instantiated or not. If you call it with an int type, it will warn about variable t being uninitialized. If you call it with a, say, struct type, there is no warnings. Is this a reasonable approach?

32 ↗

(On Diff #209706)

I can change that, seems reasonable. Should it still retain this check, though? One would imagine there are other ways of getting variables whose names begin with an underscore.

[alexfh]

clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp

21 ↗

(On Diff #209706)

And what happens, if there are multiple instantiations of the same template, each of them requiring a different fix? Can you try the check with my example above (and maybe also add f("");inside g()). I believe, the check will produce multiple warnings with conflicting fixes (and each of them will be wrong, btw).

32 ↗

(On Diff #209706)

I don't know, whether the check for leading underscore will still be valuable. I don't think there's a valid reason why variables with a leading underscore in their names should in general not be initialized.

[jpakkane]

clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp

21 ↗

(On Diff #209706)

Interestingly it does warn about it, but only once, even if you have two different template specializations.

I tried to suppress this warning when the type being instantiated is a template argument type but no matter what I tried I could not get this to work. Is there a way to get this information from the MatchedDecl object or does one need to do something more complicated like going up the AST until a function definition is found and checking if it is a template specialization (presumably with TemplatedKind)? Any help would be appreciated.

[alexfh]

clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp

21 ↗

(On Diff #209706)

If there are multiple warnings with the same message at the same location (clang-tidy/ClangTidyDiagnosticConsumer.cpp:745), they will be deduplicated. Thus, a random fix will probably be suggested. The proper way to filter out matches in template instantiations is to add unless(isInTemplateInstantiation()) to the matcher.

[jpakkane]

clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp

21 ↗

(On Diff #209706)

I tried to make this work but I just could not combine statement and declaration matching in a reliable way. Matching a statement that is not in a template declaration can be done, as well as matching a declaration without intial value, but combining those two into one is hard. After trying many, many things the best I could come up with was this:

declStmt(containsDeclaration(0, varDecl(unless(hasInitializer(anything()))).bind("vardecl"))), this)

The problem is that containsDeclaration takes an integer denoting how manyth declaration should be processed. Manually adding matchers for, say, 0, 1, 2, 3 and 4 works and does the right thing but fails if anyone has an uninitialized variable in the sixth location, things will silently fail.

The weird thing is that if you do the matching this way, you don't need to filter out things with unless(isInTemplateInstantiation()). Maybe statements are handled differently from declarations?

[alexfh]

clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp

21 ↗

(On Diff #209706)

I was struggling to understand, why you want to match a statement, but then I figured out that I should have been more precise: while isInTemplateInstantiation only works for Stmts, there's a related matcher that works for Decls: isInstantiated. See clang/include/clang/ASTMatchers/ASTMatchers.h:5187. In general, looking into this header can be useful, if you want to find a matcher that you can vaguely describe (e.g. when looking for something related to instantiations, you can search for the relevant substring and find this and a bunch of other matchers).

Sorry for the confusion. I hope, the suggestion helps.

[jpakkane]

Now properly deals with template instantiations and macros.

[jpakkane]

clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp

21 ↗

(On Diff #209706)

Thanks, got it working now.

32 ↗

(On Diff #209706)

Underscore check has been removed and macros are properly handled (description is in code comments).

[alexfh]

A general comment: "misc" is a sort of a heap of checks that otherwise don't have a good home. This one would probably better go to bugprone (or maybe there's a relevant CERT or C++ Core Guidelines rule?).

[jpakkane]

In D64671#1603427, @alexfh wrote:

A general comment: "misc" is a sort of a heap of checks that otherwise don't have a good home. This one would probably better go to bugprone (or maybe there's a relevant CERT or C++ Core Guidelines rule?).

The closest I could find is ES.20 "Always initialize an object": https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es20-always-initialize-an-object

Most other guidelines seem to be formulated as "don't read uninitialized memory" such as: https://cwe.mitre.org/data/definitions/457.html

So a potential name for this could be "cppcoreguidelines-init-variables". However that rule is about initializing all variables, even those in structs and classes. This is about local variables.

Related to this, there seems to be a talk at CppCon about doing something similar to this MR: https://cppcon2019.sched.com/event/SfYc/killing-uninitialized-memory-prot

[JonasToth]

In D64671#1606084, @jpakkane wrote:

In D64671#1603427, @alexfh wrote:

A general comment: "misc" is a sort of a heap of checks that otherwise don't have a good home. This one would probably better go to bugprone (or maybe there's a relevant CERT or C++ Core Guidelines rule?).

The closest I could find is ES.20 "Always initialize an object": https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es20-always-initialize-an-object

So a potential name for this could be "cppcoreguidelines-init-variables". However that rule is about initializing all variables, even those in structs and classes. This is about local variables.

I am in favor of something like cppcoreguidelines-init-variables that might catch other cases in the future as well. IMHO its fine to start with something partial, especially "just" local variables.

[jpakkane]

Renamed to cppcoreguidelines-init-variables.

[jpakkane]

Does this still need work? FWICT all issues raised have been fixed.

[Eugene.Zelenko]

clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp

4

It'll be better to include cstdint.

[lebedev.ri]

clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp

4

I'll correct that comment: tests normally shouldn't depend on system headers; can you just mock whatever you need from there?

[Eugene.Zelenko]

clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp

4

If test is C++, why not to include C++ equivalent of system headers? See modernize-deprecated-headers.

[aaron.ballman]

clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp

4

Because we want our tests to be hermetic and rely on the system running the test as little as possible.

[lebedev.ri]

clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp

4

I'm not commenting on c-vs-c++ header, i'm specifically commenting on an <> include; unless i'm misremembering this is advised against.

[jpakkane]

I used stdint to replicate a real world use case as I'd imagine those types would match this search quite heavily.

The tests already have one test for a typedeffed integer and one that is defined with a macro. If those are deemed sufficient, the stdint type can be removed altogether. If the stdint types are defined via some other mechanism such as compiler intrinsics, there would be no test for that. I do not know if that is the case with current libcs.

I can update the MR as requested once someone with enough stripes makes the policy decision.

[aaron.ballman]

In D64671#1644280, @jpakkane wrote:

I used stdint to replicate a real world use case as I'd imagine those types would match this search quite heavily.

The problem is that this uses the system header search paths and so the testing system has no control over *what* is found from bot to bot. That is why we structure almost all of our tests to not use system includes.

The tests already have one test for a typedeffed integer and one that is defined with a macro. If those are deemed sufficient, the stdint type can be removed altogether. If the stdint types are defined via some other mechanism such as compiler intrinsics, there would be no test for that. I do not know if that is the case with current libcs.

I don't see a need for int32_t specifically in this test.

I can update the MR as requested once someone with enough stripes makes the policy decision.

I'm not certain that we've written this instruction down so much as let new contributors know when they stumble into it. It would probably be a good idea for us to get some testing documentation written down at some point. Despite that, our policy has been to not use #include in test cases unless the included file is relative to the test.

[jpakkane]

Updated patch to remove #include<stdint.h> and the corresponding uint32_t test code snippet.

[aaron.ballman]

clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

46

Please keep this list sorted alphabetically by string literal.

clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp

21

This should not match on variable declarations that are automatically zero-initialized or have a default constructor, no?

29

You can move this closer to where it's being used.

31–32

I think this should be made into a local AST matcher so the check can be hoisted into the matchers.

53

No need for this local; you can lower into its only use.

56

You should elide braces for any single-statement blocks per our coding conventions.

59

I would rather see the correct NaN inserted along with the include. See StringFindStartswithCheck.cpp for an example of how to use the include inserter.

60

Do you expect this check to be run over ObjC code? If so, this should probably be isAnyPointerType().

61

This should be checking for C++11.

[jpakkane]

Updated to fix review comments. NOTE: detecting the include fix is broken because I could not get it to work.

[jpakkane]

clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp

21

That is correct. The test file has checks for global variables and struct members without initialisation.

31–32

As was discussed earlier, the use of isLocalVarDecl is used here because:

• it has the exact semantics needed in this particular case
• it is not available as a matcher, I could not make plain matchers replicate its behaviour and even if I could, reimplementing it from scratch seems a bit pointless

If there is a way to use that function directly in the matcher code, let me know how it's done and I will fix this. But if not, then sadly I have no idea how that should be fixed and it is probably a bigger development task than this checker itself.

59

I copied the implementation and could make it add the update. However I could not for the life of me make the test framework accept this. It will always flag an error even though there should be a comment that declares it in the test source. Any help is appreciated.

[aaron.ballman]

clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp

31–32

You can create your own AST matchers that are used only in this translation unit. e.g.,

AST_MATCHER(VarDecl, isLocalVarDecl) {
  return Node.isLocalVarDecl();
}

and then use this in your AST matchers.

59

So the include is inserted as you expect, but the test continues to fail? What does your test look like for it? What failures are you getting?

77

In C++11 mode, I think this should recommend std::numeric_limits<type>::quiet_NaN() if possible, from <limits> rather than <math.h>.

87–102

I think you want this to be:

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;
  }

[jpakkane]

Updated patch as per review comments.

[jpakkane]

clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp

31–32

Fixed, thanks.

59

Just the test in this patch. However after updating the diagnostic code to the one recommended made this problem go away. This now works as intended.

77

As a stylistic point I would disagree with this. In this particular case the "C++ way" looks very verbose and confusing. For a single variable this is not a huge problem, but it gets very ugly when you have something like this (which is common in real world code bases):

double d1, d2, d3, d4, d5, d6, d7;

This just explodes and is very ugly and unpleasant to read when every one of these gets the std::numeric_limits template value. I would recommend using plain NAN for terseness and readability, but can update the PR if the template form is deemed the better choice.

[aaron.ballman]

clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp

67

Elide braces.

77

Let's go with NAN for now. We can always add an option letting the user pick which style to replace with later.

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst

34

This looks incorrect.

clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp

4

You can drop this and its uses, we don't care about warnings produced by Clang about the variables not being used.

37

I'd like to see a test using a local static (which should not diagnose, as it's zero-initialized by default) and a local extern (which should not diagnose because it cannot have an initializer).

56

I'd like to see some tests for other initialization forms as well:

int braces{42};
int parens(42);

[jpakkane]

Fixed issues raised in review.

[aaron.ballman]

Aside from the missing documentation bit, I think this LG.

clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp

30

You need to document this option.

[jpakkane]

Added documentation. Well, actually just swiped it from abseil-string-find-startswith.

[Eugene.Zelenko]

It'll be reasonable to get IncludeStyle default from .clang-format.

clang-tools-extra/docs/ReleaseNotes.rst

71

Please sort new checks list alphabetically.

[jpakkane]

It'll be reasonable to get IncludeStyle default from .clang-format.

I looked at existing checks and they all do the same thing as this one. In fact I got the code for this by directly copypasting an existing check.

Grepping for clang format in the check tree produces zero relevant matches, so it's unclear to me how this should be written (assuming that this is not taken care of automatically by utility code).

clang-tools-extra/docs/ReleaseNotes.rst

71

The list is already incorrectly alphabetized. Should I reorganize all the entries or just put mine in a "correct" spot (between "bugprone" and "linuxkernel")?

[Eugene.Zelenko]

In D64671#1661585, @jpakkane wrote:

It'll be reasonable to get IncludeStyle default from .clang-format.

I looked at existing checks and they all do the same thing as this one. In fact I got the code for this by directly copypasting an existing check.

SInse this option may be also set in .clang-format, it's reasonable to re-use it instead of forcing user to duplicate it.

clang-tools-extra/docs/ReleaseNotes.rst

71

It'll not hurt to correct other mistakes too.

[jpakkane]

Ordered doc list alphabetically.

[aaron.ballman]

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst

41

This is still missing the documentation for MathHeader and its default value.

[jpakkane]

Added MathHeader documentation.

[aaron.ballman]

LGTM!

[jpakkane]

Rebased against master to fix a merge conflict in release notes.

[aaron.ballman]

Do you need someone to commit this on your behalf (sorry for not asking that question sooner)?

[jpakkane]

In D64671#1688626, @aaron.ballman wrote:

Do you need someone to commit this on your behalf (sorry for not asking that question sooner)?

Yes, please. I have no rights of any kind, this is in fact my first ever pull request to LLVM.

[aaron.ballman]

In D64671#1690058, @jpakkane wrote:

In D64671#1688626, @aaron.ballman wrote:

Do you need someone to commit this on your behalf (sorry for not asking that question sooner)?

Yes, please. I have no rights of any kind, this is in fact my first ever pull request to LLVM.

I'm happy to commit for you, but applying the patch causes merge conflicts. Can you rebase on ToT?

[jpakkane]

Rebased against latest Git master. Didn't see any rebase conflicts, though...

[aaron.ballman]

In D64671#1691538, @jpakkane wrote:

Rebased against latest Git master. Didn't see any rebase conflicts, though...

Oh! I just noticed you made the patch against the monorepo which I'm not using (I'm still on svn), so the paths in your patch file are all wrong for me. After some surgery on the paths, it applied cleanly for me.

I committed on your behalf in r373489. Thank you for the new feature!

clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt

 add_clang_library(clangTidyCppCoreGuidelinesModule
   AvoidGotoCheck.cpp
   CppCoreGuidelinesTidyModule.cpp
+  InitVariablesCheck.cpp
   InterfacesGlobalInitCheck.cpp
   MacroUsageCheck.cpp
   NarrowingConversionsCheck.cpp

clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

 #include "../modernize/UseOverrideCheck.h"
 #include "../readability/MagicNumbersCheck.h"
 #include "AvoidGotoCheck.h"
+#include "InitVariablesCheck.h"
 #include "InterfacesGlobalInitCheck.h"
 #include "MacroUsageCheck.h"
 #include "NarrowingConversionsCheck.h"

 class CppCoreGuidelinesModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<InitVariablesCheck>(
+        "cppcoreguidelines-init-variables");
     CheckFactories.registerCheck<modernize::AvoidCArraysCheck>(
         "cppcoreguidelines-avoid-c-arrays");
     CheckFactories.registerCheck<AvoidGotoCheck>(

clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.h

+//===--- 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"
+
+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)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_INITVARIABLESCHECK_H

clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp

+//===--- 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 {
+
+void InitVariablesCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      varDecl(unless(hasInitializer(anything())), unless(isInstantiated()))
+          .bind("vardecl"),
+      this);
+}
+
+void InitVariablesCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<VarDecl>("vardecl");
+
+  const char *InitializationString = nullptr;
+
+  if (!MatchedDecl->isLocalVarDecl())
+    return;
+
+  // 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;
+  }
+
+  StringRef VarName = MatchedDecl->getName();
+  QualType TypePtr = MatchedDecl->getType();
+
+  if (TypePtr->isIntegerType()) {
+    InitializationString = " = 0";
+  } else if (TypePtr->isFloatingType()) {
+    InitializationString = " = (0.0/0.0)"; // NaN without needing #includes
+  } else if (TypePtr->isPointerType()) {
+    if (getLangOpts().CPlusPlus) {
+      InitializationString = " = nullptr";
+    } else {
+      InitializationString = " = NULL";
+    }
+  }
+
+  if (InitializationString) {
+    diag(MatchedDecl->getLocation(), "variable %0 is not initialized")
+        << MatchedDecl;
+    diag(MatchedDecl->getLocation(), "insert initial value",
+         DiagnosticIDs::Note)
+        << FixItHint::CreateInsertion(
+               MatchedDecl->getLocation().getLocWithOffset(VarName.size()),
+               InitializationString);
+  }
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

 Improvements to clang-tidy
 --------------------------
 
+
+- New :doc:`cppcoreguidelines-init-variables
+  <clang-tidy/checks/cppcoreguidelines-init-variables>` check.
+
+  Checks whether there are local variables that are declared without
+  an initial value.
+
 - New :doc:`bugprone-dynamic-static-initializers
   <clang-tidy/checks/bugprone-dynamic-static-initializers>` check.
 

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst

+.. 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++
+
+   void function() {
+     int x = 0;
+     char *txt = nullptr;
+     double d = (0.0/0.0);
+
+     // Rest of the function.
+   }
+

clang-tools-extra/docs/clang-tidy/checks/list.rst

    cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) <cppcoreguidelines-avoid-magic-numbers>
    cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) <cppcoreguidelines-c-copy-assignment-signature>
    cppcoreguidelines-explicit-virtual-functions (redirects to modernize-use-override) <cppcoreguidelines-explicit-virtual-functions>
+   cppcoreguidelines-init-variables
    cppcoreguidelines-interfaces-global-init
    cppcoreguidelines-macro-usage
    cppcoreguidelines-narrowing-conversions

clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp

+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t
+
+#define DO_NOTHING(x) ((void)x)
+
+// 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;
+
+typedef struct {
+  int unaltered1;
+  int unaltered2;
+} UnusedStruct;
+
+typedef int my_int_type;
+#define MACRO_INT int
+#define FULL_DECLARATION() int macrodecl;
+
+template <typename T>
+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;{{$}}
+
+  DO_NOTHING(t);
+  DO_NOTHING(uninitialized);
+}
+
+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 = (0.0/0.0);{{$}}
+  float fval = 85.0;
+  double d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: variable 'd' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  double d = (0.0/0.0);{{$}}
+  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;
+
+  DO_NOTHING(x);
+  DO_NOTHING(myint);
+  DO_NOTHING(macroint);
+  DO_NOTHING(macrodecl);
+  DO_NOTHING(x0);
+  DO_NOTHING(x1);
+  DO_NOTHING(x2);
+  DO_NOTHING(y0);
+  DO_NOTHING(y1);
+  DO_NOTHING(y2);
+  DO_NOTHING(hasval);
+  DO_NOTHING(f);
+  DO_NOTHING(fval);
+  DO_NOTHING(d);
+  DO_NOTHING(dval);
+  DO_NOTHING(b);
+  DO_NOTHING(bval);
+  DO_NOTHING(ptr);
+  DO_NOTHING(ptrval);
+  DO_NOTHING(u);
+}