[PATCH] D64712: [clangd] Refactor background-index shard loading

2019-07-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL366458: [clangd] Refactor background-index shard loading (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D64745: [clangd] BackgroundIndex stores shards to the closest project

2019-07-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp:149 + std::unique_ptr create(PathRef CDBDirectory) { +assert(!CDBDirectory.empty() && + "Tried to create storage for empty directory!");

[PATCH] D64745: [clangd] BackgroundIndex stores shards to the closest project

2019-07-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 210621. kadircet marked 3 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64745/new/ https://reviews.llvm.org/D64745 Files:

[PATCH] D64745: [clangd] BackgroundIndex stores shards to the closest project

2019-07-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL366467: [clangd] BackgroundIndex stores shards to the closest project (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D64970: [clangd] Handle windows line endings in QueryDriver

2019-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM thanks! Do you have commit access? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64970/new/ https://reviews.llvm.org/D64970

[PATCH] D64980: [clangd][BackgroundIndexLoader] Directly store DependentTU while loading shard

2019-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. We were deferring the population of DependentTU field in LoadedShard until BackgroundIndexLoader was consumed.

[PATCH] D64970: [clangd] Handle windows line endings in QueryDriver

2019-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL366545: [clangd] Handle windows line endings in QueryDriver (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D64863: [clangd] Ignore diags from builtin files

2019-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 210786. kadircet added a comment. - Add tests. - Change the layer we ignore the diags: - Mark diags from headers as insidemainfile when we decide to surface them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D64863: [clangd] Ignore diags from builtin files

2019-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked 2 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:473 + SourceManager = Info.getSourceManager(); + if (!InsideMainFile && SM.isWrittenInBuiltinFile(Info.getLocation())) { +

[PATCH] D64980: [clangd][BackgroundIndexLoader] Directly store DependentTU while loading shard

2019-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL366559: Revert Revert r366458, r366467 and r366468 (authored by kadircet, committed by ). Herald added subscribers: llvm-commits, mgorny. Herald added a project: LLVM. Changed prior to commit:

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

2019-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:208 const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor(); - if (!N) -return false; - Target = llvm::make_unique(N, SM, Ctx); - return

[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

2019-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. Thanks, LGTM from my side. Do you have any concerns @sammccall ? Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:427 R"cpp(void f(int a) {

[PATCH] D64990: [clangd] Disable background-index on lit-tests by default

2019-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Since background-index can perform disk writes, we don't want to turn it on tests that won't clear it.

[PATCH] D64990: [clangd] Disable background-index on lit-tests by default

2019-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL366575: [clangd] Disable background-index on lit-tests by default (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D64863: [clangd] Ignore diags from builtin files

2019-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 210834. kadircet added a comment. - Move deduplication logic back into the `flushLastDiag` as discussed offline. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64863/new/ https://reviews.llvm.org/D64863

[PATCH] D64863: [clangd] Ignore diags from builtin files

2019-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 210838. kadircet marked 5 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64863/new/ https://reviews.llvm.org/D64863 Files:

[PATCH] D64863: [clangd] Ignore diags from builtin files

2019-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:563 FillDiagBase(*LastDiag); -adjustDiagFromHeader(*LastDiag, Info, *LangOpts); +if (!InsideMainFile) + LastDiagWasAdjusted = adjustDiagFromHeader(*LastDiag, Info, *LangOpts);

[PATCH] D65154: [clangd] Reformat use of cl::opt: use unqualified name and don't bin-pack attributes. NFC

2019-07-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. That LGTM, but i didn't have any bad feelings regarding the previous state. Since we tend to read those options in isolation and don't care about how they look like in general, but that

[PATCH] D64860: [clangd] Get rid of dots and dotsdots within GlobalCompilationDatabase

2019-07-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D64860 Files:

[PATCH] D64613: [clangd] Type hierarchy: don't resolve parents if the client only asked for children

2019-07-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64613/new/ https://reviews.llvm.org/D64613

[PATCH] D64712: [clangd][NFC] Refactor background-index shard loading

2019-07-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 210303. kadircet added a comment. - As discussed offline, moved the "staleness" detection back to background index - A renaming within Rebuilder, "LoadedTUs -> LoadedShards" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D64789: [clangd] Handle windows line endings in QueryDriver

2019-07-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL366320: [clangd] Handle windows line endings in QueryDriver (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D64863: [clangd] Ignore diags from builtin files

2019-07-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay. Herald added a project: clang. This fixes a case where we show diagnostics on arbitrary lines, in an internal codebase. Open for ideas on unittesting this.

[PATCH] D68024: [clangd] Implement GetEligiblePoints

2019-09-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 221896. kadircet added a comment. - Rename regions to points - Return EOF if there are no shared namespaces with the target Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68024/new/

[PATCH] D67748: [clangd] Add a helper for extracting nonlocal decls in a FunctionDecl

2019-09-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL372948: [clangd] Add a helper for extracting nonlocal decls in a FunctionDecl (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior

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

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 222562. kadircet added a comment. - Fix getSemiColon to handle semicolons at the end of file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66647/new/ https://reviews.llvm.org/D66647 Files:

[PATCH] D68137: [clangd] Handle template arguments in findExplicitReferences

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68137/new/ https://reviews.llvm.org/D68137

[PATCH] D68182: [Clangd] Ensure children are always RootStmt in ExtractFunction (Fixes #153)

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:110 // begins in selection range, ends in selection range and any scope that begins // outside the selection range, ends outside as well. const Node

[PATCH] D68261: [clangd] Add "inline" keyword to prevent ODR-violations in DefineInline

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added reviewers: ilya-biryukov, hokein. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D68261 Files:

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 222567. kadircet marked 5 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68143/new/ https://reviews.llvm.org/D68143 Files:

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang/lib/AST/DeclTemplate.cpp:513 getDefaultArgumentInfo()->getTypeLoc().getEndLoc()); - else -return TypeDecl::getSourceRange(); + else if(getName().empty()) +return SourceRange(getBeginLoc());

[PATCH] D68245: [Clangd] ExtractFunction: Don't extract body of enclosing function.

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:231 + if (const Stmt *S = Child->ASTNode.get()) +if (EnclosingFunc->hasBody()

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 222581. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68143/new/ https://reviews.llvm.org/D68143 Files: clang/lib/AST/DeclTemplate.cpp

[PATCH] D68024: [clangd] Implement GetEligiblePoints

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 222590. kadircet marked an inline comment as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68024/new/ https://reviews.llvm.org/D68024 Files:

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang/lib/AST/DeclTemplate.cpp:513 getDefaultArgumentInfo()->getTypeLoc().getEndLoc()); - else -return TypeDecl::getSourceRange(); + else if(getName().empty()) +

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL373340: [clang] Make handling of unnamed template params similar to function params (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[PATCH] D68273: [clangd] Fix raciness in code completion tests

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 222618. kadircet added a comment. - As discussed offline, making `consumeRequests` take an argument for how many requests to receive before returning. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68273/new/

[PATCH] D68273: [clangd] Fix raciness in code completion tests

2019-10-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added reviewers: sammccall, ilya-biryukov. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D68273 Files:

[PATCH] D65433: [clangd] DefineInline action availability checks

2019-09-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 221947. kadircet added a comment. - Use canonical decl when checking visibility Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65433/new/ https://reviews.llvm.org/D65433 Files:

[PATCH] D68024: [clangd] Implement GetEligiblePoints

2019-09-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 221920. kadircet marked an inline comment as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68024/new/ https://reviews.llvm.org/D68024 Files:

[PATCH] D68024: [clangd] Implement GetEligiblePoints

2019-09-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked 5 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.cpp:655 +lex(llvm::StringRef Code, const format::FormatStyle , +llvm::function_ref A) { // FIXME: InMemoryFileAdapter crashes unless the buffer is

[PATCH] D65433: [clangd] DefineInline action availability checks

2019-09-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 221910. kadircet added a comment. - Rebase and update testhelpers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65433/new/ https://reviews.llvm.org/D65433 Files:

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

2019-09-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 221912. kadircet added a comment. - Rebase and update testhelpers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66647/new/ https://reviews.llvm.org/D66647 Files:

[PATCH] D65433: [clangd] DefineInline action availability checks

2019-09-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 221911. kadircet added a comment. - Revert miscommit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65433/new/ https://reviews.llvm.org/D65433 Files: clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt

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

2019-09-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 221913. kadircet added a comment. - Mark tweak as visible Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66647/new/ https://reviews.llvm.org/D66647 Files:

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

2019-09-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 222180. kadircet added a comment. - Add renaming of template and function parameters Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66647/new/ https://reviews.llvm.org/D66647 Files:

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

2019-09-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D66647#1684046 , @ilya-biryukov wrote: > We also need to rename parameters sometimes, right? > > // Sometimes we need to rename parameters. > void usages(int decl_param, int); > > void usages(int def_param, int

[PATCH] D68024: [clangd] Implement GetEligiblePoints

2019-09-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked 3 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.cpp:997 + // definitions as well. One might use a closing parantheses(")" followed by an + // opening brace "{" to trigger the start. +

[PATCH] D65433: [clangd] DefineInline action availability checks

2019-09-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 222158. kadircet added a comment. - Rebase and bail out on methods inside templated classes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65433/new/ https://reviews.llvm.org/D65433 Files:

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-09-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, arphaman. Herald added a project: clang. Clang uses the location identifier should be inserted for declarator decls when a decl is unnamed. But for type template and template template

[PATCH] D68124: [clangd] Fix template type aliases in findExplicitReference

2019-09-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang-tools-extra/clangd/FindTarget.cpp:478 + explicitReferenceTargets(DynTypedNode::create(L.getType()), +

[PATCH] D67907: [clangd] Implement a smart version of HeaderSource switch.

2019-09-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks for formatting test cases, I had it in mind but forgot to mention in last round. LGTM Comment at: clang-tools-extra/clangd/HeaderSourceSwitch.cpp:113 + auto

[PATCH] D68024: [clangd] Implement GetEligiblePoints

2019-09-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.h:270 + /// It will be “a::b” for both carrot locations. + std::string CurrentNamespace; + /// Offsets into the code marking eligible points to insert a

[PATCH] D68120: [clangd] Handle type template parameters in findExplicitReferences

2019-09-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM. what about template template and non-type template params ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68120/new/

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

2019-09-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D66647#1684012 , @ilya-biryukov wrote: > It's ok to leave this out of the initial change, but could we describe our > strategy to tackle this somewhere in the comments - how we want to fix this > and when. I actually have

[PATCH] D67907: [clangd] Implement a smart version of HeaderSource switch.

2019-09-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. mostly LG, a few small comments Comment at: clang-tools-extra/clangd/HeaderSourceSwitch.cpp:86 + auto AwardTarget = [&](const char *TargetURI) { +if (auto TargetPath = URI::resolve(TargetURI, OriginalFile)) { + if (*TargetPath !=

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

2019-09-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 222102. kadircet marked 5 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66647/new/ https://reviews.llvm.org/D66647 Files:

[PATCH] D67916: [clangd] Simplify the callside of URI::resolve, NFC.

2019-09-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang-tools-extra/clangd/FindSymbols.cpp:48 if (!Path) { return llvm::make_error( +formatv("Could not resolve path for URI

[PATCH] D67826: [clangd] A helper to find explicit references and their names

2019-09-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.cpp:376 + + // We prefer to return template instantiation, but fallback to template + // pattern if instantiation is not available.

[PATCH] D67748: [clangd] Add a helper for extracting nonlocal decls in a FunctionDecl

2019-09-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 221272. kadircet marked an inline comment as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67748/new/ https://reviews.llvm.org/D67748 Files:

[PATCH] D67748: [clangd] Add a helper for extracting nonlocal decls in a FunctionDecl

2019-09-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. As discussed offline, we decided to change the file once we have more helpers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67748/new/ https://reviews.llvm.org/D67748 ___

[PATCH] D67651: [clangd] No ExtractFunction on empty selections.

2019-09-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D67651#1672489 , @hokein wrote: > fair enough, didn't notice that fix before, the testcase here probably > doesn't work after that patch. yes it shouldn't work after that one. Repository: rG LLVM Github Monorepo

[PATCH] D66937: [clangd] Fix the stale documentation about background indexing.

2019-09-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/docs/clangd/Installation.rst:363 -- Pass an experimental `-background-index` command line argument. With - this feature enabled, clangd incrementally builds an index of projects - that you work on and uses the

[PATCH] D67826: [clangd] A helper to find explicit references and their names

2019-09-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked 4 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.h:96 +}; +llvm::raw_ostream <<(llvm::raw_ostream , ReferenceLoc R); + ilya-biryukov wrote: > kadircet wrote: > > Is this for testing

[PATCH] D68324: [clangd] Always send file URIs to editors

2019-10-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL373435: [clangd] Always send file URIs to editors (authored by kadircet, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D68324: [clangd] Always send file URIs to editors

2019-10-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. kadircet added a reviewer: sammccall. Editors only know about file URIs, make sure we do not use any custom schemes while sending edits.

[PATCH] D68273: [clangd] Fix raciness in code completion tests

2019-10-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 222890. kadircet marked 5 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68273/new/ https://reviews.llvm.org/D68273 Files:

[PATCH] D68273: [clangd] Fix raciness in code completion tests

2019-10-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:1141 +ReceivedRequestCV.wait(Lock, + [this, Num] { return NumRequestsReceived == Num; }); +NumRequestsReceived = 0;

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

2019-10-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 222892. kadircet marked 5 inline comments as done. kadircet added a comment. - Update test helper to take an out parameter Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66647/new/

[PATCH] D68024: [clangd] Implement GetEligiblePoints

2019-09-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 222434. kadircet marked 7 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68024/new/ https://reviews.llvm.org/D68024 Files:

[PATCH] D68024: [clangd] Implement GetEligiblePoints

2019-09-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.h:270 + /// It will be “a::b” for both carrot locations. + std::string CurrentNamespace; + /// Offsets into the code marking eligible points to insert a function ilya-biryukov

[PATCH] D68211: [clangd] Use the index-based API to do the header-source switch.

2019-09-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. some nits, thanks! Comment at: clang-tools-extra/clangd/ClangdServer.cpp:456 + // 1) use the file-only heuristic, it requires some IO but it is much + //faster

[PATCH] D68211: [clangd] Use the index-based API to do the header-source switch.

2019-09-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. thanks, mostly LG Comment at: clang-tools-extra/clangd/ClangdServer.cpp:453 +void ClangdServer::switchSourceHeader( +PathRef Path, Callback> CB) { + if (auto CorrespondingFile = could you add some comments explaining, why we

[PATCH] D68124: [clangd] Fix template type aliases in findExplicitReference

2019-09-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.cpp:478 + explicitReferenceTargets(DynTypedNode::create(L.getType()), + DeclRelation::Alias)}; } can you explain why these two are

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

2019-09-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D66647#1685554 , @ilya-biryukov wrote: > Are we planning to fix this right away or should we keep this indefinitely? > If latter, I believe we should think about some heuristics to avoid > qualification in the short term

[PATCH] D67825: [AST] Extract Decl::printNestedNameSpecifier helper from Decl::printQualifiedName

2019-09-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, I know it is trivial, but some more unittests wouldn't hurt anyone :D Also could you update the summary to mention "doing string manipulations in the presence of template arguments

[PATCH] D68137: [clangd] Handle template arguments in findExplicitReferences

2019-09-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.cpp:556 + // We re-define Traverse*, since there's no corresponding Visit*. + bool TraverseTemplateArgumentLoc(TemplateArgumentLoc A) { ... and we need it because, template

[PATCH] D63270: [clangd] Add include-mapping for C symbols.

2019-06-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/include-mapping/gen_std.py:95 - parse_pages = [ -(cpp_root, "symbol_index.html", "std::"), -# std sub-namespace

[PATCH] D63323: [clangd] Don't maintain a list of c-family extensions in vscode extension.

2019-06-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. could you also bump the version numbers? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63323/new/ https://reviews.llvm.org/D63323

[PATCH] D63316: [clangd] Include the diagnostics's code when comparing diagnostics

2019-06-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Could you also add a lit test for the case you mentioned in the github issue? You can find pointers in clang-tools-extra/clangd/test/diagnostics.test and clang-tools-extra/clangd/test/fixits-codeaction.test Comment at:

[PATCH] D58880: [clangd] Type hierarchy subtypes

2019-06-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM with a few small comments. Thanks for implementing this awesome feature! Comment at: clang-tools-extra/clangd/FindSymbols.cpp:42 +llvm::Optional

[PATCH] D64247: [clangd] Filter out non-governed files from broadcast

2019-07-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 208890. kadircet marked 3 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64247/new/ https://reviews.llvm.org/D64247 Files:

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

2019-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 227035. kadircet added a comment. - Add comments - Early return when generating edits fails Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68937/new/ https://reviews.llvm.org/D68937 Files:

[PATCH] D69608: [clangd] Helper for getting nested namespace qualification

2019-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay. Herald added a project: clang. Introduce a new helper for getting minimally required qualifiers necessary to spell a name at a point in a given

[PATCH] D69033: [clangd] Improve symbol qualification in DefineInline code action

2019-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 227040. kadircet added a comment. - Rebase - Move qualification logic into AST.h so that it can be used by define outline, see D69033 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D69298: [clangd] Define out-of-line initial apply logic

2019-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 227108. kadircet marked 5 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69298/new/ https://reviews.llvm.org/D69298 Files:

[PATCH] D69298: [clangd] Define out-of-line initial apply logic

2019-10-30 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:55 + // Include template parameter list. + if (auto *FTD = FD->getDescribedFunctionTemplate()) +return FTD->getBeginLoc(); hokein wrote: > Could you

[PATCH] D70173: [clang][Tooling] Filter flags that generate output in SyntaxOnlyAdjuster

2019-11-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 229069. kadircet marked an inline comment as done. kadircet added a comment. - Update comment to mention droppage of options that generate output. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70173/new/

[PATCH] D70173: [clang][Tooling] Filter flags that generate output in SyntaxOnlyAdjuster

2019-11-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG16bdcc809c72: [clang][Tooling] Filter flags that generate output in SyntaxOnlyAdjuster (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D70173: [clang][Tooling] Filter flags that generate output in SyntaxOnlyAdjuster

2019-11-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: hokein. Herald added subscribers: cfe-commits, usaxena95, ilya-biryukov. Herald added a project: clang. Flags that generate output could result in failures when creating syntax only actions. This patch introduces initial logic for

[PATCH] D70209: [clangd] Expose value of enumerators to Hover API. (not UI yet)

2019-11-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:984 }}, + {R"cpp(enum Color { RED, [[GR^EEN]], };)cpp", + [](HoverInfo ) {

[PATCH] D70203: [AST] Attach comment in `/** doc */ typedef struct A {} B` to B as well as A.

2019-11-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang/test/Index/annotate-comments-typedef.m:20 } MyEnum; -// CHECK: TypedefDecl=MyEnum:[[@LINE-1]]:3 (Definition) FullCommentAsHTML=[ Documentation for MyEnum ] FullCommentAsXML=[anonymousc:@EA@MyEnumtypedef enum MyEnum MyEnum

[PATCH] D70194: [clangd] Ignore more implicit nodes in computing selection.

2019-11-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang-tools-extra/clangd/Selection.cpp:162 + // Refs to operator() and [] are (almost?) always implicit as part of calls. + if (auto *DRE =

[PATCH] D70325: [clangd] Fix hover 'local scope' to include class template params

2019-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. LGTM, with a question. What about default template params? I believe we would also like to print them, could you add a test case for that? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70325/new/

[PATCH] D70359: [clangd] Show values of more expressions on hover

2019-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:247 + // to the enclosing call. + if (E->getType()->isFunctionType() || E->getType()->isFunctionPointerType() || +

[PATCH] D69543: [clangd] Add a tweak refactoring to wrap Objective-C string literals in `NSLocalizedString` macros

2019-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. implementation lgtm with a few nits. main concern is about the new getlangopts helper Comment at: clang-tools-extra/clangd/ParsedAST.h:80 + const LangOptions () const { +return getASTContext().getLangOpts(); can we introduce

[PATCH] D70203: [AST] Attach comment in `/** doc */ typedef struct A {} B` to B as well as A.

2019-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet marked an inline comment as done. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang/test/Index/annotate-comments-typedef.m:20 } MyEnum; -// CHECK:

[PATCH] D70357: [clangd] Untangle Hover from XRefs, move into own file.

2019-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:374 + } else { +auto Offset = positionToOffset(SM.getBufferData(SM.getMainFileID()), Pos); +if (!Offset) { nit: `SM.getFileOffset(SourceLocationBeg)` ?

[PATCH] D70222: [clangd] Add support for .rsp files in compile_commands.json

2019-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Thanks for taking a look into this, the `rsp files` issue has came up before in the past but there wasn't enough investment to implement it. Haven't checked the implementation in detail yet, I believe the layering should be different; This is a common problem for all

[PATCH] D70222: [clangd] Add support for .rsp files in compile_commands.json

2019-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Thanks, layering seems better now. But as Sam mentioned in the issue it would be better to have something like `expandRSPFiles` similar to `inferMissingCompileCommands`. As the problem is not in the `JSONCompilationDatabase` itself, it can also occur in

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