carlosgalvezp added a comment.
Looks good! I need to familiarize myself better with the `Loc` manipulation
code to be able to review the free-standing function but otherwise looks
reasonable.
================
Comment at:
clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp:22
+
+SourceLocation advanceBeyondCurrentLine(const SourceManager &SM,
+ SourceLocation Start, int Offset) {
----------------
Move out of anonymous namespace and use `static`, as per LLVM guidelines.
================
Comment at:
clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp:64
+ SrcMgr::CharacteristicKind FileType) {
+ if (!SM.isInMainFile(HashLoc)) {
+ return;
----------------
I'm not familiar with `isInMainFile`, will the check work if the duplicated
include is in a header included by the main .cpp file? Should a test be added
for that?
================
Comment at:
clang-tools-extra/docs/clang-tidy/checks/readability-duplicate-include.rst:34-47
+ class LangOptionsBase {
+ public:
+ // Define simple language options (with no accessors).
+ #define LANGOPT(Name, Bits, Default, Description) unsigned Name : Bits;
+ #define ENUM_LANGOPT(Name, Type, Bits, Default, Description)
+ #include "clang/Basic/LangOptions.def"
+
----------------
As an external user that is not deeply involved in the details of LLVM, I find
this example rather complicated. Would it be possible to use a simpler example?
What about this one with `cassert`
```
#undef NDEBUG
#include <cassert>
// ... code with assertions enabled
#define NDEBUG
#include <cassert>
// ... code with assertions disabled
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D7982/new/
https://reviews.llvm.org/D7982
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
