[PATCH] D143436: [clangd] Move standard options adaptor to CommandMangler

2023-03-08 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin added a comment. @kadircet it is obvious that there is something in this diff that causes this hesitancy in accepting it. I'm ready to keep iterating on the solution but I need a clue what needs the improvement. Please comment. Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D143436: [clangd] Move standard options adaptor to CommandMangler

2023-03-10 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin updated this revision to Diff 504156. DmitryPolukhin marked 4 inline comments as done. DmitryPolukhin added a comment. Comments resolved Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143436/new/ https://reviews.llvm.org/D143436 File

[PATCH] D143436: [clangd] Move standard options adaptor to CommandMangler

2023-03-10 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin added a comment. @kadircet thank you for the review! Please take another look. Comment at: clang-tools-extra/clangd/CompileCommands.cpp:199 + // use/change working directory, which ExpandResponseFiles doesn't). + FS = llvm::vfs::getRealFileSystem(); +}

[PATCH] D143436: [clangd] Move standard options adaptor to CommandMangler

2023-03-10 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin updated this revision to Diff 504275. DmitryPolukhin added a comment. Fix test on Windows Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143436/new/ https://reviews.llvm.org/D143436 Files: clang-tools-extra/clangd/CompileCommands.c

[PATCH] D143436: [clangd] Move standard options adaptor to CommandMangler

2023-03-13 Thread Dmitry Polukhin via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG34de7da6246c: [clangd] Move standard options adaptor to CommandMangler (authored by DmitryPolukhin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143436/new

[PATCH] D143436: [clangd] Move standard options adaptor to CommandMangler

2023-03-13 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin added a comment. Had to revert this diff temporary due to broken ARM builds. It seems that X86 targets are disabled for that bots so tests are not passing. I'll fix upload new version for review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D143436: [clangd] Move standard options adaptor to CommandMangler

2023-03-14 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin updated this revision to Diff 505030. DmitryPolukhin added a comment. Update test to use any available target Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143436/new/ https://reviews.llvm.org/D143436 Files: clang-tools-extra/clan

[PATCH] D143436: [clangd] Apply standard adaptors to CDBs pushed from LSP

2023-02-07 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin updated this revision to Diff 495431. DmitryPolukhin added a comment. Fix clang-format sources Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143436/new/ https://reviews.llvm.org/D143436 Files: clang-tools-extra/clangd/ClangdLSPSer

[PATCH] D143436: [clangd] Apply standard adaptors to CDBs pushed from LSP

2023-02-13 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin added a comment. In D143436#4120899 , @nridge wrote: > Note that D138546 is removing the call to > `inferTargetAndDriverMode()` from `GlobalCompilationDatabase.cpp`. It adds > equivalent logic too `Comma

[PATCH] D143436: [clangd] Apply standard adaptors to CDBs pushed from LSP

2023-02-14 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin added a comment. In D143436#4122594 , @kadircet wrote: > That's actually because we model compile commands pushed via LSP as a "fixed > compilation database" rather than a json compilation database (you can check > the code in `parseFixed

[PATCH] D143436: [clangd] Apply standard adaptors to CDBs pushed from LSP

2023-02-16 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin updated this revision to Diff 498038. DmitryPolukhin added a comment. Move standard adaptors to CommandMangler Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143436/new/ https://reviews.llvm.org/D143436 Files: clang-tools-extra/cla

[PATCH] D143436: [clangd] Apply standard adaptors to CDBs pushed from LSP

2023-02-16 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin updated this revision to Diff 498138. DmitryPolukhin added a comment. Add test for expanded response files Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143436/new/ https://reviews.llvm.org/D143436 Files: clang-tools-extra/clangd/

[PATCH] D143436: [clangd] Apply standard adaptors to CDBs pushed from LSP

2023-02-16 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin added a comment. @nridge I'm sorry, I pushed version without all tests. Now I added test case for response files, it seems that clangd hasn't had it before this patch. Comment at: clang-tools-extra/clangd/CompileCommands.cpp:222 + tooling::addExpandedResponseFi

[PATCH] D143436: [clangd] Apply standard adaptors to CDBs pushed from LSP

2023-02-16 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin updated this revision to Diff 498140. DmitryPolukhin added a comment. Update test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143436/new/ https://reviews.llvm.org/D143436 Files: clang-tools-extra/clangd/CompileCommands.cpp cla

[PATCH] D143436: [clangd] Apply standard adaptors to CDBs pushed from LSP

2023-02-20 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin updated this revision to Diff 498878. DmitryPolukhin added a comment. Rebase and retest, cannot reproduce test failure locally Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143436/new/ https://reviews.llvm.org/D143436 Files: clang

[PATCH] D143436: [clangd] Apply standard adaptors to CDBs pushed from LSP

2023-02-20 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin added a comment. @nridge @kadircet please take another look, all tests now pass. As for the location of inserting driver mode please see my previous comment about MSVC CL check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143436/n

[PATCH] D143436: [clangd] Apply standard adaptors to CDBs pushed from LSP

2023-02-21 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin added inline comments. Comment at: clang-tools-extra/clangd/CompileCommands.cpp:222 + tooling::addExpandedResponseFiles(Cmd, Command.Directory, Tokenizer, *FS); + tooling::addTargetAndModeForProgramName(Cmd, Cmd.front()); auto &OptTable = clang::driver::getDri

[PATCH] D143436: [clangd] Apply standard adaptors to CDBs pushed from LSP

2023-02-22 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin updated this revision to Diff 499433. DmitryPolukhin added a comment. Move call addTargetAndModeForProgramName after SystemIncludeExtractor Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143436/new/ https://reviews.llvm.org/D143436 F

[PATCH] D143436: [clangd] Move standard options adaptor to CommandMangler

2023-02-23 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin updated this revision to Diff 499766. DmitryPolukhin added a comment. Failing tests seems to be just flacky, I was not able to reprocuce any issue with them locally in stress runs and under ASan and TSan Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D148663: [RFC][clangd] Use interpolation for CDB pushed via LSP protocol

2023-05-11 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin added a comment. In D148663#4318907 , @kadircet wrote: > I agree with Ilya's concerns here. > > We deliberately don't mess with compile flags pushed over LSP. These are > "overrides" to whatever information we have from other sources, turn

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-05-16 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin added inline comments. Comment at: clang-tools-extra/clangd/Preamble.cpp:694 Result->MainIsIncludeGuarded = CapturedInfo.isMainFileIncludeGuarded(); -return Result; +CapturedCtx.emplace(CapturedInfo.takeLife()); +return std::make_pair(Result, Captu

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-05-17 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:90 +if (Tasks) { + std::unique_lock Lock(*Barrier, std::try_to_lock); + Tasks->runAsync("task:" + Path + Version, std::move(Task)); kadircet wrote: > what's

[PATCH] D148663: [RFC][clangd] Use interpolation for CDB pushed via LSP protocol

2023-04-18 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin created this revision. DmitryPolukhin added reviewers: kadircet, nridge, sammccall, ilya-biryukov. DmitryPolukhin added a project: clang-tools-extra. Herald added a subscriber: arphaman. Herald added a project: All. DmitryPolukhin requested review of this revision. Herald added a sub

[PATCH] D148663: [RFC][clangd] Use interpolation for CDB pushed via LSP protocol

2023-04-26 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin added a comment. @kadircet @nridge friendly ping, could you please take a look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148663/new/ https://reviews.llvm.org/D148663 ___ cfe-commits m

[PATCH] D148663: [RFC][clangd] Use interpolation for CDB pushed via LSP protocol

2023-04-27 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin added a comment. In D148663#4298496 , @ilya-biryukov wrote: > I wanted to chime in and provide a bit of context. > This was a long time ago, so I might misremember, so take this with a grain > of salt. > > Idea behind pushing the CDB over

[PATCH] D148663: [RFC][clangd] Use interpolation for CDB pushed via LSP protocol

2023-04-28 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin added a comment. In D148663#4305202 , @ilya-biryukov wrote: > In D148663#4301589 , > @DmitryPolukhin wrote: > >> And, if they do so, this diff will just does nothing because there is an >> exact

[PATCH] D32428: Fix for Itanium mangler issue with templates (bug: 31405)

2017-04-24 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin added a comment. LGTM https://reviews.llvm.org/D32428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32428: Fix for Itanium mangler issue with templates (bug: 31405)

2017-05-17 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin added a comment. David, could you please take a look to this patch. It fixes potential issue and should have no side effects. If you have no objections, I'll commit it for Serge. https://reviews.llvm.org/D32428 ___ cfe-commits maili

[PATCH] D90835: [RFC][clang-tidy] Ignore diagnostics due to macro expansion from not-interested headers

2021-04-12 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin added a comment. I think there is no sense to invent another HeaderFilterRegex like option to control which headers should be excluded. HeaderFilterRegex should be good enough because if you would like to see warning from the header, most probably you can fix code in the header o

[PATCH] D90835: [RFC][clang-tidy] Ignore diagnostics due to macro expansion from not-interested headers

2021-04-19 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin updated this revision to Diff 338517. DmitryPolukhin added a comment. Put feature behind flag Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90835/new/ https://reviews.llvm.org/D90835 Files: clang-tools-extra/clang-tidy/ClangTidyDi

[PATCH] D90835: [RFC][clang-tidy] Ignore diagnostics due to macro expansion from not-interested headers

2021-04-19 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin updated this revision to Diff 338520. DmitryPolukhin added a comment. Fix wrong character Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90835/new/ https://reviews.llvm.org/D90835 Files: clang-tools-extra/clang-tidy/ClangTidyDiagno

<    1   2   3