[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-11-09 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Sent out rG453c2879cb2ad6c46267ef8f39f0274aed69d9ee to fix this Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135859/new/ https://reviews.llvm.or

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-11-09 Thread Maryam Moghadas via Phabricator via cfe-commits
maryammo added a comment. @kadircet, this commit causes failure on https://lab.llvm.org/buildbot/#/builders/121 which is possible to reproduce locally, can you please take a look? [ 35/111] Linking CXX executable tools/clang/tools/extra/include-cleaner/unittests/ClangIncludeCleanerTests F

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-11-08 Thread Thorsten via Phabricator via cfe-commits
tschuett added a comment. No worries. The style asks to move the definitions out of the anonymous namespace, but Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135859/new/ https://reviews.llvm.org/D135859 _

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-11-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:25 namespace clang::include_cleaner { namespace { +using DeclCallback = tschuett wrote: > There is a cuter way to use anonymous namespaces: > https://llvm.org/docs/Cod

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-11-08 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd19ba74dee0b: [Includecleaner] Introduce RefType to ast walking (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D135859?vs=472925&id=474001#toc Repository: rG LLVM Github Mo

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-11-03 Thread Thorsten via Phabricator via cfe-commits
tschuett added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:25 namespace clang::include_cleaner { namespace { +using DeclCallback = There is a cuter way to use anonymous namespaces: https://llvm.org/docs/CodingStandards.html#an

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-11-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 472925. kadircet marked 5 inline comments as done. kadircet added a comment. - Fix typo - Change comment - Use Types.h Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135859/new/ https://reviews.llvm.org/D135859

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-11-03 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. The current patch looks good from my side. There is a remaining discussion around the verbosity of unittests, but I think we should not be blocked by that. Comment at: clan

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-10-28 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 471512. kadircet added a comment. - Fix typo in Ambiguous - Rebase for Types.h, introduce SymbolReference - Use comments rather than an optional parameter to indicate header-ness of main files in tests - Rather than assuming Root is in main file, check that

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-10-28 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked 6 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/AnalysisInternal.h:33 +/// Indicates the relation between the reference and the target. +enum class RefType { + /// Target is explicit from the reference,

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-10-25 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:24 +/// Indicates the relation between the reference and the target. +enum class RefType { as discussed I moved the other vocab types into t

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-10-25 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/AnalysisInternal.h:33 +/// Indicates the relation between the reference and the target. +enum class RefType { + /// Target is explicit from the reference, e.g. function call. hokei

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-10-25 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 470560. kadircet marked an inline comment as done. kadircet added a comment. - Clarify default constructor call example in RefType::Implicit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135859/new/ https://re

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-10-25 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp:107 TEST(WalkAST, DeclRef) { - testWalk("int ^x;", "int y = ^x;"); - testWalk("int ^foo();", "int y = ^foo();"); - testWalk("namespace ns { int ^x; }", "int y = ns::^x;");

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-10-25 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 470558. kadircet marked 5 inline comments as done. kadircet added a comment. - Rename Related to Ambigious in RefKind - Add more tests for implicit constructor calls - Default to Explicit as RefKind when reporting references - Don't report unused overloads, u

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-10-14 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/AnalysisInternal.h:33 +/// Indicates the relation between the reference and the target. +enum class RefType { + /// Target is explicit from the reference, e.g. function call. I'm won

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-10-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:41 - void report(SourceLocation Loc, NamedDecl *ND) { + void report(SourceLocation Loc, NamedDecl *ND, RefType RT) { if (!ND || Loc.isInvalid()) using default

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-10-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: hokein. Herald added a subscriber: mgrang. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. RefTypes are distinct categories for eac