aaron.ballman added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:46
+ CheckFactories.registerCheck<InitVariablesCheck>(
+ "cppcoreguidelines-init-variables");
CheckFactories.registerCheck<modernize::AvoidCArraysCheck>(
----------------
Please keep this list sorted alphabetically by string literal.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:21
+ Finder->addMatcher(
+ varDecl(unless(hasInitializer(anything())), unless(isInstantiated()))
+ .bind("vardecl"),
----------------
This should not match on variable declarations that are automatically
zero-initialized or have a default constructor, no?
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:29
+
+ const char *InitializationString = nullptr;
+
----------------
You can move this closer to where it's being used.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:31-32
+
+ if (!MatchedDecl->isLocalVarDecl())
+ return;
+
----------------
I think this should be made into a local AST matcher so the check can be
hoisted into the matchers.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:53
+
+ StringRef VarName = MatchedDecl->getName();
+ QualType TypePtr = MatchedDecl->getType();
----------------
No need for this local; you can lower into its only use.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:56
+
+ if (TypePtr->isIntegerType()) {
+ InitializationString = " = 0";
----------------
You should elide braces for any single-statement blocks per our coding
conventions.
================
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()) {
----------------
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.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:60
+ InitializationString = " = (0.0/0.0)"; // NaN without needing #includes
+ } else if (TypePtr->isPointerType()) {
+ if (getLangOpts().CPlusPlus) {
----------------
Do you expect this check to be run over ObjC code? If so, this should probably
be `isAnyPointerType()`.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:61
+ } else if (TypePtr->isPointerType()) {
+ if (getLangOpts().CPlusPlus) {
+ InitializationString = " = nullptr";
----------------
This should be checking for C++11.
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
