[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables
Jussi Pakkanen via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 2 14:33:58 PDT 2019
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
More information about the cfe-commits
mailing list