njames93 added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp:11-13
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
----------------
You don't need to include or use the AST matchers in a preprocessor only check.
================
Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp:50
+
+ (void)FilenameRange;
+ (void)File;
----------------
I have a feeling this is to suppress some unused parameter linting check. If it
is then it should be removed as unused parameters in an overridden function
shouldn't emit a warning.
Same for below.
Side note: File a bug with whichever linter tool gave you that warning (if
there was one).
================
Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp:62
+ Check.diag(HashLoc, "suspicious #include")
+ << FixItHint::CreateReplacement(FilenameRange,
+ ((IsAngled ? "<" : "\"") + FileName +
----------------
jroelofs wrote:
> njames93 wrote:
> > This replacement is dangerous, I have a feeling no fix-it should be
> > provided or at least do a search of the include directories to see if file
> > you are trying to include actually does exist. The correct file could be
> > `*.hpp` like what boost uses for all its header files
> Yeah, perhaps the FixIt should only be added if there is a single candidate
> replacement that exists on the `-I` path.
>
> Another option is to not add FixIts at all, and instead emit a list of
> `note:`s suggesting each of the candidates.
How about the case if someone wants to (for whatever reason) include something
like this:
```
#include "example.h"
#include "example.cpp"
```
That looks intentional and a fix it shouldn't be emitted.
================
Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.h:31
+ : ClangTidyCheck(Name, Context) {}
+ void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP);
----------------
This should be marked `override`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74669/new/
https://reviews.llvm.org/D74669
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
