aaron.ballman added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:34
+ std::is_base_of<Redeclarable<T>, T>::value
+ &&std::is_base_of<NamedDecl, T>::value;
+
----------------
================
Comment at:
clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:84
+ if (!Check.checkCorrespongdingHeader())
+ return; // Found a good candidate for matching decl
+ StringRef ThisStem = path::stem(SM.getFilename(BeginLoc));
----------------
================
Comment at:
clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:87
+ if (!ThisStem.empty() && Stem.startswith_lower(ThisStem))
+ return; // Found a good candidate for matching decl
+ }
----------------
================
Comment at:
clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:162
+
+static NameAndFixMode getNameAndFixMode(TagTypeKind Kind, bool CPlusPlus) {
+ FixMode Mode = CPlusPlus ? FixMode::AnonNamespace : FixMode::None;
----------------
clang-tidy diagnostics do not start with a capital letter, so these string
literals should all be lowercase.
================
Comment at:
clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:190
+ utils::defaultFileExtensionDelimiters())) {
+ llvm::errs() << "Invalid implementation file extension: "
+ << RawStringImplementationFileExtensions << "\n";
----------------
Should this be using `configurationDiag()` instead of `llvm::errs()`?
================
Comment at:
clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:232-234
+ checkDecl(*this, *VD, *Result.Context, {"Variable", FixMode::Static});
+ else if (const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("FD"))
+ checkDecl(*this, *FD, *Result.Context, {"Function", FixMode::Static});
----------------
These string literals should also be lowercased.
================
Comment at:
clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:253
+ // Disable running this check on a file that isn't classed as an
+ // implementation file. can occur when running in clangd.
+ if (!isImplementationFile(getCurrentMainFile()))
----------------
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp:12
+extern bool DeclInSource;
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: Variable 'DeclInSource' is
declared as extern in a source file
+// CHECK-FIXES: {{^\s*}}bool DeclInSource;
----------------
Warning on this construct worries me -- it's not uncommon to use extern
declarations in some language modes. For instance, in C, you'll still find
plenty of older code bases that use an extern declaration of a system function
rather than `#include`'ing the standard library header. Also, folks will use
extern definitions as a way to smuggle data between TUs without exposing a
public interface (not always the best of practices, but it is an approach to
hiding implementation details).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73413/new/
https://reviews.llvm.org/D73413
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
