[PATCH] D138779: [include-cleaner] Filter out references that not spelled in the main file.

hokein updated this revision to Diff 480367. hokein added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138779/new/ https://reviews.llvm.org/D138779 Files: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysi

[PATCH] D138200: [include-cleaner] Make use of locateSymbol in WalkUsed

hokein added a comment. sorry for the delay, just realize it is sitting in the review list, left some comments. Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:25 +// Gets all the providers for a symbol by tarversing each location. +llvm::SmallVector findAllHead

[PATCH] D139087: [include-cleaner] Handle base class member access from derived class.

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:68 +RecordDecl *RD = getDeclFromType(Type); +report(E->getMemberLoc(), RD); return true; nit: just `report(E->getMemberLoc(), getDeclFromTye(Type));`. =

[PATCH] D139087: [include-cleaner] Handle base class member access from derived class.

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:65 +Expr *BE = E->getBase()->IgnoreImpCasts(); +RecordDecl *RD = BE->getType()->getAsRecordDecl(); +report(E->getMemberLoc(), RD); This is not safe, we can get

[PATCH] D138821: [include-cleaner] Remove filtering from UsingDecl visit.

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:86 report(UD->getLocation(), TD, IsUsed ? RefType::Explicit : RefType::Ambiguous); } VitaNuo wrote: > kadircet wrote: > > hokein wrote: > > > we

[PATCH] D138821: [include-cleaner] Remove filtering from UsingDecl visit.

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp:130 TEST(WalkAST, Using) { - // Make sure we ignore unused overloads. + // We should report unused overloads as ambiguous. testWalk(R"cpp( nit: update the

[PATCH] D138797: [include-cleaner] Implement IWYU begin_keep/end_keep pragma support.

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. thanks, looks good. Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:312 + + size_t offsetToLineNum(llvm::Annotations MainFile, size_t Offset) { +i

[PATCH] D138821: [include-cleaner] Remove filtering from UsingDecl visit.

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:86 report(UD->getLocation(), TD, IsUsed ? RefType::Explicit : RefType::Ambiguous); } we should report all references as explicit. =

[PATCH] D138797: [include-cleaner] Implement IWYU begin_keep/end_keep pragma support.

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:312 + + size_t offsetToLineNum(llvm::Annotations MainFile, size_t Offset) { +int Count = MainFile.code().substr(0, Offset).count('\n'); this is only used in

[PATCH] D138779: [include-cleaner] Filter out references that not spelled in the main file.

hokein updated this revision to Diff 478228. hokein marked 3 inline comments as done. hokein added a comment. address comments: - polish and simplify the tests - inline the spelling-loc-check logic - add a comment for the walkUsed Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D138797: [include-cleaner] Implement IWYU begin_keep/end_keep pragma support.

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:219 +if ((Top.Block && HashLine > Top.SeenAtLine) || +Top.SeenAtLine == HashLine) { + Out->ShouldKeep.insert(HashLine); nit: remove the brace for the `if`,

[PATCH] D138782: [include-cleaner] Implement IWYU begin_keep/end_keep pragma support.

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:299 + } else if (Pragma->starts_with("end_keep")) { +InsidePragmaKeepBlock = false; + } using a simple variable is not enough to handle the nested case li

[PATCH] D138780: [include-cleaner] Merge 2 parseIWYUPragma impls in libToolingInclusions

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138780/new/ https://reviews.llvm.org/D138780

[PATCH] D138780: [include-cleaner] Minor fixes to parseIWYUPragma:

hokein added a comment. Thanks! The change is LG, I think it would be great to have some tests. Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:146 // to share the code? static llvm::Optional parseIWYUPragma(const char *Text) { + // Skip the comment start, // or

[PATCH] D138779: [include-cleaner] Filter out references that not spelled in the main file.

hokein created this revision. hokein added reviewers: kadircet, sammccall. Herald added a project: All. hokein requested review of this revision. Herald added a project: clang-tools-extra. A HTML report of gtest after this patch: https://gist.githubusercontent.com/hokein/73eee6f65a803e5702d8388c18

[PATCH] D138678: [include-cleaner] Capture private headers in PragmaIncludes.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG10d183b889da: [include-cleaner] Capture private headers in PragmaIncludes. (authored by VitaNuo, committed by hokein). Repository: rG LLVM Github

[PATCH] D138678: [include-cleaner] Capture private headers in PragmaIncludes.

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. Thanks, looks good from my side, a few nits on the test. Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:364 + EXPECT_EQ(PI.getPublic(PrivateFE.get(

[PATCH] D138676: [include-cleaner] HTMLReport shows headers that would be inserted

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. Herald added a subscriber: kadircet. nice! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138676/new/ https://reviews.llvm.org/D138676 _

[PATCH] D138678: [include-cleaner] Capture private headers in PragmaIncludes.

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:76 + /// Returns true if the given file is marked with the IWYU private pragma + /// without public mapping. + bool isPrivate(const FileEntry *File) const; -

[PATCH] D138649: [include-cleaner] Show details for #include directives (used/unused)

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. The demo is really nice! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138649/new/ https://reviews.llvm.org/D138649 ___

[PATCH] D138559: Record macro references in #ifdef clause.

This revision was automatically updated to reflect the committed changes. Closed by commit rG6ebd0aa42066: [include-cleaner] Record macro references in #ifdef clause. (authored by VitaNuo, committed by hokein). Changed prior to commit: https://reviews.llvm.org/D138559?vs=477722&id=477759#toc R

[PATCH] D138559: Record macro references in #ifdef clause.

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. Thanks, LGTM. I will commit it for you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138559/new/ https://reviews.llvm.org/D138559

[PATCH] D138559: Record macro references in #ifdef clause.

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:235 + +int main() { + #ifdef Y nit: we can get rid of the main function, it is not needed. Comment at: clang-tools-extra/include-clean

[PATCH] D138559: Record macro references in #ifdef clause.

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:87 + void Ifdef(SourceLocation Loc, const Token &MacroNameTok, + const MacroDefinition &MD) override { +if (!Active) the indentation doesn't look r

[PATCH] D138559: Record macro references in #ifdef clause.

hokein added a comment. Thanks, this code looks good to me overall. I think we can extend it to handle the `Defined`, `Elifdef`, `Elifndef` cases. And please add a `[include-cleaner]` prefix in the commit title. Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:95

[PATCH] D138219: [include-cleaner] Show includes matched by refs in HTML report.

hokein accepted this revision. hokein added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/include-cleaner/lib/HTMLReport.cpp:116 const PragmaIncludes *PI; FileID File; + const FileEntry *MainFE; nit: maybe

[PATCH] D138219: [include-cleaner] Show includes matched by refs in HTML report. Demo: https://htmlpreview.github.io/?https://gist.githubusercontent.com/sam-mccall/ecee6869e37af3db28089b64d8dce806/ra

hokein added a comment. In D138219#3946020 , @sammccall wrote: > (I do think the outstanding issues are not related to this patch, so this is > ready for review) Agree (sorry, I didn't mean we should address in this patch). We have found a few issues,

[PATCH] D138219: [include-cleaner] Show includes matched by refs in HTML report. Demo: https://htmlpreview.github.io/?https://gist.githubusercontent.com/sam-mccall/ecee6869e37af3db28089b64d8dce806/ra

hokein added a comment. >> we are missing refs in some using declarations (line 31, line32), but line >> 33 does have a link which seems weird > > They are different types of symbol (template vs function), may make a > difference? You're right, the template is the reason here. Taking a closing

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0cf888514454: [include-cleaner] Add self-contained file support for PragmaIncludes. (authored by hokein). Repository: rG LLVM Github Monorepo CHA

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:47 + } + void buildAST() { AST = std::make_unique(Inputs); } + kadircet wrote: > nit: i'd actually rename this to `astWithIncludes` and take in a set of >

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

hokein updated this revision to Diff 476425. hokein marked 3 inline comments as done. hokein added a comment. update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137698/new/ https://reviews.llvm.org/D137698 Files: clang-tools-extra/include-clea

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/CMakeLists.txt:12 LINK_LIBS clangAST mstorsjo wrote: > Note that this file changed a bit further in > 68294afa0836bb62be921e2143d147cdfdc8ba70, so you may want to rebase agai

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

hokein updated this revision to Diff 476410. hokein marked 2 inline comments as done. hokein added a comment. address comment: split tests, add a smoke test for PragmaInclude::isSelfContained. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137698/ne

[PATCH] D138219: [include-cleaner] Show includes matched by refs in HTML report. Demo: https://htmlpreview.github.io/?https://gist.githubusercontent.com/sam-mccall/ecee6869e37af3db28089b64d8dce806/ra

hokein added a comment. Thanks, I like the idea of marking missing-include refs. I haven't read the code yet, and below are all my findings when I played with the demo html (some of them are unrelated to the current patch, just want to dump all): 1. size_t shows duplicated entries, line 465 2.

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

hokein marked 2 inline comments as done. hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:39 FileID FID = SM.getFileID(Loc.physical()); -const auto *FE = SM.getFileEntryForID(FID); +const auto *FE = getResponsibleHeader(

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

hokein updated this revision to Diff 476372. hokein marked 4 inline comments as done. hokein added a comment. rebase and update based on the offline discussion Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137698/new/ https://reviews.llvm.org/D1376

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

hokein marked an inline comment as done. hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:39 FileID FID = SM.getFileID(Loc.physical()); -const auto *FE = SM.getFileEntryForID(FID); +const auto *FE = getResponsibleHeader(

[PATCH] D137677: [include-cleaner] add macro symbols and implicit refs to HTML report

hokein added a comment. The new UI looks good. Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:24 UsedSymbolCB CB) { + // This is duplicated in HTMLReport.cpp, changes should be mirrored there. tooling::stdlib::Recognizer Recognizer; --

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

hokein added a comment. In D137697#3924305 , @Hahnfeld wrote: > @hokein it seems the unit test wasn't updated for the last-minute API change, > which breaks `check-all` for me. I've posted > https://reviews.llvm.org/D137927, could you take a look if tha

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

hokein updated this revision to Diff 475067. hokein marked 3 inline comments as done. hokein added a comment. rebase and address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137698/new/ https://reviews.llvm.org/D137698 Files: clang-to

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdd46a08008f7: Move the isSelfContainedHeader function from clangd to libtooling. (authored by hokein). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

hokein updated this revision to Diff 474751. hokein marked an inline comment as done. hokein added a comment. use FileEntry in the API. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137697/new/ https://reviews.llvm.org/D137697 Files: clang-tools

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

hokein added inline comments. Comment at: clang/include/clang/Tooling/Inclusions/Header.h:21 +/// +/// A header is considered self-contained if either it has a proper header guard +/// or it doesn't has dont-include-me-similar pattern. kadircet wrote: > kadircet

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

hokein updated this revision to Diff 474735. hokein added a comment. getBufferData => getMemoryBufferForFileOrNone Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137697/new/ https://reviews.llvm.org/D137697 Files: clang-tools-extra/clangd/Headers

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

hokein updated this revision to Diff 474707. hokein marked 3 inline comments as done. hokein added a comment. address comments: - mention #import case - rename file name - API: FileEntry => FileID Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D13769

[PATCH] D137320: [include-cleaner] Initial version for the "Location=>Header" step

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG779554804813: [include-cleaner] Initial version for the "Location=>Header" step (authored by hokein). Changed prior to commit: https://reviews.llv

[PATCH] D137320: [include-cleaner] Initial version for the "Location=>Header" step

hokein updated this revision to Diff 474685. hokein added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137320/new/ https://reviews.llvm.org/D137320 Files: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysi

[PATCH] D137319: [include-cleaner] Add export IWYU pragma support.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. hokein marked an inline comment as done. Closed by commit rGf3e8a117d2bc: [include-cleaner] Add export IWYU pragma support. (authored by hokein). Changed prior to comm

[PATCH] D137319: [include-cleaner] Add export IWYU pragma support.

hokein marked 2 inline comments as done. hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:84 +Out->IWYUExportBy[IncludedHeader->getUniqueID()].push_back( +save(SM.getFileEntryForID(Top.SeenAtFile)->tryGetRealPathName()

[PATCH] D137319: [include-cleaner] Add export IWYU pragma support.

hokein updated this revision to Diff 474505. hokein marked 8 inline comments as done. hokein added a comment. address remaining comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137319/new/ https://reviews.llvm.org/D137319 Files: clang-too

[PATCH] D137677: [include-cleaner] add macro symbols and implicit refs to HTML report

hokein accepted this revision. hokein added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/include-cleaner/lib/HTMLReport.cpp:243 +for (Header H : T.Providers) { + OS << "Candidate: "; + escapeString(llvm::to_string(H

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

hokein created this revision. hokein added a reviewer: kadircet. Herald added a project: All. hokein requested review of this revision. Herald added a project: clang-tools-extra. Based on https://reviews.llvm.org/D137697 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D137698 Fi

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

hokein created this revision. hokein added reviewers: kadircet, sammccall. Herald added a subscriber: arphaman. Herald added a project: All. hokein requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. Herald added projects: clang, clang-tools-extra. We plan to reuse

[PATCH] D137252: [include-cleaner] Testing helpers for ast walking

hokein added a comment. In D137252#3907743 , @kadircet wrote: > In D137252#3907665 , @hokein wrote: > >> Just share my experience when reading the code, overall I have a feeling >> that we're regressing the code

[PATCH] D137320: [include-cleaner] Initial version for the "Location=>Header" step

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:58 +Results = {{FE}}; +// FIXME: compute a closure of exporter headers. +for (const auto *Export : PI.getExporters(FE, SM.getFileManager())) kadircet wrote: >

[PATCH] D137320: [include-cleaner] Initial version for the "Location=>Header" step

hokein updated this revision to Diff 473185. hokein marked 3 inline comments as done. hokein added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137320/new/ https://reviews.llvm.org/D137320 Files: clang-tools-extra/in

[PATCH] D137252: [include-cleaner] Testing helpers for ast walking

hokein added a comment. Just share my experience when reading the code, overall I have a feeling that we're regressing the code readability of the tests (left some comments on regressing samples): 1. with the sugared macros, the syntax of `TargetCode` and `ReferenceCode` are identical -- both

[PATCH] D137319: [include-cleaner] Add export IWYU pragma support.

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:82 + llvm::DenseMap> + IWYUExportBy; kadircet wrote: > kadircet wrote: > > what about a smallvector, instead of a denseset here? > nit: inste

[PATCH] D137319: [include-cleaner] Add export IWYU pragma support.

hokein updated this revision to Diff 473174. hokein marked 7 inline comments as done. hokein added a comment. Herald added a subscriber: mgrang. address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137319/new/ https://reviews.llvm.

[PATCH] D137320: [include-cleaner] Initial version for the "Location=>Header" step

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:63 +// FIXME: use a real Class! +using SymbolLocation = std::variant; + kadircet wrote: > let's move this to `AnalysisInternal.h`, as it isn't used

[PATCH] D137320: [include-cleaner] Initial version for the "Location=>Header" step

hokein updated this revision to Diff 472911. hokein marked 6 inline comments as done. hokein added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137320/new/ https://reviews.llvm.org/D137320 Files: clang-tools-extra/in

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

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] D137320: [include-cleaner] Initial version for the "Location=>Header" step

hokein created this revision. hokein added reviewers: kadircet, sammccall. Herald added a project: All. hokein requested review of this revision. Herald added a project: clang-tools-extra. This patch implements the initial version of "Location => Header" step: - define the interface; - integrate

[PATCH] D137319: [include-cleaner] Add export IWYU pragma support.

hokein created this revision. hokein added reviewers: kadircet, sammccall. Herald added a project: All. hokein requested review of this revision. Herald added a project: clang-tools-extra. - add support to PragmaIncludes to handle IWYU export/begin_exports/end_exports pragma; - implement an API t

[PATCH] D136071: [include-cleaner] Add PragmaIncludes which handles include-mapping pragmas.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9ab0827f70a5: [include-cleaner] Add a data-structure to capture IWYU pragmas. (authored by hokein). Changed prior to commit: https://reviews.llvm.

[PATCH] D136071: [include-cleaner] Add PragmaIncludes which handles include-mapping pragmas.

hokein updated this revision to Diff 471995. hokein marked 2 inline comments as done. hokein added a comment. address comments around tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136071/new/ https://reviews.llvm.org/D136071 Files: clang-

[PATCH] D137063: [clangd] Run semantic highligting in clangd check.

This revision was automatically updated to reflect the committed changes. hokein marked an inline comment as done. Closed by commit rGe3ec9dd0ba42: [clangd] Run semantic highligting in clangd check. (authored by hokein). Changed prior to commit: https://reviews.llvm.org/D137063?vs=471951&id=471

[PATCH] D137063: [clangd] Run semantic highligting in clangd check.

hokein marked an inline comment as done. hokein added inline comments. Comment at: clang-tools-extra/clangd/tool/Check.cpp:213 +auto Highlights = getSemanticHighlightings(*AST); +if (LineRange) { + for (const auto HL : Highlights) { kadircet wrote: >

[PATCH] D137063: [clangd] Run semantic highligting in clangd check.

hokein updated this revision to Diff 471951. hokein marked an inline comment as done. hokein added a comment. add line-range support Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137063/new/ https://reviews.llvm.org/D137063 Files: clang-tools-ex

[PATCH] D137064: [clangd] Fix a semantic-highlighting crash.

This revision was automatically updated to reflect the committed changes. hokein marked 2 inline comments as done. Closed by commit rG1258747a59db: [clangd] Fix a semantic-highlighting crash. (authored by hokein). Changed prior to commit: https://reviews.llvm.org/D137064?vs=471930&id=471947#toc

[PATCH] D136951: [clangd] Populate ranges and symbol origin for paramname completions

hokein accepted this revision. hokein added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:2019 + // Point back to cursor location. + Offset += 2; + CompletionRange.start = offsetToPosition(ParseInput.Con

[PATCH] D136071: [include-cleaner] WIP: Add PragmaIncludes which handles include-mapping pragmas.

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:41 +// spelling header rather than the header directly defines the symbol. +class PragmaIncludes { +public: kadircet wrote: > i think this interfac

[PATCH] D136071: [include-cleaner] WIP: Add PragmaIncludes which handles include-mapping pragmas.

hokein updated this revision to Diff 471936. hokein marked 6 inline comments as done. hokein added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136071/new/ https://reviews.llvm.org/D136071 Files: clang-tools-extra/in

[PATCH] D137064: [clangd] Fix a semantic-highlighting crash.

hokein created this revision. hokein added a reviewer: kadircet. Herald added a subscriber: arphaman. Herald added a project: All. hokein requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository: rG LLVM Github Mon

[PATCH] D137063: [clangd] Run semantic highligting in clangd check.

hokein created this revision. hokein added a reviewer: kadircet. Herald added a subscriber: arphaman. Herald added a project: All. hokein requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Allowing us to test this feature

[PATCH] D136567: [clangd] Avoid hanging in Selection when PP corrects the token sequence.

hokein accepted this revision. hokein added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/Selection.cpp:316 // return the eof token. if (ExpandedTokens.back().kind() == tok::eof) ExpandedTokens = ExpandedTo

[PATCH] D136564: [clang] Instantiate NTTPs and template default arguments with sugar

hokein added a comment. In D136564#3890676 , @rtrieu wrote: > I noticed some build failures after your commit. I'm trying to get a > standalone reproducer. Here is the broken testcase: // -std=c++17 #include template using FixedTypeT = Ty

[PATCH] D136723: [include-cleaner] Record main-file macro occurences and includes

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. looks good from my side. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:52 struct Symbol { enum Kind { // A canonical clang dec

[PATCH] D136782: [include-cleaner] Move the writeHTMLReport to the public header, NFC.

hokein created this revision. hokein added a reviewer: sammccall. Herald added a project: All. hokein requested review of this revision. Herald added a project: clang-tools-extra. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D136782 Files: clang-tools-extra/include-cleaner/in

[PATCH] D136539: [Lex] Bring back the magic number 50 in updateConsecutiveMacroArgTokens.

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG11c1d8b7fd82: [Lex] Bring back the magic number 50 in updateConsec

[PATCH] D136539: [Lex] Bring back the magic number 50 in updateConsecutiveMacroArgTokens.

hokein updated this revision to Diff 470764. hokein added a comment. upload the full reland version. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136539/new/ https://reviews.llvm.org/D136539 Files: clang/lib/Lex/TokenLexer.cpp clang/test/Lexe

[PATCH] D136723: [include-cleaner] Record main-file macro occurences and includes

hokein added a comment. Left a few initial comments, it looks roughly good to me (for the macro-usage case, I might miss some historical context there, I think we come to an agreement that what this patch proposes is the designed behavior). Comment at: clang-tools-extra/incl

[PATCH] D136539: [Lex] Bring back the magic number 50 in updateConsecutiveMacroArgTokens.

hokein added a comment. In D136539#3882922 , @sammccall wrote: > In D136539#3882316 , @hokein wrote: > >>> This is a subtle change that needs careful review, and honestly should have >>> a test. >> >> I agree, an

[PATCH] D136539: [Lex] Bring back the magic number 50 in updateConsecutiveMacroArgTokens.

hokein updated this revision to Diff 470599. hokein marked an inline comment as not done. hokein added a comment. refine the test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136539/new/ https://reviews.llvm.org/D136539 Files: clang/lib/Lex/To

[PATCH] D136539: [Lex] Bring back the magic number 50 in updateConsecutiveMacroArgTokens.

hokein added a comment. > This is a subtle change that needs careful review, and honestly should have a > test. I agree, and thanks for the nice review comments! I'd like to add a unittest, but I don't see a straight way (I manually test it by comparing the number of allocated SLocEntries and

[PATCH] D136539: [Lex] Bring back the magic number 50 in updateConsecutiveMacroArgTokens.

hokein updated this revision to Diff 470457. hokein marked 4 inline comments as done. hokein added a comment. address comments and add a stress test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136539/new/ https://reviews.llvm.org/D136539 Files:

[PATCH] D136539: [Lex] Bring back the magic number 50 in updateConsecutiveMacroArgTokens.

hokein created this revision. hokein added a reviewer: sammccall. Herald added a project: All. hokein requested review of this revision. Herald added a project: clang. The magic number 50 was removed in D134942 , as a behavior change for performance reason. Whil

[PATCH] D136293: [IncludeCleaner] Add public API

hokein accepted this revision. hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:7 +// +//===--===// + would be nice to have so

[PATCH] D136071: [include-cleaner] WIP: Add PragmaIncludes which handles include-mapping pragmas.

hokein added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Hooks.h:38 +// spelling header rather than the header directly defines the symbol. +class PragmaIncludes { +public: sammccall wrote: > sammccall wrote: > >

[PATCH] D136071: [include-cleaner] WIP: Add PragmaIncludes which handles include-mapping pragmas.

hokein updated this revision to Diff 468688. hokein marked 4 inline comments as done. hokein added a comment. - rebase - address comments - implement shouldKeep - add unittests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136071/new/ https://revie

[PATCH] D135956: [include-cleaner] Add include-cleaner tool, with initial HTML report

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. It looks good to me. As discussed offline, would be nice to show line number for each line of code in the html dump. Comment at: clang-tools-extra/include-cleaner/lib/HTMLR

[PATCH] D136071: [include-cleaner] WIP: Add PragmaIncludes which handles include-mapping pragmas.

hokein added a comment. Post it early in order to get initial feedback on the APIs.It is not finished, I plan to implement the IWYU keep pragma and add unittest. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136071/new/ https://reviews.llvm.org/D1

[PATCH] D136071: [include-cleaner] WIP: Add PragmaIncludes which handles include-mapping pragmas.

hokein created this revision. hokein added reviewers: sammccall, kadircet. Herald added a project: All. hokein requested review of this revision. Herald added a project: clang-tools-extra. PragmaIncludes captures the progma-based header-mapping information, it is used in the "Location => Header" s

[PATCH] D135956: [include-cleaner] Add include-cleaner tool, with initial HTML report

hokein added a comment. +1, it looks a great start to me (left some initial comments). The plan also looks good to me. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Hooks.h:1 +//===--- Hooks.h - Record compiler events --

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

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] D135696: [pseudo] Document disambiguation design progress

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. Thanks, the doc looks great, it covers everything we've discussed so far. In D135696#3850044 , @sammccall wrote: > The diagrams are rendered by github

[PATCH] D135857: [clangd] Fix a crash in ExtractFunction tweak.

This revision was automatically updated to reflect the committed changes. Closed by commit rG7ac10e5b347f: [clangd] Fix a crash in ExtractFunction tweak. (authored by hokein). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135857/new/ https://reviews

[PATCH] D135857: [clangd] Fix a crash in ExtractFunction tweak.

hokein created this revision. hokein added a reviewer: kadircet. Herald added a subscriber: arphaman. Herald added a project: All. hokein requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository: rG LLVM Github Mon

[PATCH] D135440: [SourceManager] Speedup getFileIDLocal with a separate Offset Table.

hokein added a comment. In D135440#3847720 , @nickdesaulniers wrote: > Is it worth it and possible to fully decompose `LocalSLocEntryTable` into > arrays of its constituent parts, and only construct a `SLocEntry` when > necessary? Fully decomposing `L

[PATCH] D135440: [SourceManager] Speedup getFileIDLocal with a separate Offset Table.

hokein updated this revision to Diff 466729. hokein marked an inline comment as done. hokein added a comment. add assert Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135440/new/ https://reviews.llvm.org/D135440 Files: clang/include/clang/Basic/

<    1   2   3   4   5   6   7   8   9   10   >