[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 17 02:20:05 PST 2020


njames93 added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp:43
+
+void SuspiciousIncludePPCallbacks::InclusionDirective(
+    SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
----------------
This function is used for `#import` on modules. Do we still want the same behaviour when importing modules? More thinking about the extension suggestions with that one. 


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp:59
+        Optional<FileEntryRef> File = PP->LookupFile(
+            HashLoc /* FIXME: lies */, (FileName + RE).str(), IsAngled, nullptr,
+            nullptr, CurDir, nullptr, nullptr, nullptr, nullptr, nullptr);
----------------
jroelofs wrote:
> FIXME: is this a reasonable `SourceLocation` to give to `LookupFile`?
Main thing is to make sure that the same location is used for the warnings and the notes.
I personally found this the most pleasing diagnostics

```
SourceLocation DiagLoc = FilenameRange.getBegin().getLocWithOffset(1);
```

```
/home/nathan/src/llvm-project/build/Release/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-suspicious-include.cpp.tmp.cpp:7:11: warning: suspicious #include of file with .cpp extension [bugprone-suspicious-include]
#include "a.cpp"
          ^
/home/nathan/src/llvm-project/build/Release/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-suspicious-include.cpp.tmp.cpp:7:11: note: did you mean to include 'a'?
/home/nathan/src/llvm-project/build/Release/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-suspicious-include.cpp.tmp.cpp:7:11: note: did you mean to include 'a.h'?
/home/nathan/src/llvm-project/build/Release/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-suspicious-include.cpp.tmp.cpp:7:11: note: did you mean to include 'a.hpp'?
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669





More information about the cfe-commits mailing list