[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo updated this revision to Diff 503335. VitaNuo added a comment. Fix formatting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143496 Files: clang-tools-extra/clangd/Config.h

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo updated this revision to Diff 503322. VitaNuo added a comment. Try another approach. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143496 Files: clang-tools-extra/clangd/Config.h

[PATCH] D143496: [clangd] Add support for missing includes analysis.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG46447e0ba2e3: Revert Revert Re-land [clangd] Add support for missing includes analysis. (authored by VitaNuo). Changed prior to commit:

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo updated this revision to Diff 503315. VitaNuo added a comment. Try to fix windows build. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143496 Files:

[PATCH] D143496: [clangd] Add support for missing includes analysis.

PiotrZSL added a comment. @VitaNuo Why did you recommit this again without any fix, breaking regression again. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143496

[PATCH] D143496: [clangd] Add support for missing includes analysis.

hokein added inline comments. Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:464 + Cfg.Diagnostics.Includes.IgnoreHeader = { + [](llvm::StringRef Header) { return Header == testPath("buzz.h"); }}; + WithContextValue Ctx(Config::Key,

[PATCH] D143496: [clangd] Add support for missing includes analysis.

thakis added a comment. Oh, that was reported a while ago already. Reverted in 2eb5ac99a76dbbf8ac68c538211fabeaa5ac0bfd for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D143496: [clangd] Add support for missing includes analysis.

thakis added a comment. This breaks tests on windows: http://45.33.8.238/win/75486/step_9.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/

[PATCH] D143496: [clangd] Add support for missing includes analysis.

PiotrZSL added a comment. This change broke regression, non-one read: "This revision was landed with ongoing or failed builds." ./ClangdTests.exe/IncludeCleaner/GenerateMissingHeaderDiags tests fails on windows Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D143496: [clangd] Add support for missing includes analysis.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG38b9fb5a129d: [clangd] Add support for missing includes analysis. (authored by VitaNuo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo updated this revision to Diff 503045. VitaNuo added a comment. Rebase to main. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143496 Files: clang-tools-extra/clangd/Config.h

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo updated this revision to Diff 503043. VitaNuo marked 3 inline comments as done. VitaNuo added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143496 Files:

[PATCH] D143496: [clangd] Add support for missing includes analysis.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks for bearing with me, let's ship it! Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:456 + tooling::IncludeStyle IncludeStyle; + auto DefaultStyle =

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo added a comment. Thanks for the comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143496 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo updated this revision to Diff 503033. VitaNuo marked 6 inline comments as done. VitaNuo added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143496 Files:

[PATCH] D143496: [clangd] Add support for missing includes analysis.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:456 + auto IncludeStyle = + clang::format::getLLVMStyle().IncludeStyle; + auto DefaultStyle = clang::format::getStyle( creating a copy of LLVM style unnecessarily all

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo added a comment. Thanks for the comments! Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:712 + for (auto *Inc : ConvertedIncludes.match(H)) { +if (Pragmas == nullptr || Pragmas->getPublic(Inc->Resolved).empty()) +

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo updated this revision to Diff 503012. VitaNuo marked 10 inline comments as done. VitaNuo added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143496 Files:

[PATCH] D143496: [clangd] Add support for missing includes analysis.

kadircet added a comment. thanks, looks great! Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:451 + tooling::HeaderIncludes HeaderIncludes(AST.tuPath(), Code, + tooling::IncludeStyle{}); + const SourceManager =

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo marked an inline comment as done. VitaNuo added a comment. Thank you for all the thoughtful comments! Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:414 + +std::string findResolvedPath(const include_cleaner::Header ) { + std::string ResolvedPath;

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo updated this revision to Diff 502084. VitaNuo marked 23 inline comments as done. VitaNuo added a comment. Improve test coverage. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143496 Files:

[PATCH] D143496: [clangd] Add support for missing includes analysis.

kadircet added a comment. thanks! looks amazing, we're missing a little bit of test coverage though Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:280 +bool isFilteredByConfig(const Config , llvm::StringRef HeaderSpelling) { + // Convert the path to Unix slashes

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo updated this revision to Diff 499896. VitaNuo added a comment. Merge upstream changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143496 Files: clang-tools-extra/clangd/Config.h

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo updated this revision to Diff 499886. VitaNuo added a comment. Upload once again. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143496 Files: clang-tools-extra/clangd/Config.h

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo added a comment. Thanks for the comments! Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:408 + +bool isFilteredByConfig(const Config , llvm::StringRef HeaderSpelling) { + for (auto : Cfg.Diagnostics.Includes.IgnoreHeader) { kadircet wrote: >

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo updated this revision to Diff 499879. VitaNuo marked 18 inline comments as done. VitaNuo added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143496 Files:

[PATCH] D143496: [clangd] Add support for missing includes analysis.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:291 } for (auto : Cfg.Diagnostics.Includes.IgnoreHeader) { // Convert the path to Unix slashes and try to match against the filter. can you also change the logic

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo added a comment. Thanks for the comments! I should have addressed everything apart from testing. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:523 +TransformedInc.Angled = WrittenRef.starts_with("<"); +if (auto FE =

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo updated this revision to Diff 499553. VitaNuo marked 24 inline comments as done. VitaNuo added a comment. Address review comments (apart from testing). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/

[PATCH] D143496: [clangd] Add support for missing includes analysis.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:359 +TransformedInc.Angled = WrittenRef.starts_with("<"); +auto FE = SM.getFileManager().getFile(Inc.Resolved); +if (!FE) unfortunately `getFile` returns an

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo added a comment. Thanks for the extensive explanations! See the updated version, it should also take the discussions on the document into account. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:512 +include_cleaner::Includes +convertIncludes(const

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo updated this revision to Diff 498401. VitaNuo marked 14 inline comments as done. VitaNuo added a comment. Herald added a subscriber: ChuanqiXu. Address review comments. Make diagnostics symref-centered and attach them to each reference. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D143496: [clangd] Add support for missing includes analysis.

kadircet added a comment. Thanks! Details mostly looks good, but i think we had different ideas about how the final diagnostics should look like. Let's try to clear that up a bit. My suggested proposal is: - Having a main diagnostic for each symbol that doesn't have a satisfying include in

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo added a comment. Thank you for the comments! I've addressed (almost) all of them. In some places, I'm not so happy about how the code has become very nested, but I don't have ideas on how to make it better atm. Comment at: clang-tools-extra/clangd/Config.h:91 +

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo updated this revision to Diff 497349. VitaNuo marked 9 inline comments as done. VitaNuo added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143496 Files:

[PATCH] D143496: [clangd] Add support for missing includes analysis.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Config.h:91 + enum class MissingIncludesPolicy { +/// Diagnose missing includes. rather than duplicating, what about renaming `UnusedIncludesPolicy` to `IncludesPolicy` and use it for

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo updated this revision to Diff 495545. VitaNuo added a comment. Add header spelling to fix suggestion. Fix range to only underline the symbol. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143496

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo updated this revision to Diff 495517. VitaNuo added a comment. Remove extra line. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143496 Files: clang-tools-extra/clangd/Config.h

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo updated this revision to Diff 495516. VitaNuo added a comment. Undo turning spellHeader method public. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143496/new/ https://reviews.llvm.org/D143496 Files: clang-tools-extra/clangd/Config.h

[PATCH] D143496: [clangd] Add support for missing includes analysis.

VitaNuo created this revision. Herald added subscribers: kadircet, arphaman. Herald added a project: All. VitaNuo requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository: rG LLVM Github Monorepo