[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

sylvestre.ledru added a comment. Thanks for this, it found 74 occurrences of this in the firefox code base https://hg.mozilla.org/mozilla-central/rev/b07f125d09fd Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7982/new/ https://reviews.llvm.org/D798

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

This revision was automatically updated to reflect the committed changes. Closed by commit rGd2e8fb331835: [clang-tidy] Add readability-duplicate-include check (authored by LegalizeAdulthood). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7982/new/

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

LegalizeAdulthood added a comment. OK, removing Alex has cleared it. 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

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

LegalizeAdulthood added a comment. In D7982#3264009 , @carlosgalvezp wrote: > In D7982#3263920 , @LegalizeAdulthood > wrote: > >> In D7982#3263790 , @carlosgalvezp >> wrote: >

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

carlosgalvezp added a comment. In D7982#3263920 , @LegalizeAdulthood wrote: > In D7982#3263790 , @carlosgalvezp > wrote: > >> Great patch, thanks!! > > Can you mark the patch as accepted in phabricator? > Thanks I

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

LegalizeAdulthood added a comment. In D7982#3263790 , @carlosgalvezp wrote: > Great patch, thanks!! Can you mark the patch as accepted in phabricator? Thanks CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7982/new/ https://reviews.llvm.org/D7982

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

carlosgalvezp accepted this revision. carlosgalvezp added a comment. Great patch, thanks!! 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://

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include.h:1 +#if !defined(READABILITY_DUPLICATE_INCLUDE_H) +#define READABILITY_DUPLICATE_INCLUDE_H carlosg

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

LegalizeAdulthood updated this revision to Diff 402237. LegalizeAdulthood added a comment. Update from review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7982/new/ https://reviews.llvm.org/D7982 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-e

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

carlosgalvezp requested changes to this revision. carlosgalvezp added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include.h:1 +#if !defined(READA

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

LegalizeAdulthood updated this revision to Diff 402071. LegalizeAdulthood added a comment. - Update from review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7982/new/ https://reviews.llvm.org/D7982 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

LegalizeAdulthood added a comment. In D7982#3260403 , @carlosgalvezp wrote: > LGTM! Had some nits that can be fixed without review. I think you need to do "Accept Revision" in phabricator. It's in the "Add Action..." dropdown on the bottom of the review pa

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include.h:1 +#if !defined(READABILITY_DUPLICATE_INCLUDE_H) +#define READABILITY_DUPLICATE_INCLUDE_H carlosg

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

carlosgalvezp accepted this revision. carlosgalvezp added a comment. LGTM! Had some nits that can be fixed without review. Can't see anything else major that needs change. As said I'm fairly new here so probably a more experienced reviewer can find some more improvements/optimizations, especial

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

LegalizeAdulthood added a comment. @carlosgalvezp I believe I've addressed your comments now. Thanks for the review, it definitely improved this check. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7982/new/ https://reviews.llvm.org/D7982 ___

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

LegalizeAdulthood updated this revision to Diff 401837. LegalizeAdulthood added a comment. - Properly apply fixes to header files - Add tests for fixes in header files - Move function to static outside anonymous namespace - Improve documentation CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

LegalizeAdulthood added a comment. > ! In D7982#325 , @carlosgalvezp > wrote: > I need to familiarize myself better with the `Loc` manipulation > code to be able to review the free-standing function but otherwise > looks reasonable. The "clang inter

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

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 `

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

carlosgalvezp added a comment. There's also a negative vote from @alexfh , what about that? 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:/

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

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 +

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

LegalizeAdulthood added a comment. Ping. Still waiting for a review on this check. The implementation isn't particularly long or complicated, so it should not take much time to review. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7982/new/ https://reviews.llvm.org/D7982 ___

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

LegalizeAdulthood added a comment. 2nd. ping. Still waiting for a review on this check. 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://li

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

LegalizeAdulthood added a comment. Ping. This review has been waiting for a week without any comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7982/new/ https://reviews.llvm.org/D7982 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

LegalizeAdulthood updated this revision to Diff 398756. LegalizeAdulthood added a comment. clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7982/new/ https://reviews.llvm.org/D7982 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-extra/clang-tidy

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

LegalizeAdulthood added a comment. In D7982#3216332 , @Eugene.Zelenko wrote: > Functionality-wise this check is superseded by Include What You Use > . That's not part of clang-tidy and has it'

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

LegalizeAdulthood updated this revision to Diff 396941. LegalizeAdulthood added a comment. Move included headers to clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include Stub out included headers to isolate from system headers Run test for C++98 or later CHANGES SINCE