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

ilya-biryukov added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:5101 +const auto *ND = Base.getType()->getAsCXXRecordDecl(); +if (isa(ND) || +isa(ND)) { kadircet wrote: > kadircet wrote: > > ilya-biryukov wrote: > > > Why special-ca

[PATCH] D53687: [clangd] Make in-memory CDB always available as an overlay, refactor.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clangd/ClangdLSPServer.cpp:670 + /*Output=*/""); + if (Old != New) +CDB->setCompileCommand(File, std::move(New)); ---

[PATCH] D53688: [clangd] Add fallbackFlags initialization extension.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D53688#1275793, @sammccall wrote: > It's hard to reason about UX outside of concrete tools - the only use I know > of (atom-ide-cpp) never needs to ch

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

ilya-biryukov added inline comments. Comment at: include/clang/Sema/CodeCompleteConsumer.h:767 + /// When Kind == RK_Declaration and a FieldDecl has been passed as + /// Declaration, this will hold the identifiers name. To be used later on when + /// generating constructors f

[PATCH] D53651: [clangd] Use thread pool for background indexing.

ilya-biryukov added a comment. Update from the offline meeting: decided to start with `llvm::ThreadPool` for background tasks and lower thread priorities for background tasks. Comment at: clangd/index/Background.cpp:89 } - QueueCV.notify_all(); + QueueCV.notify_one(); }

[PATCH] D53651: [clangd] Use thread pool for background indexing.

ilya-biryukov added inline comments. Comment at: clangd/Threading.cpp:101 +void setThreadPriority(std::thread &T, ThreadPriority Priority) { +#ifdef HAVE_PTHREAD_H Maybe put this helper into `llvm/Support/Threading.h`? Comment at: clangd/Thre

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

ilya-biryukov added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:813 +static void AddTemplateParameterChunks(ASTContext &Context, + const PrintingPolicy &Policy, We don't seem to need this fwd-decl anymore. Re

[PATCH] D53648: [clangd] Only log ignored diagnostics with -log=verbose.

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

[PATCH] D53648: [clangd] Only log ignored diagnostics with -log=verbose.

ilya-biryukov added a comment. Sorry for the delay, it was open in my browser for days... Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53648 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

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

ilya-biryukov added a comment. Mostly NITs Comment at: lib/Sema/SemaCodeComplete.cpp:5136 + auto AddDefaultCtorInit = [&](const char *TypedName, +const char *TypeName, +const NamedDecl* ND) { kadi

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

ilya-biryukov added a comment. Why does resetting the conditional stack is the right thing to do here? I can see how it can hide the problem, but can't come up with with a consistent model to handle the fact that the file contents were trimmed. Failing to build the preamble in that case seems li

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

ilya-biryukov updated this revision to Diff 181309. ilya-biryukov added a comment. - Put more AST-centric information into ActionInputs - Restructure and refactor the code Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56267/new/ https://reviews.llvm

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

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, mgorny. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D56610 Files: clangd/CMakeLists.txt clangd/refactor/actions/QualifyNam

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

ilya-biryukov updated this revision to Diff 181311. ilya-biryukov added a comment. - Add the code to actually instantiate a code action Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56610/new/ https://reviews.llvm.org/D56610 Files: clangd/CMakeLi

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

ilya-biryukov updated this revision to Diff 181312. ilya-biryukov added a comment. - Add some forgotten helpers Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56610/new/ https://reviews.llvm.org/D56610 Files: clangd/CMakeLists.txt clangd/CodeAct

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

ilya-biryukov added a comment. This is a somewhat simple action to illustrate the use of code action APIs. Still missing tests and trying to figure out what information we want to expose in order to avoid walking over ASTs in each of the actions, so this is not final. Should be a good reference

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

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, mgorny. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D56611 Files: clangd/CMakeLists.txt clangd/CodeActions.cpp clangd/re

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

ilya-biryukov added a comment. A deliberately simple syntactic transformation. Missing tests, but should work very reliably. To serve as an reference point for writing similar actions. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https:

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

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, jfb, arphaman, mgrang, jkorous, MaskRay, ioeric, mgorny. Only available in the source files to fit into the model of single-file actions. Doing the same in headers would require mor

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

ilya-biryukov planned changes to this revision. ilya-biryukov added a comment. The most complicated of the sample actions, requires considerable work and thorough testing before it can be landed. Serves the purpose of illustrating how to write the two-stage actions. Repository: rCTE Clang Too

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

ilya-biryukov updated this revision to Diff 181321. ilya-biryukov added a comment. - Remove 'using namespace llvm' Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56267/new/ https://reviews.llvm.org/D56267 Files: clangd/CMakeLists.txt clangd/Clan

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

ilya-biryukov added a comment. I've added a few sample actions, please take a look. The major thing that's missing from the design is how we can define an interface for actions to get the nodes they interested in from the AST without doing an AST traversal in each of the actions separately. I h

[PATCH] D56592: [clangd] Do not override contents of the shards without modification

ilya-biryukov added inline comments. Comment at: clangd/index/Background.cpp:309 for (const auto &I : *Index.Sources) { +// We already have the map from uris to absolutepaths in the cache, +// therefore traverse Index.Sources rather than Files to get rid of absolute -

[PATCH] D56656: [clangd] Fix a reference invalidation

ilya-biryukov added inline comments. Comment at: clangd/index/Background.cpp:517 +// well. +CurDependencyPath = Dependencies[CurrentDependency].Path; + } We should avoid modifying the container we're iterating over instead. I suggest we separ

[PATCH] D56656: [clangd] Fix a reference invalidation

ilya-biryukov added inline comments. Comment at: clangd/index/Background.cpp:488 +Dependencies.push_back(std::move(ToVisit.front())); +auto &CurDependency = Dependencies.back(); +ToVisit.pop(); This reference makes it just as easy to access the vector

[PATCH] D56656: [clangd] Fix a reference invalidation

ilya-biryukov added inline comments. Comment at: clangd/index/Background.cpp:488 +Dependencies.push_back(std::move(ToVisit.front())); +auto &CurDependency = Dependencies.back(); +ToVisit.pop(); ilya-biryukov wrote: > This reference makes it just as ea

[PATCH] D56656: [clangd] Fix a reference invalidation

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM, but please add a comment the vector is not modified (it might be useful as a reminder in case we want to change the code later) Repository: rCTE Clang Tools Extra CHANG

[PATCH] D55256: [clangd] Support clang-tidy configuration in clangd.

ilya-biryukov added a comment. Just a small comment wrt to a particular change in `ClangdLSPServer`. I haven't looked at the patch more closely, though. Comment at: clangd/ClangdLSPServer.h:132 - RealFileSystemProvider FSProvider; /// Options used for code completion ---

[PATCH] D55250: [clangd] Enhance macro hover to see full definition

ilya-biryukov added inline comments. Comment at: clangd/XRefs.cpp:578 +bool Invalid; +StringRef Buffer = SM.getBufferData(SM.getFileID(StartLoc), &Invalid); +if (!Invalid) { malaperle wrote: > ilya-biryukov wrote: > > Unfortunately we can't get the bu

[PATCH] D55250: [clangd] Enhance macro hover to see full definition

ilya-biryukov added inline comments. Comment at: clangd/XRefs.cpp:578 +bool Invalid; +StringRef Buffer = SM.getBufferData(SM.getFileID(StartLoc), &Invalid); +if (!Invalid) { ilya-biryukov wrote: > malaperle wrote: > > ilya-biryukov wrote: > > > Unfort

[PATCH] D55250: [clangd] Enhance macro hover to see full definition

ilya-biryukov added inline comments. Comment at: clangd/XRefs.cpp:578 +bool Invalid; +StringRef Buffer = SM.getBufferData(SM.getFileID(StartLoc), &Invalid); +if (!Invalid) { ilya-biryukov wrote: > ilya-biryukov wrote: > > malaperle wrote: > > > ilya-b

[PATCH] D56592: [clangd] Fix updated file detection logic in indexing

ilya-biryukov added a comment. This looks neat, thanks for the change! Just a few NITs from my side Comment at: clangd/index/Background.cpp:264 +/// Given index results from a TU, only update symbols coming from files with +/// different digests than \p DigestsSnapshot. Also st

[PATCH] D56592: [clangd] Fix updated file detection logic in indexing

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM after the NITs are fixed. Unless there are other significant changes planned that I need to look at. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https

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

ilya-biryukov added inline comments. Comment at: clangd/refactor/actions/SwapIfBranches.cpp:36 + + bool VisitIfStmt(IfStmt *If) { +auto R = toHalfOpenFileRange(Ctx.getSourceManager(), Ctx.getLangOpts(), jkorous wrote: > It seems to me we don't find If token

[PATCH] D56680: [Tooling] Make clang-tool find libc++ dir on mac when running on a file without compilation database.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56680/new/ https://reviews.llvm.org/D56680 ___

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

ilya-biryukov created this revision. ilya-biryukov added reviewers: kadircet, rsmith. Preferred types are used by code completion for ranking. This commit considerably increases the number of points in code where those types are propagated. In order to avoid complicating signatures of Parser's me

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

ilya-biryukov added a comment. @kadircet for preferred-type related bits. @rsmith for parser-related bits. More specifically, (1) should we be worried that this might affect parser performance on the hot path and (2) whether there are better alternatives to do something like this. Repository:

[PATCH] D56718: [clangd] Update docs to mention YCM integration and new LSP features

ilya-biryukov added inline comments. Comment at: docs/clangd.rst:125 + +One of the options of using :program:`Clangd` in :program:`vim` is to utilize +`YouCompleteMe Given how little information we actually provide for each option here (it's literally just point

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

ilya-biryukov added a comment. Will take a look. For reference: the particular failing test was added in r351222 (D56680 ), not in this change. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54630/new/ https://reviews.llvm.org/D54

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

ilya-biryukov added a comment. r351334 should fix this. However, this is an indication that libc++ living alongside the compiler (i.e. in `/bin/../include/c++/v1`) cannot be found on PS4. I am not familiar with PS4 myself, but I believe it would be reasonable to assume finding libc++ living alon

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

ilya-biryukov updated this revision to Diff 182078. ilya-biryukov added a comment. Herald added a subscriber: mgrang. - Rename ActionProvider to Tweak - Update an interface of Tweak - ActionInputs -> Tweak::Selection - Remove CodeAction.h, use tweak registry instead Repository: rCTE Clang Tool

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

ilya-biryukov updated this revision to Diff 182079. ilya-biryukov added a comment. - Update after changes to parent revision Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56610/new/ https://reviews.llvm.org/D56610 Files: clangd/CMakeLists.txt c

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

ilya-biryukov updated this revision to Diff 182080. ilya-biryukov added a comment. - Update after changes to parent revision Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clangd/CMakeLists.txt c

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

ilya-biryukov updated this revision to Diff 182082. ilya-biryukov added a comment. - Update after changes to parent revision Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56612/new/ https://reviews.llvm.org/D56612 Files: clangd/AST.cpp clangd/A

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

ilya-biryukov added inline comments. Comment at: clangd/SourceCode.h:64 +/// 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 jkorous wrote: > It seems to m

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

ilya-biryukov updated this revision to Diff 182102. ilya-biryukov added a comment. - Fix a typo in the id of the SwapIfBranches Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clangd/CMakeLists.txt

[PATCH] D56841: [clangd] Filter out plugin related flags.

ilya-biryukov added subscribers: sammccall, klimek. ilya-biryukov added a comment. LG, just a few comments. Supporting compiler plugins in clangd is non-trivial and we don't support them so far, so filtering out the options make sense. It feels these helper belong in the Tooling library, that wo

[PATCH] D56841: [clangd] Filter out plugin related flags.

ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:429 + const auto &CommandLine = Inputs.CompileCommand.CommandLine; + for (size_t I = 0, E = CommandLine.size(); I != E; I++) { +// According to https://clang.llvm.org/docs/ClangPlugins.html -

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

ilya-biryukov added a comment. Thanks for a quick response, @kadircet! Leaving some first comments, will address the rest later. In D56723#1361366 , @kadircet wrote: > One general comment: I am not really sure if the handling done in > `ParseCastExpressi

[PATCH] D56841: [clangd] Filter out plugin related flags.

ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:429 + const auto &CommandLine = Inputs.CompileCommand.CommandLine; + for (size_t I = 0, E = CommandLine.size(); I != E; I++) { +// According to https://clang.llvm.org/docs/ClangPlugins.html -

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

ilya-biryukov updated this revision to Diff 182272. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Build tweaks as an object library. To make sure linker does not optimize away global constructors. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION

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

ilya-biryukov updated this revision to Diff 182273. ilya-biryukov added a comment. - Rebase after parent change Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56610/new/ https://reviews.llvm.org/D56610 Files: clangd/SourceCode.cpp clangd/SourceC

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

ilya-biryukov updated this revision to Diff 182274. ilya-biryukov added a comment. - Rebase after parent change Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clangd/refactor/tweaks/CMakeLists.txt

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

ilya-biryukov updated this revision to Diff 182276. ilya-biryukov added a comment. - Rebase after parent change Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56612/new/ https://reviews.llvm.org/D56612 Files: clangd/AST.cpp clangd/AST.h clangd

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

ilya-biryukov added a comment. Haven't yet addressed all the comments, but switched to use the "object library" (i.e. a collection of .o files) to make sure linker does not optimize away global ctors required by registry. Comment at: clangd/refactor/Tweak.cpp:38 +namespace {

[PATCH] D56841: [clangd] Filter out plugin related flags.

ilya-biryukov added a comment. In D56841#1361492 , @klimek wrote: > (I know, I know, I'm not a big help) That's fine, mostly wanted to hear your opinion :-) > That said, if we can find a nice abstraction to pull out, I'm generally in > favor :) Eric s

[PATCH] D56856: [tooling] Add a new argument adjuster for deleting plugin related command line args

ilya-biryukov added inline comments. Comment at: include/clang/Tooling/ArgumentsAdjusters.h:69 +/// Gets an argument adjuster which strips plugin related command line +/// arguments. Maybe put the function before `combineAdjusters`? The latter is a function tha

[PATCH] D56856: [tooling] Add a new argument adjuster for deleting plugin related command line args

ilya-biryukov added a comment. Just a few NITs from my side, LG when fixed. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56856/new/ https://reviews.llvm.org/D56856 ___ cfe-commits mailing list cfe-commits@lists.llvm

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

ilya-biryukov updated this revision to Diff 182315. ilya-biryukov marked 9 inline comments as done. ilya-biryukov added a comment. - Remove intermediate StringMap, use registry directly - Rename codeAction to codeTweak in ClangdServer - Rename a helper to get position to sourceLocationInMainFile -

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

ilya-biryukov updated this revision to Diff 182316. ilya-biryukov added a comment. - Update to reflect changes in parent revision Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56610/new/ https://reviews.llvm.org/D56610 Files: clangd/SourceCode.cp

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

ilya-biryukov updated this revision to Diff 182317. ilya-biryukov added a comment. - Update to reflect changes in parent revision Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clangd/refactor/twea

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

ilya-biryukov updated this revision to Diff 182318. ilya-biryukov added a comment. - Update to reflect changes in parent revision Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56612/new/ https://reviews.llvm.org/D56612 Files: clangd/AST.cpp cla

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

ilya-biryukov updated this revision to Diff 182320. ilya-biryukov added a comment. - Remove the header file Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56610/new/ https://reviews.llvm.org/D56610 Files: clangd/SourceCode.cpp clangd/SourceCode.

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

ilya-biryukov updated this revision to Diff 182322. ilya-biryukov added a comment. - Remove the header file Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56612/new/ https://reviews.llvm.org/D56612 Files: clangd/AST.cpp clangd/AST.h clangd/ref

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

ilya-biryukov updated this revision to Diff 182321. ilya-biryukov added a comment. - Remove the header file Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clangd/refactor/tweaks/CMakeLists.txt cl

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

ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.cpp:346 + {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND, + ExecuteCommandParams::CLANGD_APPLY_CODE_ACTION}}, }}, sammccall wrote: > Seems a

[PATCH] D56856: [tooling] Add a new argument adjuster for deleting plugin related command line args

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM after the nits are fixed Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56856/new/ https://reviews.llvm.org/D56856 ___

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

ilya-biryukov added a comment. This should be ready for another round of review. Let me know if I missed any of the older comments. There are two more things that I'd like to do before this lands: - go through the docs again and update/simplify them to make sure they're in sync with the new int

[PATCH] D56841: [clangd] Filter out plugin related flags and move all commandline manipulations into OverlayCDB.

ilya-biryukov added a comment. just a few nits Comment at: clangd/GlobalCompilationDatabase.cpp:23 + +void AdjustArguments(tooling::CompileCommand &Cmd, + const std::string &ResourceDir) { naming NIT: use `adjustArguments`

[PATCH] D56915: [clangd] Make background index less chatty

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric. It is producing too much input in non-verbose mode, i.e. a message per indexed file https://reviews.llvm.org/D56915 Files: clang-tools-extra/

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

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:363 + return CB(A.takeError()); +return CB((*A)->apply(*Inputs)); + }; sammccall wrote: > we should `format::cleanUpAroundReplacements`... fine to leave this as a FIXME Added a FIX

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

ilya-biryukov updated this revision to Diff 182534. ilya-biryukov marked 10 inline comments as done. ilya-biryukov added a comment. - Update comments - Return an ID+Title instead of a Tweak object - Rename codeAction -> tweak everywhere - Remove Tweak::Selection::File - Return Expected instead of

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

ilya-biryukov updated this revision to Diff 182545. ilya-biryukov added a comment. - Move to the monorepo - Move out the source code helpers (they're now in swap-if-branches) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56610/new/ https://reviews.llvm.org/D56610 Files: clang-tools-e

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

ilya-biryukov updated this revision to Diff 182546. ilya-biryukov added a comment. - Move source code helpers to this patch 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/clan

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

ilya-biryukov updated this revision to Diff 182547. ilya-biryukov added a comment. - Move to the monorepo 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-ex

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

ilya-biryukov updated this revision to Diff 182555. ilya-biryukov added a comment. - Add tests. 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.h clang-tool

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

ilya-biryukov added a comment. In D56610#1363408 , @JonasToth wrote: > Is this for something like `add const`? > If yes, there is clang-tidy effort on that, see > https://reviews.llvm.org/D54943 and https://reviews.llvm.org/D54395 for a > similar effor

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Thanks for fixing this. Quick LGTM to fix a crash, albeit a fews NITs. Comment at: clangd/index/SymbolCollector.cpp:372 + // ObjCPropertyDecl may have an Ori

[PATCH] D56841: [clangd] Filter out plugin related flags and move all commandline manipulations into OverlayCDB.

ilya-biryukov added inline comments. Comment at: clangd/GlobalCompilationDatabase.cpp:24 +void AdjustArguments(tooling::CompileCommand &Cmd, + const std::string &ResourceDir) { + // Strip plugin related command line arguments. Clangd does kad

[PATCH] D56976: [clang] [test] Pass -ccc-install-dir in mac compilation db test

ilya-biryukov added a comment. LGTM to unbreak the tests on *BSD, but could you please leave a comment? It would probably be best to take `argv[0]` into account on *BSD systems as well (possibly only if `clang` could not be found). At least if that's the behaviour on **all** other systems. Rep

[PATCH] D56976: [clang] [test] Pass -ccc-install-dir in mac compilation db test

ilya-biryukov added inline comments. Comment at: test/Tooling/clang-check-mac-libcxx-fixed-compilation-db.cpp:15 +// RUN: -stdlib=libc++ -target x86_64-apple-darwin \ +// RUN: -ccc-install-dir %t/mock-libcxx/bin #include Could you leave a comment this i

[PATCH] D56976: [clang] [test] Pass -ccc-install-dir in mac compilation db test

ilya-biryukov added a comment. In D56976#1365087 , @mgorny wrote: > Actually, it works on other systems because they don't use `argv[0]`. The > problem is in compilation db code using getMainExecutable() in ctor without > passing the real argv to it. B

[PATCH] D56976: [clang] [test] Pass -ccc-install-dir in mac compilation db test

ilya-biryukov added a comment. Was confused about your comment, so it works on other systems because they don't rely on `argv[0]` to get the path to main executable. Got you. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56976/new/ https://reviews.llvm.org/D5697

[PATCH] D55256: [clangd] Support clang-tidy configuration in clangd.

ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.h:132 - RealFileSystemProvider FSProvider; /// Options used for code completion hokein wrote: > sammccall wrote: > > ilya-biryukov wrote: > > > Could we instead call `getRealFS()` when

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

ilya-biryukov added a comment. @dgoldman, do you have commit access or should I land this for you? Comment at: clangd/index/SymbolCollector.cpp:375 + // not a NamedDecl. + const NamedDecl *OriginalDecl = dyn_cast(ASTNode.OrigD); + if (!OriginalDecl) NIT: use

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

ilya-biryukov added a comment. Good point, @aaron.ballman! @dgoldman, could you please add a test case? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56916/new/ https://reviews.llvm.org/D56916 ___ cfe-c

[PATCH] D56841: [clangd] Filter out plugin related flags and move all commandline manipulations into OverlayCDB.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clangd/GlobalCompilationDatabase.cpp:24 +void AdjustArguments(tooling::CompileCommand &Cmd, + const std::string &ResourceDi

[PATCH] D56860: [clangd] NFC: Use buildCompilerInvocation in CodeComplete

ilya-biryukov added inline comments. Comment at: clangd/CodeComplete.cpp:1030 auto &FrontendOpts = CI->getFrontendOpts(); FrontendOpts.DisableFree = false; FrontendOpts.SkipFunctionBodies = true; remove this line, `buildCompilerInvocation` handles this f

[PATCH] D56860: [clangd] NFC: Use buildCompilerInvocation in CodeComplete

ilya-biryukov added a reviewer: sammccall. ilya-biryukov added a comment. Adding Sam as a reviewer, IIRC we used to have `buildCompilerInvocation` here and he was the one who inlined it. I would personally LGTM this change (with the two comments fixed), but Sam might have a different opinion on

[PATCH] D56976: [clang] [test] Pass -ccc-install-dir in mac compilation db test

ilya-biryukov accepted this revision. ilya-biryukov added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56976/new/ https://reviews.llvm.org/D56976 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org

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

ilya-biryukov updated this revision to Diff 182870. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. - Inline Tweak::Selection::create() - Define Tweak::id() in REGISTER_TWEAK. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56267/new/ https://reviews.llvm.or

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

ilya-biryukov marked an inline comment as not done. ilya-biryukov added inline comments. Comment at: clangd/refactor/Tweak.h:40 + struct Selection { +static llvm::Optional create(llvm::StringRef File, +llvm::StringRef Code, ---

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

ilya-biryukov updated this revision to Diff 182872. ilya-biryukov added a comment. - Update id of swap branches in tests - Rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clang-tools-extra/clangd/SourceCode.cpp clang-tools-extr

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

ilya-biryukov updated this revision to Diff 182871. ilya-biryukov added a comment. - Rebase 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/clangd/refactor/twea

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

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/refactor/Tweak.cpp:27 +std::vector> prepareTweaks(const Tweak::Selection &S) { +#ifndef NDEBUG + { Please note I added these assertions here

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

ilya-biryukov updated this revision to Diff 182873. ilya-biryukov added a comment. - Rebase 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-extra/clangd/ref

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

ilya-biryukov updated this revision to Diff 182884. ilya-biryukov added a comment. - Use helper to avoid creating RewriteBuffer - Use std::string to fix stack corruption CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clang-tools-extra/

[PATCH] D56903: [clangd] Suggest adding missing includes for incomplete type diagnostics.

ilya-biryukov added a comment. A drop-by comment. Comment at: clangd/IncludeFixer.cpp:66 + vlog("Trying to fix include for incomplete type {0}", IncompleteType); + FuzzyFindRequest Req; + Req.AnyScope = false; sammccall wrote: > limit? Why do we use fuzzyFin

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

ilya-biryukov updated this revision to Diff 182900. ilya-biryukov added a comment. - Use a helper to avoid creating RewriteBuffer CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56611/new/ https://reviews.llvm.org/D56611 Files: clang-tools-extra/clangd/SourceCode.cpp clang-tools-extr

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

ilya-biryukov updated this revision to Diff 182910. ilya-biryukov marked 4 inline comments as done. ilya-biryukov added a comment. - Fix a typo in a comment: isValidRange -> isValidFileRange - Make action available under 'else' keywords and conditions - Move the logic of creating replacements to a

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