[PATCH] D69382: [clangd] Do not insert parentheses when completing a using declaration

ilya-biryukov updated this revision to Diff 226253. ilya-biryukov added a comment. - Fix name of a constructor parameter Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69382/new/ https://reviews.llvm.org/D69382 Files: clang-tools-extra/clangd/Cod

[PATCH] D69382: [clangd] Do not insert parentheses when completing a using declaration

ilya-biryukov marked 3 inline comments as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:463 + // Check all-scopes completions too. + Opts.AllScopes = true; + Results = completions(R"cpp( kadirce

[PATCH] D69382: [clangd] Do not insert parentheses when completing a using declaration

ilya-biryukov updated this revision to Diff 226293. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Use the same CodeCompletionContext Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69382/new/ https://reviews.llvm.or

[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

ilya-biryukov accepted this revision. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:339 + +const tooling::Replacement

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:139 +// that constructor. FIXME(nridge): this should probably be handled in +// targetDecl() itself. +const CXXConstructorDecl *findCalledConstructo

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

ilya-biryukov added a comment. In D68937#1710915 , @kadircet wrote: > I totally agree that the solution you proposed would also work, but don't > think it would be any less code. Since one needs to correlate > parameters between two different declaration

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

ilya-biryukov added a comment. Ah, we're actually trying to rename parameters in the declaration to match the ones in the definition... So the try-catch blocks are not a problem, actually Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68937/new/ ht

[PATCH] D69431: [clangd] Do not highlight keywords in semantic highlighting

ilya-biryukov created this revision. ilya-biryukov added a reviewer: hokein. Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. Editors are good at highlightings the keywords themselves. Note that this only affects highlightings of builtin typ

[PATCH] D69382: [clangd] Do not insert parentheses when completing a using declaration

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1466 Output.Completions.back().Score = C.second; Output.Completions.back().CompletionTokenRange = ReplacedRange; } kadircet wrote: > why not handle `Snip

[PATCH] D69382: [clangd] Do not insert parentheses when completing a using declaration

ilya-biryukov updated this revision to Diff 226426. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Rename flag to GenerateSnippets, document it Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69382/new/ https://revie

[PATCH] D69263: [clangd] Implement cross-file rename.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:269 +cat(Features), +desc("Enable cross-file rename feature."), +init(false), Could you document that the feature is highly experimental and may lead to bro

[PATCH] D69382: [clangd] Do not insert parentheses when completing a using declaration

ilya-biryukov updated this revision to Diff 226620. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Update the comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69382/new/ https://reviews.llvm.org/D69382 Files:

[PATCH] D69382: [clangd] Do not insert parentheses when completing a using declaration

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:483 bool EnableFunctionArgSnippets; + bool CompleteArgumentList; }; kadircet wrote: > kadircet wrote: > > maybe rather use `GenerateSnippets`? Since this doesn't gener

[PATCH] D69382: [clangd] Do not insert parentheses when completing a using declaration

This revision was automatically updated to reflect the committed changes. Closed by commit rGd9971d0b2e34: [clangd] Do not insert parentheses when completing a using declaration (authored by ilya-biryukov). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D69431: [clangd] Do not highlight keywords in semantic highlighting

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:233 - bool VisitTypeLoc(TypeLoc TL) { -if (auto K = kindForType(TL.getTypePtr())) hokein wrote: > as we are not visiting this base `TypeLoc` in this patch, w

[PATCH] D69431: [clangd] Do not highlight keywords in semantic highlighting

ilya-biryukov added a comment. Thanks for the review! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69431/new/ https://reviews.llvm.org/D69431 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D69431: [clangd] Do not highlight keywords in semantic highlighting

This revision was automatically updated to reflect the committed changes. Closed by commit rGc814f4c4592c: [clangd] Do not highlight keywords in semantic highlighting (authored by ilya-biryukov). Changed prior to commit: https://reviews.llvm.org/D69431?vs=226419&id=226632#toc Repository: rG

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:278 +const auto *Target = +llvm::dyn_cast(Ref.Targets.front()->getCanonicalDecl()); +auto It = Pa

[PATCH] D69511: [clangd] Do not report anonymous entities in findExplicitReferences

ilya-biryukov created this revision. ilya-biryukov added a reviewer: hokein. Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. Otherwise every client dealing with name location should handle anonymous names in a special manner. This seems to

[PATCH] D69511: [clangd] Do not report anonymous entities in findExplicitReferences

ilya-biryukov added a comment. Stumbled upon it when trying to move semantic highlight to use `findExplicitReferences`. This will create some trouble for D68937 , but I believe it's actually worth it. `NameLoc` was never intended to point outside the name token.

[PATCH] D69511: [clangd] Do not report anonymous entities in findExplicitReferences

This revision was automatically updated to reflect the committed changes. Closed by commit rG4c430a7c6f6b: [clangd] Do not report anonymous entities in findExplicitReferences (authored by ilya-biryukov). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D69511: [clangd] Do not report anonymous entities in findExplicitReferences

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.cpp:453 + // and the clients are not prepared to handle that. + if (ND->getDeclName().isIdentifier() && + !ND->getDeclName().get

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

ilya-biryukov added a comment. D69511 broke the anonymous parameters. Sorry about that, I hope that's for the best in the long run :-) We'll need some code to update this patch. Other than that, I think this patch is good to go! Repository: rG LLVM Github Mo

[PATCH] D69506: [clangd] Add missing highlights for using decls.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:51 } + if (auto *UD = dyn_cast(D)) { +if (UD->shadow_size() == 0) Could we reuse `kindForCandidateDecls` instead? It's best to keep one way to highlight mul

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:233 +bool fillTemplateParameterMapping( +const FunctionDecl *Dest, const FunctionDecl *Source, +llvm::DenseMap &ParamToNewName) { NIT: instead of re

[PATCH] D69506: [clangd] Add missing highlights for using decls.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. See the NITs, though Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:51 } + if (auto *USD = dyn_cast(D)) +return kindForDecl(USD->ge

[PATCH] D69544: [clangd] NFC, reuse the source manager variable in the RawStringLiteral apply method

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69544/new/ https://reviews.llvm.org/D69544 _

[PATCH] D69517: [clangd] Add a hidden tweak to dump symbol under the curosr.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69517/new/ https://reviews.llvm.org/D69517 _

[PATCH] D69517: [clangd] Add a hidden tweak to dump symbol under the curosr.

ilya-biryukov added a comment. NIT: a typo in the title: s/curosr/cursor Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69517/new/ https://reviews.llvm.org/D69517 ___ cfe-commits mailing list cfe-commit

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

ilya-biryukov added a comment. Just a few final NITs from my side. And would also be nice to get tests with macros. Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:251 +} +NewName.append(SourceParam->getName()); +ParamToNewName[DestParam->getC

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1517 +template +void foo(PARAM, TYPE b, TYPE c, TYPE d = BODY(x)){} + We fail to rename the parameter in that case, right? Should the action not be availab

[PATCH] D69162: [clangd] Remove using-namespace present inside a namespace.

ilya-biryukov added a comment. The most important comment is in the tests. Is there a way to have the same effect with less changes? Comment at: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:47 const UsingDirectiveDecl *TargetDirective = nullptr; + cons

[PATCH] D80721: Updates to the 'CLion Integration' section in ClangFormat docs

This revision was automatically updated to reflect the committed changes. Closed by commit rG9982d48a92be: Updates to the 'CLion Integration' section in ClangFormat docs (authored by ilya-biryukov). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D39882: [clangd] Filter completion results by fuzzy-matching identifiers.

ilya-biryukov added a comment. We definitely need to: - Rebase this change on top of current head (to account for limits and scoring) - Set `incomplete=true` for fuzzy-matched completion results Maybe also make fuzzy-matching configurable via a flag? Off-by-default for now, so we could start te

[PATCH] D40132: [clangd] Tracing improvements

ilya-biryukov accepted this revision. ilya-biryukov added inline comments. This revision is now accepted and ready to land. Comment at: clangd/JSONRPCDispatcher.h:61 + llvm::Optional ID) + : Out(Out), ID(std::move(ID)), Tracer(new trace::Span(Method)) { +

[PATCH] D40132: [clangd] Tracing improvements

ilya-biryukov requested changes to this revision. ilya-biryukov added a comment. This revision now requires changes to proceed. Oops, accidentally accepted the revision. There are no major issues, but still wanted to learn the reasons we use rval-refs and allocate span dynamically. https://revi

[PATCH] D39571: [clangd] DidChangeConfiguration Notification

ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.cpp:250 + // Verify if path has value and is a valid path + if (Params.settings.compilationDatabasePath.hasValue()) { +CDB.setCompileCommandsDir( Replace `Settings` instead of `Params.s

[PATCH] D39836: [clangd] Drop impossible completions (unavailable or inaccessible)

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D39836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D40132: [clangd] Tracing improvements

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clangd/JSONRPCDispatcher.h:78 llvm::Optional ID; + std::unique_ptr Tracer; }; sammccall wrote: > ilya-biryukov wrote: > >

[PATCH] D40302: Avoid copying the data of in-memory preambles

ilya-biryukov updated this revision to Diff 124074. ilya-biryukov added a comment. - Fixed a typo. https://reviews.llvm.org/D40302 Files: include/clang/Frontend/PrecompiledPreamble.h lib/Frontend/PrecompiledPreamble.cpp Index: lib/Frontend/PrecompiledPreamble.cpp =

[PATCH] D40132: [clangd] Tracing improvements

ilya-biryukov added inline comments. Comment at: clangd/JSONRPCDispatcher.h:78 llvm::Optional ID; + std::unique_ptr Tracer; }; ilya-biryukov wrote: > sammccall wrote: > > ilya-biryukov wrote: > > > Why do we need `unique_ptr`? Are `Span`s non-movable? > > Sp

[PATCH] D40301: [clangd] Ensure preamble outlives the AST

ilya-biryukov updated this revision to Diff 124167. ilya-biryukov added a comment. - Moved PreambleData declaration up to avoid fwd-decl. https://reviews.llvm.org/D40301 Files: clangd/ClangdUnit.cpp clangd/ClangdUnit.h Index: clangd/ClangdUnit.h

[PATCH] D40450: [clangd] Refactoring of GlobalCompilationDatabase

ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.cpp:254 + std::move(Primary), llvm::make_unique( + ".", ArrayRef{})); +} Clangd still changes working directory when running the parsing, so `"."` might en

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

ilya-biryukov created this revision. Herald added a subscriber: mgorny. It will be used to pass around things like Logger and Tracer throughout clangd classes. https://reviews.llvm.org/D40485 Files: clangd/CMakeLists.txt clangd/Context.cpp clangd/Context.h clangd/TypedValueMap.h unitt

[PATCH] D40486: [clangd] Implemented logging using Context

ilya-biryukov created this revision. https://reviews.llvm.org/D40486 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/ClangdUnitStore.cpp clangd/ClangdUnitStore.h clangd/GlobalCompilationDatabase.cpp

[PATCH] D40488: [clangd] Implemented tracing using Context

ilya-biryukov created this revision. https://reviews.llvm.org/D40488 Files: clangd/ClangdUnit.cpp clangd/JSONRPCDispatcher.cpp clangd/JSONRPCDispatcher.h clangd/ProtocolHandlers.cpp clangd/Trace.cpp clangd/Trace.h clangd/tool/ClangdMain.cpp unittests/clangd/TraceTests.cpp Index:

[PATCH] D40489: [clangd] Changed tracing interfaces

ilya-biryukov created this revision. Introduced begin_event, end_event and instant_event instead of a string phase name. https://reviews.llvm.org/D40489 Files: clangd/Trace.cpp clangd/Trace.h Index: clangd/Trace.h === --- clan

[PATCH] D40488: [clangd] Implemented tracing using Context

ilya-biryukov planned changes to this revision. ilya-biryukov added inline comments. Comment at: clangd/JSONRPCDispatcher.h:80 + // Ctx must be before Tracer! Context Ctx; JSONOutput &Out; This is still wrong, `Context` is used by `Tracer` internally and s

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

ilya-biryukov added inline comments. Comment at: clangd/Context.h:79 +/// Otherwise returns an empty Context. +Context &globalCtx(); + bkramer wrote: > This is a giant code smell. If we want the context route, please pass > contexts everywhere. I really don't wa

[PATCH] D40489: [clangd] Changed tracing interfaces

ilya-biryukov updated this revision to Diff 124371. ilya-biryukov added a comment. - Fixed a typo. https://reviews.llvm.org/D40489 Files: clangd/Trace.cpp clangd/Trace.h Index: clangd/Trace.h === --- clangd/Trace.h +++ clangd/

[PATCH] D40489: [clangd] Changed tracing interfaces

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clangd/Trace.h:41 + /// Context. static PtrKey CtxKey; luckygeck wrote: > This is a (1)static non-pod member in an (2) interface. Is it really a good > idea? I

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

ilya-biryukov added inline comments. Comment at: clangd/Context.cpp:16 + +static Context *GlobalCtx = nullptr; +static Context EmptyContext(nullptr, {}); sammccall wrote: > Seems like it would simplify things if: > - GlobalCtx was always set (static local trick)

[PATCH] D40548: [clangd] Prototyping index support and naive index-based global code completion. WIP

ilya-biryukov added a comment. In https://reviews.llvm.org/D40548#937626, @malaperle wrote: > Hi Eric! As you might know I'm working on persisted indexing. I was wondering > which cases needed the index for code completion? Could you give a small > example? I thought the AST alone would be suff

[PATCH] D40060: [clangd] Fuzzy match scorer

ilya-biryukov added inline comments. Comment at: clangd/FuzzyMatch.cpp:118 +0x00, 0x00, 0x00, 0x00, // Control characters +0xff, 0xff, 0xff, 0xff, // Punctuation +0x55, 0x55, 0xf5, 0xff, // Numbers->Lower, more Punctuation. I'm not sure if we care, bu

[PATCH] D40562: [Sema] Ignore decls in namespaces when global decls are not wanted.

ilya-biryukov added a comment. This is how I always perceived this option in the first place, so LGTM. But maybe its intention is different, so we should wait for @arphaman's comments. Could you also update comments of `CodeCompleteConsumer::includeGlobals` and `CodeCompleteOptions::IncludeGlob

[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

ilya-biryukov added inline comments. Comment at: include/clang/Sema/CodeCompleteConsumer.h:284 + llvm::Optional ScopeSpecifier; + Maybe add a brief comment for consistency with other decls? Comment at: lib/Sema/SemaCodeComplete.cpp:4609 +

[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

ilya-biryukov added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:4609 + if (SS.isInvalid()) { +CodeCompletionContext CC(CodeCompletionContext::CCC_Name); ilya-biryukov wrote: > Why do we alter this code path? Maybe we should add a test or pro

[PATCH] D38425: [clangd] Document highlights for clangd

ilya-biryukov requested changes to this revision. ilya-biryukov added inline comments. This revision now requires changes to proceed. Comment at: clangd/ClangdLSPServer.cpp:67 ); + if (Params.rootUri && !Params.rootUri->file.empty()) malaperle wro

[PATCH] D40562: [Sema] Ignore decls in namespaces when global decls are not wanted.

ilya-biryukov added a comment. In https://reviews.llvm.org/D40562#939950, @arphaman wrote: > This change breaks cached completions for declarations in namespaces in > libclang. What exactly are you trying to achieve here? We could introduce > another flag maybe. Am I right to assume this cach

[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

ilya-biryukov added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:4609 + if (SS.isInvalid()) { +CodeCompletionContext CC(CodeCompletionContext::CCC_Name); ioeric wrote: > ilya-biryukov wrote: > > ilya-biryukov wrote: > > > Why do we alter this

[PATCH] D40596: [clangd] New conventions for JSON-marshalling functions, centralize machinery

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Thanks you very much for this patch, JSON parsing now looks better than ever! LGTM. (Just a few minor NITs). Comment at: clangd/JSONExpr.h:71 +// The convention

[PATCH] D40654: [clangd] Set completion options per-request.

ilya-biryukov created this revision. Herald added a subscriber: klimek. Previously, completion options were set per ClangdServer instance. It will allow to change completion preferences during the lifetime of a single ClangdServer instance. Also rewrote ClangdCompletionTest.CompletionOptions to r

[PATCH] D40562: [Sema] Ignore decls in namespaces when global decls are not wanted.

ilya-biryukov added a comment. In https://reviews.llvm.org/D40562#941570, @arphaman wrote: > I'm not actually 100% sure, but I would imagine that this one of the reasons, > yes. It would be nice to improve the cache to have things like > namespace-level `Decl`, although how will lookup work in

[PATCH] D40060: [clangd] Fuzzy match scorer

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clangd/FuzzyMatch.h:53 + + int PatN, WordN; // Length of pattern and word. + char Pat[MaxPat], Word[MaxWord];

[PATCH] D39882: [clangd] Filter completion results by fuzzy-matching identifiers.

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/D39882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D39430: [clangd] various fixes

ilya-biryukov added a comment. In https://reviews.llvm.org/D39430#943387, @malaperle wrote: > I'd rather keep one commit per fix or feature. I would think the clang-format > part should be one commit/review on its own. But I'm not very familiar with > the practices here so I'll let others comme

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

ilya-biryukov updated this revision to Diff 125539. ilya-biryukov added a comment. Herald added a subscriber: klimek. Addressed some review comments. - Removed global context. - Context now stores a shared_ptr to ContextData to handle lifetimes properly. - Added helper getters for some commonly u

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

ilya-biryukov marked 6 inline comments as done. ilya-biryukov added a comment. A second attempt at implementing the `Context`s. Still missing a few comments, but hopefully ready for review. Comment at: clangd/Context.h:71 +/// before any clangd functions are called. +class Glo

[PATCH] D40486: [clangd] Implemented logging using Context

ilya-biryukov updated this revision to Diff 125545. ilya-biryukov added a comment. Herald added a subscriber: klimek. - Updated to match the new Context implementation. - Logger is now stored in a global variable instead of the global Context. - Removed json-specific RequestContext, now storing al

[PATCH] D40489: [clangd] Changed tracing interfaces

ilya-biryukov planned changes to this revision. ilya-biryukov added a comment. Have to update to match the new `Context` implementation. https://reviews.llvm.org/D40489 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D40486: [clangd] Implemented logging using Context

ilya-biryukov added a comment. Additionally: - All async methods now take `Context` by-value and pass it to their `Callback`s or return them in `future`s. - All sync methods take `Context` by-ref In https://reviews.llvm.org/D40486#941210, @sammccall wrote: > This is pretty bikesheddy, but I

[PATCH] D40488: [clangd] Implemented tracing using Context

ilya-biryukov updated this revision to Diff 125690. ilya-biryukov added a comment. Herald added a subscriber: klimek. - Updated tracing to use the new Context implementation. - Restored global tracer. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40488 Files: clangd/ClangdUn

[PATCH] D40489: [clangd] Changed tracing interfaces

ilya-biryukov updated this revision to Diff 125696. ilya-biryukov added a comment. Herald added a subscriber: klimek. - Update the patch to accomodate changes from tracing and Context patches. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40489 Files: clangd/Trace.cpp clan

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

ilya-biryukov updated this revision to Diff 125712. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Removed unused helpers from Context.h - Use assert instead of llvm_unreachable in getExisiting(Key<>) Repository: rCTE Clang Tools Extra https://reviews.llvm.or

[PATCH] D40488: [clangd] Implemented tracing using Context

ilya-biryukov updated this revision to Diff 125713. ilya-biryukov added a comment. - Use getExistingKey instead of getPtr in JSONRPCDispatcher. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40488 Files: clangd/ClangdUnit.cpp clangd/JSONRPCDispatcher.cpp clangd/Trace.cpp

[PATCH] D39375: [clang] Add PPCallbacks list to preprocessor when building a preacompiled preamble.

ilya-biryukov requested changes to this revision. ilya-biryukov added inline comments. This revision now requires changes to proceed. Comment at: lib/Frontend/PrecompiledPreamble.cpp:242 std::shared_ptr PCHContainerOps, bool StoreInMemory, -PreambleCallbacks &Callbacks)

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

ilya-biryukov added inline comments. Comment at: clangd/Context.h:25 + +class ContextData { +public: sammccall wrote: > IIUC, the only reason we expose separate `Context` and `ContextData` types is > to give things like Span a stable reference to hold onto (`Con

[PATCH] D39375: [clang] Add PPCallbacks list to preprocessor when building a preacompiled preamble.

ilya-biryukov added inline comments. Comment at: lib/Frontend/PrecompiledPreamble.cpp:242 std::shared_ptr PCHContainerOps, bool StoreInMemory, -PreambleCallbacks &Callbacks) { +PreambleCallbacks &Callbacks, std::unique_ptr PPCallbacks) { assert(VFS && "VFS is nul

[PATCH] D40952: [clangd] Convert lit code completion tests to unit-tests. NFC

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: unittests/clangd/CodeCompleteTests.cpp:289 +}; +void Foo::pub() { this->^ } + )cpp"); The `^` symbol conflicts with t

[PATCH] D40489: [clangd] Changed tracing interfaces

ilya-biryukov added inline comments. Comment at: clangd/Trace.cpp:138 return; - if (!Args) -Args = llvm::make_unique(); - T->event(Ctx, "E", - Args ? json::obj{{"args", std::move(*Args)}} : json::obj{}); + assert(Args && "Args can't be null at this point");

[PATCH] D38639: [clangd] #include statements support for Open definition

ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:38 +class DelegatingPPCallbacks : public PPCallbacks { + Nebiroth wrote: > ilya-biryukov wrote: > > What's the purpose of this class? > We need to be able to use a wrapper class to be able t

[PATCH] D38425: [clangd] Document highlights for clangd

ilya-biryukov added a comment. Overall looks good, just a few minor comments. Let me know if need someone to land this patch for you after you fix those. Comment at: clangd/ClangdUnit.cpp:249 +// Don't keep the same Macro info multiple times. +// This can happen when no

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

ilya-biryukov requested changes to this revision. ilya-biryukov added a comment. This revision now requires changes to proceed. This looks useful. Could we avoid creating the `OverlayFileSystem`, though? Comment at: include/clang/Frontend/PrecompiledPreamble.h:109 + std::chr

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

ilya-biryukov added a comment. In https://reviews.llvm.org/D41005#949550, @cameron314 wrote: > It's been a while since I was in this code, but I seem to recall the file > needed to exist on disk in order to uniquely identify it (via inode). Does > this break the up-to-date check? When the fil

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

ilya-biryukov updated this revision to Diff 126315. ilya-biryukov added a comment. - Made ContextData a private member of Context. - Added clone method. - Added forgotten header guard in TypedValueMap.h - Pass Key<> by const-ref instead of by-ref. - Pass Context by-const-ref instead of by-ref. - M

[PATCH] D40486: [clangd] Implemented logging using Context

ilya-biryukov updated this revision to Diff 126316. ilya-biryukov added a comment. - Pass Context by const-ref instead of by-ref. - Don't expose global logger, it is only used in log method now. - Renamed logImpl to log. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40486 File

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

ilya-biryukov updated this revision to Diff 126317. ilya-biryukov added a comment. - Return `const Type*` instead of `Type*` in map getters. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 Files: clangd/CMakeLists.txt clangd/Context.cpp clangd/Context.h clangd/Type

[PATCH] D40489: [clangd] Changed tracing interfaces

ilya-biryukov updated this revision to Diff 126318. ilya-biryukov added a comment. - Update the patch to accomodate changes from tracing and Context patches. - Updated tracing after changes to Context. We now store a clone() of Context in Span instead of ContextData. - Pass Context by const-ref i

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

ilya-biryukov updated this revision to Diff 126319. ilya-biryukov marked 4 inline comments as done. ilya-biryukov added a comment. - Added a comment about the Parent vs Data lifetimes. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 Files: clangd/CMakeLists.txt clangd/

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

ilya-biryukov updated this revision to Diff 126320. ilya-biryukov added a comment. - Rephrase the comment. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 Files: clangd/CMakeLists.txt clangd/Context.cpp clangd/Context.h clangd/TypedValueMap.h unittests/clangd/CMa

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

ilya-biryukov updated this revision to Diff 126321. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Added r-value overload for derive(). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 Files: clangd/CMakeLists.txt clangd/Context.cpp c

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

ilya-biryukov updated this revision to Diff 126322. ilya-biryukov added a comment. - Replaced emptyCtx with Context::empty Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 Files: clangd/CMakeLists.txt clangd/Context.cpp clangd/Context.h clangd/TypedValueMap.h unit

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

ilya-biryukov added inline comments. Comment at: clangd/Context.h:11 +// Context for storing and retrieving implicit data. Useful for passing implicit +// parameters on a per-request basis. +// sammccall wrote: > This could use a bit more I think, e.g. > >

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

ilya-biryukov added inline comments. Comment at: clangd/Context.h:65 + Context *Parent; + TypedValueMap Data; +}; sammccall wrote: > ilya-biryukov wrote: > > sammccall wrote: > > > We add complexity here (implementation and conceptual) to allow multiple > > >

[PATCH] D40486: [clangd] Implemented logging using Context

ilya-biryukov updated this revision to Diff 126327. ilya-biryukov added a comment. Udpated the patch after changes in Context Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40486 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/Clan

[PATCH] D40488: [clangd] Implemented tracing using Context

ilya-biryukov updated this revision to Diff 126328. ilya-biryukov added a comment. Updated the patch after changes to Context Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40488 Files: clangd/ClangdUnit.cpp clangd/JSONRPCDispatcher.cpp clangd/Trace.cpp clangd/Trace.h

[PATCH] D40489: [clangd] Changed tracing interfaces

ilya-biryukov updated this revision to Diff 126329. ilya-biryukov added a comment. Updated the patch after changes to Context Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40489 Files: clangd/Trace.cpp clangd/Trace.h Index: clangd/Trace.h =

[PATCH] D40486: [clangd] Implemented logging using Context

ilya-biryukov updated this revision to Diff 126337. ilya-biryukov marked 15 inline comments as done. ilya-biryukov added a comment. - Copy Context in forceReparse's call to scheduleCancelRebuild. - Renamed Key<> for ID, Out and Span. - Removed the FIXME - Got rid of getExisting(RequestSpan) - Remo

[PATCH] D40486: [clangd] Implemented logging using Context

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:220 - // Note that std::future from this cleanup action is ignored. - scheduleCancelRebuild(std::move(Recreated.RemovedFile)); + // Note that std::future from this cleanup action. + // FIXME(ibiryukov)

[PATCH] D40486: [clangd] Implemented logging using Context

ilya-biryukov updated this revision to Diff 126340. ilya-biryukov added a comment. - Remove mention of globalLogger() from the comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40486 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h

<    18   19   20   21   22   23   24   25   26   27   >