[PATCH] D56611: [clangd] A code action to swap branches of an if statement

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clangd/refactor/tweaks/SwapIfBranches.cpp:34 +/// After: +/// if (foo) { continue; } else { return 10; } +class SwapIfBranches : public Tweak { sammccall wrote: > T

[PATCH] D56540: [clangd] Clean the cache of file statuses on vscode-clangd when clangd crashes.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clangd/clients/clangd-vscode/src/extension.ts:127 +(fileStatus) => { status.onFileUpdated(fileStatus); }); +} e

[PATCH] D56539: [clangd] Drop documentation in static index if symbols are not indexed for completion.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Just realized I missed this review, sorry about that. LGTM with a nit. Comment at: clangd/index/FileIndex.cpp:51 CollectorOpts.RefFilter = RefKind::All; -

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

ilya-biryukov added a comment. > Unfortunately I can't get ninja check-clangd to build: Looks like `clang-tools-extra` uses an old revision. Rebasing after rL350916 should do the trick. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https:

[PATCH] D56723: [CodeComplete] Propagate preferred types through parser in more cases

ilya-biryukov added a comment. Thanks for the suggestion, this should definitely work! I did struggle to figure out a way to do this without annotating every path with `enterUnknown` and failed. I'll try playing around with your idea, my initial plan is to store preferred type alongside the cur

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

ilya-biryukov added a comment. Done. I had to remove `Container::_magic` from the list of expeced symbols to make the test pass, let me know if it should actually be there. Again, thanks for the fix! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56916/new/ https:

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

ilya-biryukov added a comment. It looks like `Container::_magic` is a platform-dependent completion, I don't have it on Linux, but http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/42665 fails because it's not in the list. Submitted rL351943

[PATCH] D57093: [CodeComplete] [clangd] Fix crash on ValueDecl with a null type

ilya-biryukov created this revision. ilya-biryukov added a reviewer: kadircet. Herald added subscribers: arphaman, jkorous, MaskRay, ioeric. https://reviews.llvm.org/D57093 Files: clang-tools-extra/clangd/ExpectedTypes.cpp clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp clang/lib/S

[PATCH] D57093: [CodeComplete] [clangd] Fix crash on ValueDecl with a null type

ilya-biryukov updated this revision to Diff 183273. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Rename the test - clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57093/new/ https://reviews.llvm.org/D57093 Files: clang-tools-extra/clangd

[PATCH] D57093: [CodeComplete] [clangd] Fix crash on ValueDecl with a null type

ilya-biryukov added inline comments. Comment at: clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp:2322 +TEST(CompletionTest, CrashOnNullType) { + auto Results = completions(R"cpp( kadircet wrote: > nit: WorksWithNullType ? it doesn't seem like crashing

[PATCH] D56723: [CodeComplete] Propagate preferred types through parser in more cases

ilya-biryukov updated this revision to Diff 183297. ilya-biryukov added a comment. - Remove enterUnknown(), keep token location instead. - Inline update() and remove it CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56723/new/ https://reviews.llvm.org/D56723 Files: clang/include/clang

[PATCH] D56723: [CodeComplete] Propagate preferred types through parser in more cases

ilya-biryukov marked 3 inline comments as done. ilya-biryukov added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:373 +} +Type = QualType(); + }); kadircet wrote: > nit: maybe move this into top and get rid of the return statements inside >

[PATCH] D56723: [CodeComplete] Propagate preferred types through parser in more cases

ilya-biryukov added a comment. In D56723#1366885 , @rsmith wrote: > `ConsumeToken` is a fairly hot function; if you can avoid changes there > that'd be preferable. Done, there are no `enterUnknown` calls anymore and to avoid updating on each call to `C

[PATCH] D56723: [CodeComplete] Propagate preferred types through parser in more cases

ilya-biryukov updated this revision to Diff 183303. ilya-biryukov added a comment. - Improve a comment CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56723/new/ https://reviews.llvm.org/D56723 Files: clang/include/clang/Parse/Parser.h clang/include/clang/Sema/CodeCompleteConsumer.h

[PATCH] D56723: [CodeComplete] Propagate preferred types through parser in more cases

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:373 +} +Type = QualType(); + }); ilya-biryukov wrote: > kadircet wrote: > > nit: maybe move this into top and get rid of the re

[PATCH] D56723: [CodeComplete] Propagate preferred types through parser in more cases

ilya-biryukov updated this revision to Diff 183937. ilya-biryukov added a comment. - Use 'else if' to simplify the code CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56723/new/ https://reviews.llvm.org/D56723 Files: clang/include/clang/Parse/Parser.h clang/include/clang/Sema/CodeCo

[PATCH] D56267: [clangd] Interfaces for writing code actions

ilya-biryukov updated this revision to Diff 183949. ilya-biryukov marked 7 inline comments as done. ilya-biryukov added a comment. - s/applyCodeAction/applyTweak - Added a comment to TweakArgs - Added a unit test for sourceLocationInMainFile - Extract a 'validateRegistry' function - Do not log Can

[PATCH] D56267: [clangd] Interfaces for writing code actions

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Protocol.cpp:425 +const llvm::StringLiteral ExecuteCommandParams::CLANGD_APPLY_TWEAK = +"clangd.applyCodeAction"; + sammccall wrote: > tweak or applyTweak Done. Thanks for spotting this

[PATCH] D57223: [Tooling] Handle #pragma once header guard in include insertion.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/Tooling/Inclusions/HeaderIncludes.cpp:77 +// before/after header guards (e.g. #ifndef/#define pair, #pragma once). If no +// header guard p

[PATCH] D56723: [CodeComplete] Propagate preferred types through parser in more cases

ilya-biryukov updated this revision to Diff 184038. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Get rid of the RAII object CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56723/new/ https://reviews.llvm.org/D56723 Files: clang/include/clang/Parse/Pa

[PATCH] D56723: [CodeComplete] Propagate preferred types through parser in more cases

ilya-biryukov marked 2 inline comments as done. ilya-biryukov added inline comments. Comment at: clang/include/clang/Sema/Sema.h:319-346 +class PreferredTypeBuilder::RestoreRAII { +public: + RestoreRAII(RestoreRAII const &) = delete; + RestoreRAII &operator=(RestoreRAII const &

[PATCH] D56267: [clangd] Interfaces for writing code actions

ilya-biryukov updated this revision to Diff 184039. ilya-biryukov added a comment. - Update license headers in new files - Add an empty cpp file to avoid cmake errors due to empty inputs - clang-format - Update the 'fixits-command.test' to unbreak it (the line number was larger than the number of

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

ilya-biryukov updated this revision to Diff 184040. ilya-biryukov added a comment. - Update license header CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56610/new/ https://reviews.llvm.org/D56610 Files: clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt clang-tools-extra/clang

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

ilya-biryukov updated this revision to Diff 184041. ilya-biryukov added a comment. - Update the license header CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clang-tools-extra/clangd/SourceCode.cpp clang-tools-extra/clangd/SourceCode

[PATCH] D56612: [clangd] A code action to remove 'using namespace'

ilya-biryukov updated this revision to Diff 184043. ilya-biryukov added a comment. - Update license header CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56612/new/ https://reviews.llvm.org/D56612 Files: clang-tools-extra/clangd/AST.cpp clang-tools-extra/clangd/AST.h clang-tools-e

[PATCH] D57384: [clangd] Make -clang-tidy-checks a non-hidden command-line arg

ilya-biryukov created this revision. ilya-biryukov added a reviewer: hokein. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric. This looks like a useful user-facing configuration parameter, which should be discoverable. Also fix a small typo in the description. https://revi

[PATCH] D57509: [clangd] Append "(fix available)" to diagnostic message when fixes are present.

ilya-biryukov added a comment. Could this be made optional for VSCode? As mentioned in the discussion before, I would personally prefer to turn it off, even though I do acknowledge the need for this for some clients, e.g. vim. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D57507: [clang] Add getCommentHandler to PreambleCallbacks

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM, just a few nits Comment at: include/clang/Frontend/PrecompiledPreamble.h:286 virtual std::unique_ptr createPPCallbacks(); + /// Adds the returned Comm

[PATCH] D57509: [clangd] Append "(fix available)" to diagnostic message when fixes are present.

ilya-biryukov added a comment. In D57509#1378943 , @ioeric wrote: > Yes, but a new option seems a bit of an overkill here. The text is appended > and doesn't seem to affect the readability of original diagnostic message > much (IMO). Could you elaborat

[PATCH] D57509: [clangd] Append "(fix available)" to diagnostic message when fixes are present.

ilya-biryukov added a comment. > I don't think it's pure noise. Vscode displays diagnostics in the "PROBLEMS" > tab. A suffix allows you to tell whether fixes are available without hovering > on the errors. And it shows bulb icons when you hover over diagnostics in this dialog too. To be clear

[PATCH] D57532: [Index] Make sure c-index-test finds libc++ on Mac

ilya-biryukov created this revision. ilya-biryukov added reviewers: thakis, ioeric. Herald added a reviewer: EricWF. Herald added subscribers: llvm-commits, arphaman. Herald added a project: clang. Specifically, the version of libc++ that lives alongside the c-index-test binary. After r348365 driv

[PATCH] D57532: [Index] Make sure c-index-test finds libc++ on Mac

ilya-biryukov added a comment. I'm not sure who the owners of c-index-test are, so adding people who touched the file recently or might be interested in this landing. Suggestions for other reviewers are **very** welcome. Still struggling to make one test pass: `Index/index-file.cu` fails with:

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.h:63 +/// Turns a token range into a half-open range and checks its correctness. +/// The resulting range will have only valid source location on both sides, both sammccall wr

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

ilya-biryukov updated this revision to Diff 184588. ilya-biryukov marked 8 inline comments as done. ilya-biryukov added a comment. Herald added a project: clang. Herald added a subscriber: llvm-commits. - Remove Dummy.cpp - Add halfOpenRangeTouches - Add a comment about file vs expansion locations

[PATCH] D53926: [clangd] Only add global scope to completion query scopes for TU context.

ilya-biryukov added inline comments. Comment at: clangd/CodeComplete.cpp:563 for (auto *Context : CCContext.getVisitedContexts()) { - if (isa(Context)) + if (isa(Context)) { Info.AccessibleScopes.push_back(""); // global namespace Anonymou

[PATCH] D53866: [Preamble] Fix preamble for circular #includes

ilya-biryukov added a comment. > Because this case can be detected and handled without loosing the benefits of > the preamble. The cases where recursive includes make sense are incredibly rare in practice. Most of the time, recursively including the same file is unintentional should be conside

[PATCH] D53654: [clang] Improve ctor initializer completions.

ilya-biryukov added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:176 public: +void AddConstructorResults(const CXXRecordDecl *Record, Result R); + AddConstructorResults is not defined and not used. Remove it? Repository: rC Clang https:/

[PATCH] D53926: [clangd] Only add global scope to completion query scopes for TU context.

ilya-biryukov added inline comments. Comment at: clangd/CodeComplete.cpp:563 for (auto *Context : CCContext.getVisitedContexts()) { - if (isa(Context)) + if (isa(Context)) { Info.AccessibleScopes.push_back(""); // global namespace ioeric w

[PATCH] D53654: [clang] Improve ctor initializer completions.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Thanks for the change! Repository: rC Clang https://reviews.llvm.org/D53654 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D53941: [clangd][wip] Add 'related information' to diagnostics. Does not work for built-in notes

ilya-biryukov created this revision. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ioeric. A prototype that I made a while ago and never landed. Not suppossed to be a final solution and will probably merge-conflict now, publishing to not loose it. Repository: rC

[PATCH] D53946: [clangd] Signal when skipping the diagnostic rebuilds.

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, jfb, arphaman, jkorous, MaskRay, ioeric, javed.absar. NFC on the LSP level, only produces notifications in the C++ API. Useful for the clients of the C++ API that provide an indicat

[PATCH] D53946: [clangd] Signal when skipping the diagnostic rebuilds.

ilya-biryukov added inline comments. Comment at: clangd/TUScheduler.h:77 +class DiagnosticsResult { +public: To avoid boilerplate this could be changed to: ``` using DiagnosticsResult = Optional>; ``` Would still keep it a named typedef to allow documenting wh

[PATCH] D53946: [clangd] Signal when skipping the diagnostic rebuilds.

ilya-biryukov updated this revision to Diff 172006. ilya-biryukov added a comment. - Instead of changing the interface, added a test we can rely on Context to give us what we need Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53946 Files: unittests/clangd/ClangdTests.cpp I

[PATCH] D53946: [clangd] Test Context can be used for file status. NFC

ilya-biryukov added a comment. Added a test that the context behaves the way we want it to. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

[PATCH] D53946: [clangd] Test Context can be used for file status. NFC

ilya-biryukov updated this revision to Diff 172008. ilya-biryukov added a comment. - Fix a typo Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53946 Files: unittests/clangd/ClangdTests.cpp Index: unittests/clangd/ClangdTests.cpp ==

[PATCH] D53866: [Preamble] Fix preamble for circular #includes

ilya-biryukov added a comment. In https://reviews.llvm.org/D53866#1282582, @yvvan wrote: > @ilya-biryukov > As far as I understand the problem the same thing happens when you are in > the header a.h which includes b.h and b.h includes a.h at the same time. So > you get this recursion indirect

[PATCH] D54077: [clangd] Implemented DraftFileSystem

ilya-biryukov added subscribers: simark, klimek. ilya-biryukov added a comment. Thanks for the patch! I believe many people I talked to want this behavior (myself included). Some people like what we do now more. It feels like it depends on the workflow: for people who auto-save *all* files befo

[PATCH] D54077: [clangd] Implemented DraftFileSystem

ilya-biryukov added a comment. > There's the usual concern that the current behavior is reasonable and people > like it. I think it would be reasonable to say that a large portion of C++ users are used to the behavior that this patch introduces. Specifically, all IDEs (Clion, Eclipse, Visual St

[PATCH] D54105: [clangd] Deduplicate query scopes.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54105 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D54077: [clangd] Implemented DraftFileSystem

ilya-biryukov added a comment. In https://reviews.llvm.org/D54077#1287289, @klimek wrote: > don't most IDEs show whether a file is saved or just modified? They do, but whenever you run the build from them, they will save all modified files before actually running it. In https://reviews.llvm.o

[PATCH] D54156: [CodeComplete] Do not complete self-initializations

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added a subscriber: arphaman. Removes references to initialized variable from the following completions: int x = ^; Handles only the trivial cases where the variable name is completed immediately at the star

[PATCH] D54156: [CodeComplete] Do not complete self-initializations

ilya-biryukov updated this revision to Diff 172773. ilya-biryukov added a comment. - Remove std::move, the target is const ref, so it does nothing (thanks, clang-tidy!) Repository: rC Clang https://reviews.llvm.org/D54156 Files: lib/Sema/SemaCodeComplete.cpp test/CodeCompletion/ordinary

[PATCH] D54303: [clangd] Don't treat top-level decls as "local" if they are from the preamble.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM! Comment at: unittests/clangd/ClangdUnitTests.cpp:268 + auto AST = TU.build(); + ASSERT_EQ(AST.getLocalTopLevelDecls().size(), 1u); + NamedDecl *ND = dy

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, arphaman. Herald added a reviewer: EricWF. Herald added subscribers: kadircet, christof, ioeric. When they read compiler args from compile_commands.json. This change allows to run clang-based tools, like clang-tidy or c

[PATCH] D54311: Add a test checking clang-tidy can find libc++ on Mac

ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, arphaman. Herald added a reviewer: EricWF. Herald added a subscriber: christof. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54311 Files: test/clang-tidy/Inputs/mock-libcxx/include/c++/v1/mock_vect

[PATCH] D52273: [clangd] Initial implementation of expected types

ilya-biryukov updated this revision to Diff 173337. ilya-biryukov added a comment. - Simplify the initial implementation - Rename SType to OpaqueType Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52273 Files: clangd/CMakeLists.txt clangd/ExpectedTypes.cpp clangd/Expected

[PATCH] D52273: [clangd] Initial implementation of expected types

ilya-biryukov added a comment. I've run the measurements on a highly simplified vs the original complicated model and got roughly the same results wrt to ranking improvements, so sending a new version of the patch with highly simplified mode for the type representation. I believe there are stil

[PATCH] D52274: [clangd] Collect and store expected types in the index

ilya-biryukov updated this revision to Diff 173340. ilya-biryukov added a comment. - Rebase and update after dependent changes. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52274 Files: clangd/index/Index.cpp clangd/index/Index.h clangd/index/SymbolCollector.cpp clang

[PATCH] D52276: [clangd] Add type boosting in code completion

ilya-biryukov updated this revision to Diff 173341. ilya-biryukov added a comment. - Rebase and update wrt to dependent changes Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52276 Files: clangd/CodeComplete.cpp clangd/Quality.cpp clangd/Quality.h Index: clangd/Quality.

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

ilya-biryukov updated this revision to Diff 173364. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Update the comment. Repository: rC Clang https://reviews.llvm.org/D54310 Files: include/clang/Lex/HeaderSearchOptions.h lib/Frontend/CreateInvocationFromCo

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

ilya-biryukov added a comment. In https://reviews.llvm.org/D54310#1292924, @sammccall wrote: > If you know of a good reviewer for this, you may want a second opinion! This would definitely be nice, but I also don't know who'd be the proper person to review this. I'll take a pause until Monday,

[PATCH] D52273: [clangd] Initial implementation of expected types

ilya-biryukov added a comment. In https://reviews.llvm.org/D52273#1294767, @malaperle wrote: > What is the goal for doing this without the AST? Is the goal to not have to > keep the AST and save memory? We don't have AST for index completions. Repository: rCTE Clang Tools Extra https://re

[PATCH] D54311: Add a test checking clang-tidy can find libc++ on Mac

ilya-biryukov updated this revision to Diff 173630. ilya-biryukov added a comment. - Updated the test Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54311 Files: test/clang-tidy/Inputs/mock-libcxx/include/c++/v1/mock_vector test/clang-tidy/clang-tidy-mac-libcxx.cpp Index:

[PATCH] D54311: Add a test checking clang-tidy can find libc++ on Mac

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: test/clang-tidy/clang-tidy-mac-libcxx.cpp:11 +// Pretend clang is installed beside the mock library that we provided. +// RUN: echo '[{"directory":"%t","command":"%t/mock-libcxx/bin/c

[PATCH] D54311: Add a test checking clang-tidy can find libc++ on Mac

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: test/clang-tidy/clang-tidy-mac-libcxx.cpp:11 +// Pretend clang is installed beside the mock library that we provided. +// RUN: echo '[{"directory":"%t","command":"%t/mock-libcxx/bin/c

[PATCH] D54311: Add a test checking clang-tidy can find libc++ on Mac

ilya-biryukov updated this revision to Diff 173635. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Check for a diagnostic inside the found library Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54311 Files: test/clang-tidy/Inputs/mock-libcxx/

[PATCH] D54311: Add a test checking clang-tidy can find libc++ on Mac

ilya-biryukov updated this revision to Diff 173659. ilya-biryukov added a comment. - Check with a clang-tidy warning instead of a clang error Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54311 Files: test/clang-tidy/Inputs/mock-libcxx/include/c++/v1/mock_vector test/clang

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

ilya-biryukov updated this revision to Diff 173660. ilya-biryukov added a comment. - Added a test with a compiler path relative to the working dir Repository: rC Clang https://reviews.llvm.org/D54310 Files: include/clang/Lex/HeaderSearchOptions.h lib/Frontend/CreateInvocationFromCommandL

[PATCH] D52273: [clangd] Initial implementation of expected types

ilya-biryukov added a comment. @ioeric, thanks for the review round! Answering the most important comments, will shortly send changes to actually address the rest. Comment at: clangd/ExpectedTypes.cpp:40 + +llvm::Optional encodeType(ASTContext &Ctx, QualType T) { + assert(!T.

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

ilya-biryukov added a comment. > Before this change, this was not a problem because OverriddenFiles were keyed > on Status.getUniqueID(). Starting with this change, the key is the file path. I suggest keeping two maps for overridden files: one for existing files (key is UniqueID), another one f

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

ilya-biryukov added a comment. In https://reviews.llvm.org/D54310#1296080, @arphaman wrote: > Apologies for not seeing this earlier. No worries, thanks for the input! >> The logic to do this was based on resource dir as an approximation of >> where the compiler is installed. This broke the to

[PATCH] D54475: [clangd] Allow observation of changes to global CDBs.

ilya-biryukov added inline comments. Comment at: clangd/Function.h:90 +class Event { +public: + // A Listener is the callback through which events are delivered. I assume the `Event` is supposed to be used only with non-reference and non-const qualified types.

[PATCH] D54475: [clangd] Allow observation of changes to global CDBs.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clangd/Function.h:147 +private: + static_assert(std::is_same::type, T>::value, +"use a plain type: event values are always pas

[PATCH] D54553: [clangd] Fix crash hovering on non-decltype trailing return

ilya-biryukov added a reviewer: ilya-biryukov. ilya-biryukov added a comment. This seems ready, so putting myself in as a reviewer. Let me know if there's more work to do and I you don't want the review yet. Thanks for the fix, just a single NIT comment. Comment at: clangd/XRe

[PATCH] D52273: [clangd] Initial implementation of expected types

ilya-biryukov updated this revision to Diff 174217. ilya-biryukov marked 10 inline comments as done. ilya-biryukov added a comment. - Address comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52273 Files: clangd/CMakeLists.txt clangd/ExpectedTypes.cpp clangd/Expecte

[PATCH] D52273: [clangd] Initial implementation of expected types

ilya-biryukov marked 2 inline comments as done. ilya-biryukov added inline comments. Comment at: clangd/ExpectedTypes.cpp:12 + +static llvm::Optional toEquivClass(ASTContext &Ctx, QualType T) { + if (T.isNull() || T->isDependentType()) sammccall wrote: > returni

[PATCH] D54553: [clangd] Fix crash hovering on non-decltype trailing return

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54553 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D54630: Move detection of libc++ include dirs to Driver on MacOS

ilya-biryukov created this revision. ilya-biryukov added reviewers: JDevlieghere, arphaman. Herald added a reviewer: EricWF. Herald added subscribers: kadircet, christof, ioeric. The intention is to make the tools replaying compilations from 'compile_commands.json' (clang-tidy, clangd, etc.) find

[PATCH] D54475: [clangd] Allow observation of changes to global CDBs.

ilya-biryukov requested changes to this revision. ilya-biryukov added inline comments. This revision now requires changes to proceed. Comment at: clangd/Function.h:108 +Subscription &operator=(Subscription &&Other) { + std::tie(Parent, ListenerID) = std::tie(Other.Parent

[PATCH] D54630: Move detection of libc++ include dirs to Driver on MacOS

ilya-biryukov added a comment. In https://reviews.llvm.org/D54630#1301283, @arphaman wrote: > I don't think we want to move the logic to add a libc++ path to the driver. > `-cc1` with `-resource-dir` and `-stdlib=libc++` should still work as before. > In this case the previous patch is better,

[PATCH] D53866: [Preamble] Fix preamble for circular #includes

ilya-biryukov added a comment. In https://reviews.llvm.org/D53866#1301086, @nik wrote: > I still don't have feedback for a real world case except "unintentional > #include". Unfortunately, in real world cases the cyclic include might be not > obvious at all. > @ilya: As far as I understand you

[PATCH] D54691: [FileManager] getFile(open=true) after getFile(open=false) should open the file.

ilya-biryukov added a comment. Overall LG. The only concerning bit is that the initialization of the `UFE` is now scattered across the function body. Grouping `File` and `DeferredOpen` into a struct or adding a comment that those are initialized separately from the rest of the fields might make

[PATCH] D54691: [FileManager] getFile(open=true) after getFile(open=false) should open the file.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Thanks! LGTM Repository: rC Clang https://reviews.llvm.org/D54691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D54694: [clangd] Replay preamble #includes to clang-tidy checks.

ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:129 +class ReplayPreamble : public PPCallbacks { + const IncludeStructure &Includes; + PPCallbacks *Delegate; Maybe move fields and the private function to the end of the class? We definitel

[PATCH] D54475: [clangd] Allow observation of changes to global CDBs.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clangd/Function.h:147 +private: + static_assert(std::is_same::type, T>::value, +"use a plain type: event values are always pas

[PATCH] D54694: [clangd] Replay preamble #includes to clang-tidy checks.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D54760: [clangd] Cleanup: make the diags callback global in TUScheduler

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, javed.absar. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54760 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h cl

[PATCH] D54760: [clangd] Cleanup: make the diags callback global in TUScheduler

ilya-biryukov added a comment. The tests still model the old callbacks using callbacks, unfortunately I see no good way to test same things in any other way. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54760 ___ cfe-commits mail

[PATCH] D54746: [clangd] Respect task cancellation in TUScheduler.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clangd/TUScheduler.cpp:598 +if (!isCancelled(I->Ctx)) + continue; +// Cancelled reads are moved to the front of the queue and run

[PATCH] D54760: [clangd] Cleanup: make the diags callback global in TUScheduler

ilya-biryukov updated this revision to Diff 174914. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Merge the diagnostics callback into ParsingCallbacks - updateWithDiags(File, getInputs(File...)) -> updateWithDiags(File, ...) Repository: rCTE Clang Tools Extra

[PATCH] D54760: [clangd] Cleanup: make the diags callback global in TUScheduler

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.h:232 + /// addDocument. Used to avoid races when sending diagnostics to the clients. + static Key DocVersionKey; sammccall wrote: > I'm not sure using context here buys much: there aren't m

[PATCH] D52311: [clangd] Add support for hierarchical documentSymbol

ilya-biryukov updated this revision to Diff 174958. ilya-biryukov marked 15 inline comments as done. ilya-biryukov added a comment. - Address comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52311 Files: clangd/AST.cpp clangd/AST.h clangd/ClangdLSPServer.cpp clan

[PATCH] D52311: [clangd] Add support for hierarchical documentSymbol

ilya-biryukov added inline comments. Comment at: clangd/FindSymbols.cpp:190 + index::SymbolInfo SymInfo = index::getSymbolInfo(&ND); + SymbolKind SK = indexSymbolKindToSymbolKind(SymInfo.Kind); sammccall wrote: > may want to add a FIXME here: per the tests, i

[PATCH] D52311: [clangd] Add support for hierarchical documentSymbol

ilya-biryukov updated this revision to Diff 174960. ilya-biryukov added a comment. - Remove accidentally added qualifiers Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52311 Files: clangd/AST.cpp clangd/AST.h clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd

[PATCH] D54796: [clangd] **Prototype**: C++ API for emitting file status

ilya-biryukov added a comment. Have you considered moving **all** status updates into the TUScheduler? TUScheduler has full information about the file status changes, so it seems to most natural place to provide this kind of information. Repository: rCTE Clang Tools Extra https://reviews.llv

[PATCH] D54781: [clangd] Add 'Switch header/source' command in clangd-vscode

ilya-biryukov added a comment. Overall LG, merely stylistic NITs. Comment at: clangd/clients/clangd-vscode/src/extension.ts:69 +const docIdentifier = TextDocumentIdentifier.create(uri.toString()); +clangdClient.sendRequest(SwitchSourceHeaderRequest.type, docIde

[PATCH] D54781: [clangd] Add 'Switch header/source' command in clangd-vscode

ilya-biryukov added a comment. And many thanks for the change! I've tried it out, will definitely be one of the most-used clangd features for me :-) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54781 ___ cfe-commits mailing list

[PATCH] D54829: [clangd] Cleanup: make diagnostics callbacks from TUScheduler non-racy

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, jfb, arphaman, jkorous, MaskRay, ioeric, javed.absar. Previously, removeDoc followed by an addDoc to TUScheduler resulted in racy diagnostic responses, i.e. the old dianostics could

[PATCH] D54760: [clangd] Cleanup: make the diags callback global in TUScheduler

ilya-biryukov updated this revision to Diff 175028. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Remove accidentally added redundant 'private:' section Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54760 Files: clangd/ClangdServer.cpp cl

[PATCH] D54760: [clangd] Cleanup: make the diags callback global in TUScheduler

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:77 +FileIndex *FIndex, +llvm::unique_function)> OnDiags) { + using DiagsCallback = decltype(OnDiags); sammccall wrote: > hmm, the double-indirection for diags seems a bit funny, esp

<    5   6   7   8   9   10   11   12   13   14   >