LegalizeAdulthood added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp:64
+ SrcMgr::CharacteristicKind FileType) {
+ if (!SM.isInMainFile(HashLoc)) {
+ return;
----------------
carlosgalvezp wrote:
> 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?
Good catch. I originally wrote this check in 2015 (yes, that's
how long it's been waiting for a review) and recently revived it.
At the time, I didn't understand that checks could apply fixits to
headers and that user headers could be differentiated from system
headers, so this is a mistake.
================
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"
+
----------------
carlosgalvezp wrote:
> 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
> ```
Good idea and I like your example better.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D7982/new/
https://reviews.llvm.org/D7982
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
