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

[Jussi Pakkanen via Phabricator via cfe-commits]

jpakkane marked 9 inline comments as done.
jpakkane added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:21
+  Finder->addMatcher(
+      varDecl(unless(hasInitializer(anything())), unless(isInstantiated()))
+          .bind("vardecl"),
----------------
aaron.ballman wrote:
> This should not match on variable declarations that are automatically zero-initialized or have a default constructor, no?
That is correct. The test file has checks for global variables and struct members without initialisation.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:31-32
+
+  if (!MatchedDecl->isLocalVarDecl())
+    return;
+
----------------
aaron.ballman wrote:
> I think this should be made into a local AST matcher so the check can be hoisted into the matchers.
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.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:59
+  } else if (TypePtr->isFloatingType()) {
+    InitializationString = " = (0.0/0.0)"; // NaN without needing #includes
+  } else if (TypePtr->isPointerType()) {
----------------
aaron.ballman wrote:
> 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.
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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64671/new/

https://reviews.llvm.org/D64671