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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
