Page MenuHomePhabricator

ilya-biryukov (Ilya Biryukov)Administrator
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 6 2017, 1:42 AM (132 w, 1 d)
Roles
Administrator

Recent Activity

Today

ilya-biryukov added inline comments to D68977: [clangd] Report declaration references in findExplicitReferences..
Fri, Oct 18, 2:56 AM · Restricted Project

Yesterday

ilya-biryukov added a comment to D68977: [clangd] Report declaration references in findExplicitReferences..

Mostly NITs from my side, the change LG, thanks!

Thu, Oct 17, 7:02 AM · Restricted Project
ilya-biryukov added inline comments to D68977: [clangd] Report declaration references in findExplicitReferences..
Thu, Oct 17, 5:16 AM · Restricted Project
ilya-biryukov added inline comments to D68977: [clangd] Report declaration references in findExplicitReferences..
Thu, Oct 17, 4:57 AM · Restricted Project
ilya-biryukov added inline comments to D68977: [clangd] Report declaration references in findExplicitReferences..
Thu, Oct 17, 4:57 AM · Restricted Project
ilya-biryukov added inline comments to D68977: [clangd] Report declaration references in findExplicitReferences..
Thu, Oct 17, 4:38 AM · Restricted Project
ilya-biryukov added inline comments to D68977: [clangd] Report declaration references in findExplicitReferences..
Thu, Oct 17, 4:38 AM · Restricted Project

Wed, Oct 16

ilya-biryukov added a comment to D69033: [clangd] Improve symbol qualification in DefineInline code action.

We seem to have trouble only in presence of using declarations and using namespace directives.
I wonder how complicated it would be to take them into account instead. That would clearly be easier to read, as we'll hit right into the center of the problem.

Wed, Oct 16, 6:55 AM · Restricted Project
ilya-biryukov abandoned D67827: [clangd] Simplify name qualification in DefineInline.

D66647 was updated to use findExplicitReferences, abandoning this revision.

Wed, Oct 16, 6:03 AM · Restricted Project
ilya-biryukov abandoned D56612: [clangd] A code action to remove 'using namespace'.

This was finalized and landed as D68562

Wed, Oct 16, 5:59 AM
ilya-biryukov added a comment to D68937: [clangd] Add parameter renaming to define-inline code action.

An alternative approach I'm thinking of:
After D68977 lands, we could try using findExplicitReferences to produce all of these edits:

  1. we collect locations of all references and declaration of relevant parameters and template parameters.
  2. find the ones where the name differs and produce the changes.
Wed, Oct 16, 2:43 AM · Restricted Project
ilya-biryukov added a comment to D68977: [clangd] Report declaration references in findExplicitReferences..

Mostly LG, but please take a look at the NITs and the implementation-related comments.

Wed, Oct 16, 2:20 AM · Restricted Project

Tue, Oct 15

ilya-biryukov added inline comments to D68978: [clangd] Propagate main context into ClangdServer.
Tue, Oct 15, 7:49 AM · Restricted Project

Mon, Oct 14

ilya-biryukov added a comment to D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154).

LGTM from my side, but not marking as accepted yet to make sure @hokein has a chance to take a final look.

Mon, Oct 14, 1:07 AM · Restricted Project

Fri, Oct 11

ilya-biryukov added a comment to D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154).

I like how we went from using heuristics, to not using heuristics, to using heuristics again :)

But admittedly, the new heuristics are more accurate because they're based on phase 1 lookup results rather than just syntax, so I'm happy with the outcome!

Fri, Oct 11, 3:13 AM · Restricted Project

Thu, Oct 10

ilya-biryukov accepted D68562: [clangd] Add RemoveUsingNamespace tweak..

LGTM, many thanks!

Thu, Oct 10, 10:53 AM · Restricted Project
ilya-biryukov added inline comments to D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154).
Thu, Oct 10, 12:57 AM · Restricted Project

Wed, Oct 9

ilya-biryukov added a comment to D68562: [clangd] Add RemoveUsingNamespace tweak..

Many thanks, this LG overall, just a few NITs and documentation requests.

Wed, Oct 9, 7:11 AM · Restricted Project
ilya-biryukov added inline comments to D68702: [clangd] Make sure ReplyCallbacks are destroyed before RequestCancelersMutex.
Wed, Oct 9, 6:33 AM · Restricted Project
ilya-biryukov added a comment to D67536: [WIP] [clangd] Add support for an inactive regions notification.

Just noticed the next version of LSP added diagnostic tags for things like "unused field" or "dead code":
https://microsoft.github.io/language-server-protocol/specifications/specification-3-15, search for DiagnosticTag.

Wed, Oct 9, 3:33 AM · Restricted Project
ilya-biryukov added inline comments to D68562: [clangd] Add RemoveUsingNamespace tweak..
Wed, Oct 9, 3:14 AM · Restricted Project
ilya-biryukov committed rGaeae71cd96c3: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU (authored by ilya-biryukov).
[Sema] Emit diagnostics for uncorrected delayed typos at the end of TU
Wed, Oct 9, 3:06 AM
ilya-biryukov closed D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU.
Wed, Oct 9, 3:06 AM · Restricted Project
ilya-biryukov added inline comments to D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU.
Wed, Oct 9, 3:05 AM · Restricted Project
ilya-biryukov committed rGdf7ea71c3adf: Revert r374006: Reland 'Add VFS support for sanitizers' blacklist' (authored by ilya-biryukov).
Revert r374006: Reland 'Add VFS support for sanitizers' blacklist'
Wed, Oct 9, 2:47 AM
ilya-biryukov added a comment to D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU.

Tried the suggested approach and ran into the issue described above.
Either we make the diagnostics less useful ('did you mean foo::bar?' turns into 'unresolved identifier bar'; without mentioning the correction) or we even stop providing some corrections in addition to that.

On the other hand, I agree that over time we will start emitting too many diagnostics at the end of TU if keep the patch as is. That is not a good way.
There should be a better option for emitting the uncorrected diagnostics closer to where they are produced, but I don't have a good idea on what it should be off the top of my head. Ideas warmly welcome.

I might be wrong here, but I thought that diagnostics were delayed until typo correction has been completed on an expression. For example, you'll only get something like unresolved identifier bar instead of did you mean foo::bar? only when you call the DiagHandler with either a proper TypoCorrection or an empty one. If so, I'm not sure how you'd get into this case if you're calling CorrectDelayedTyposInExpr and tracking which typos have been resolved already.

Wed, Oct 9, 2:47 AM · Restricted Project
ilya-biryukov added a comment to D67536: [WIP] [clangd] Add support for an inactive regions notification.

It also lets them consistently highlight part of the line (e.g. dead expressions or statements can be marked in gray even if they are on the same line).

Highlighting part of a line is not applicable to inactive preprocessor branches in C++.

Note that the opposite is true - inactive preprocessor branches can be expressed as range-based highlightings.

Wed, Oct 9, 2:01 AM · Restricted Project

Tue, Oct 8

ilya-biryukov committed rG1b36caf45e5e: [clangd] Disable expand auto on decltype(auto) (authored by ilya-biryukov).
[clangd] Disable expand auto on decltype(auto)
Tue, Oct 8, 7:10 AM
ilya-biryukov closed D68630: [clangd] Disable expand auto on decltype(auto).
Tue, Oct 8, 7:10 AM · Restricted Project
ilya-biryukov added a comment to D68630: [clangd] Disable expand auto on decltype(auto).

as the automatic build report did not work:

Tue, Oct 8, 7:01 AM · Restricted Project
ilya-biryukov created D68630: [clangd] Disable expand auto on decltype(auto).
Tue, Oct 8, 3:36 AM · Restricted Project
ilya-biryukov added a comment to D67536: [WIP] [clangd] Add support for an inactive regions notification.

So I don't think clients will/should prefer that - for best rendering they should know this is a line highlight.

Tue, Oct 8, 1:51 AM · Restricted Project

Mon, Oct 7

ilya-biryukov added inline comments to D68562: [clangd] Add RemoveUsingNamespace tweak..
Mon, Oct 7, 8:18 AM · Restricted Project
ilya-biryukov accepted D68565: [clang] Add test for FindNextToken in Lexer..

Thanks!

Mon, Oct 7, 6:18 AM · Restricted Project
ilya-biryukov added a comment to D68565: [clang] Add test for FindNextToken in Lexer..

Could you add a rationale in the change description? E.g. something like we do not have any tests for findNextToken

Mon, Oct 7, 4:49 AM · Restricted Project
ilya-biryukov added a comment to D68562: [clangd] Add RemoveUsingNamespace tweak..

The main comment is about limiting this only to global namespace. PTAL.

Mon, Oct 7, 4:18 AM · Restricted Project
ilya-biryukov accepted D68467: [clangd] If an undocumented definition exists, don't accept documentation from other forward decls..

LGTM

Mon, Oct 7, 2:55 AM · Restricted Project, Restricted Project

Fri, Oct 4

ilya-biryukov accepted D68458: [clangd] Collect missing macro references..

LGTM

Fri, Oct 4, 5:51 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D66647: [clangd] DefineInline action apply logic with fully qualified names.
Fri, Oct 4, 1:42 AM · Restricted Project
ilya-biryukov accepted D68273: [clangd] Fix raciness in code completion tests.

LGTM

Fri, Oct 4, 1:19 AM · Restricted Project, Restricted Project
ilya-biryukov committed rG8613e90ba71d: [CodeComplete] Ensure object is the same in compareOverloads() (authored by ilya-biryukov).
[CodeComplete] Ensure object is the same in compareOverloads()
Fri, Oct 4, 1:12 AM
ilya-biryukov added inline comments to D67536: [WIP] [clangd] Add support for an inactive regions notification.
Fri, Oct 4, 12:56 AM · Restricted Project
ilya-biryukov added inline comments to D68335: [CodeComplete] Ensure object is the same in compareOverloads().
Fri, Oct 4, 12:56 AM · Restricted Project, Restricted Project
ilya-biryukov updated the diff for D68335: [CodeComplete] Ensure object is the same in compareOverloads().
  • Address comments
Fri, Oct 4, 12:56 AM · Restricted Project, Restricted Project

Wed, Oct 2

ilya-biryukov updated the summary of D68335: [CodeComplete] Ensure object is the same in compareOverloads().
Wed, Oct 2, 6:58 AM · Restricted Project, Restricted Project
ilya-biryukov created D68335: [CodeComplete] Ensure object is the same in compareOverloads().
Wed, Oct 2, 6:48 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154).
Wed, Oct 2, 3:14 AM · Restricted Project
ilya-biryukov added inline comments to D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154).
Wed, Oct 2, 1:54 AM · Restricted Project
ilya-biryukov added a comment to D68273: [clangd] Fix raciness in code completion tests.

It just hit me that we should probably just wait for the async request to finish before returning from completion.
We might be doing this to reduce latency a little, but I don't recall us measuring that this provides a significant performance win.

Wed, Oct 2, 12:00 AM · Restricted Project, Restricted Project

Tue, Oct 1

ilya-biryukov added a comment to D68273: [clangd] Fix raciness in code completion tests.

This is still racy if there were > 1 request. I wonder if there a better way to address this?

Tue, Oct 1, 7:36 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D68143: [clang] Make handling of unnamed template params similar to function params.

Many thanks! LGTM

Tue, Oct 1, 6:07 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D68143: [clang] Make handling of unnamed template params similar to function params.
Tue, Oct 1, 3:33 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D68143: [clang] Make handling of unnamed template params similar to function params.
Tue, Oct 1, 3:16 AM · Restricted Project, Restricted Project
ilya-biryukov committed rG1d32da82490a: [clangd] Handle template arguments in findExplicitReferences (authored by ilya-biryukov).
[clangd] Handle template arguments in findExplicitReferences
Tue, Oct 1, 3:02 AM
ilya-biryukov added a comment to D66647: [clangd] DefineInline action apply logic with fully qualified names.

So currently AST doesn't store any information regarding template parameter locations except the deepest one.
Therefore I've changed the availability to discard any methods inside templated classes, since there is no way to
validate the template parameter names. Hopefully this should be a rare use-case, and I believe most of the times
people have same template paramater names on declaration and definition. Let me know what you think about
it.

Tue, Oct 1, 1:05 AM · Restricted Project
ilya-biryukov added inline comments to D68137: [clangd] Handle template arguments in findExplicitReferences.
Tue, Oct 1, 12:54 AM · Restricted Project, Restricted Project
ilya-biryukov updated the diff for D68137: [clangd] Handle template arguments in findExplicitReferences.
  • Restore dlog()
  • Check reference to function pointer non-type template parameters in tests
  • Extend comment of TraverseTemplateArgumentLoc
Tue, Oct 1, 12:54 AM · Restricted Project, Restricted Project
ilya-biryukov committed rGa160a0ba5313: [clangd] Handle OverloadExpr in targetDecl (authored by ilya-biryukov).
[clangd] Handle OverloadExpr in targetDecl
Tue, Oct 1, 12:28 AM
ilya-biryukov added inline comments to D67695: [clangd] Implement getBeginning for overloaded operators..
Tue, Oct 1, 12:07 AM · Restricted Project, Restricted Project

Mon, Sep 30

ilya-biryukov accepted D67695: [clangd] Implement getBeginning for overloaded operators..

Mostly LGTM

Mon, Sep 30, 9:21 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D67695: [clangd] Implement getBeginning for overloaded operators..

Mostly NITs, except the naming of the new TokenKind enum.
I think it's better to pick something that's not clashing with clang::tok::TokenKind, even if the enum is in a different namespace.

Mon, Sep 30, 5:29 AM · Restricted Project, Restricted Project

Fri, Sep 27

ilya-biryukov added inline comments to D68143: [clang] Make handling of unnamed template params similar to function params.
Fri, Sep 27, 11:10 AM · Restricted Project, Restricted Project
ilya-biryukov committed rG4ae238143001: [clangd] Fix template type aliases in findExplicitReference (authored by ilya-biryukov).
[clangd] Fix template type aliases in findExplicitReference
Fri, Sep 27, 10:59 AM
ilya-biryukov updated the diff for D68124: [clangd] Fix template type aliases in findExplicitReference.
  • Add a comment
Fri, Sep 27, 10:53 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D68137: [clangd] Handle template arguments in findExplicitReferences.
Fri, Sep 27, 6:11 AM · Restricted Project, Restricted Project
ilya-biryukov created D68137: [clangd] Handle template arguments in findExplicitReferences.
Fri, Sep 27, 6:06 AM · Restricted Project, Restricted Project
ilya-biryukov committed rGc383509ce69c: [clangd] Handle type template parameters in findExplicitReferences (authored by ilya-biryukov).
[clangd] Handle type template parameters in findExplicitReferences
Fri, Sep 27, 4:00 AM
ilya-biryukov updated the diff for D68120: [clangd] Handle type template parameters in findExplicitReferences.
  • Add tests for non-type template paremeters
Fri, Sep 27, 4:00 AM · Restricted Project, Restricted Project
ilya-biryukov reopened D68072: [clang-format] Reference qualifiers in member templates causing extra indentation..

The patch breaks some tests, see
http://lab.llvm.org:8011/builders/clang-cmake-x86_64-sde-avx512-linux/builds/27891/steps/ninja%20check%201/logs/FAIL%3A%20Clang-Unit%3A%3AFormatTest.FunctionAnnotations

Fri, Sep 27, 2:57 AM · Restricted Project, Restricted Project, Restricted Project
ilya-biryukov committed rG4627bdedd90d: Revert r373056: [clang-format] Reference qualifiers in member templates causing… (authored by ilya-biryukov).
Revert r373056: [clang-format] Reference qualifiers in member templates causing…
Fri, Sep 27, 2:49 AM
ilya-biryukov added inline comments to D68124: [clangd] Fix template type aliases in findExplicitReference.
Fri, Sep 27, 2:47 AM · Restricted Project, Restricted Project
ilya-biryukov committed rG2774457b2a5c: [clangd] Support OverloadExpr in findExplicitReferences (authored by ilya-biryukov).
[clangd] Support OverloadExpr in findExplicitReferences
Fri, Sep 27, 2:39 AM
ilya-biryukov added inline comments to D66647: [clangd] DefineInline action apply logic with fully qualified names.
Fri, Sep 27, 2:39 AM · Restricted Project
ilya-biryukov added a comment to D66647: [clangd] DefineInline action apply logic with fully qualified names.

I actually have a fixme saying:

// FIXME: Instead of fully qualifying we should try deducing visible scopes at
// target location and generate minimal edits.

Elaborating on it by saying "we can start by using the namespaces in targetcontext".

Fri, Sep 27, 2:30 AM · Restricted Project
ilya-biryukov committed rGc5343e721ba7: [clang-format] Reference qualifiers in member templates causing extra… (authored by ilya-biryukov).
[clang-format] Reference qualifiers in member templates causing extra…
Fri, Sep 27, 2:24 AM
ilya-biryukov added a comment to D68120: [clangd] Handle type template parameters in findExplicitReferences.

LGTM. can you also add some tests for template template and non-type template params ?

Fri, Sep 27, 2:24 AM · Restricted Project, Restricted Project
ilya-biryukov created D68124: [clangd] Fix template type aliases in findExplicitReference.
Fri, Sep 27, 2:11 AM · Restricted Project, Restricted Project

Thu, Sep 26

ilya-biryukov created D68120: [clangd] Handle type template parameters in findExplicitReferences.
Thu, Sep 26, 11:31 PM · Restricted Project, Restricted Project
ilya-biryukov created D68119: [clangd] Handle OverloadExpr in targetDecl.
Thu, Sep 26, 11:18 PM · Restricted Project, Restricted Project
ilya-biryukov created D68118: [clangd] Support OverloadExpr in findExplicitReferences.
Thu, Sep 26, 11:10 PM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D68024: [clangd] Implement GetEligiblePoints.
Thu, Sep 26, 6:52 AM · Restricted Project
ilya-biryukov accepted D68080: [clangd][vscode] Add npm helper commands to package/release the extension..

LGTM

Thu, Sep 26, 6:51 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D66647: [clangd] DefineInline action apply logic with fully qualified names.

We also need to rename parameters sometimes, right?

// Sometimes we need to rename parameters.
void usages(int decl_param, int);
Thu, Sep 26, 5:32 AM · Restricted Project
ilya-biryukov added inline comments to D68080: [clangd][vscode] Add npm helper commands to package/release the extension..
Thu, Sep 26, 5:23 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D66647: [clangd] DefineInline action apply logic with fully qualified names.

Currently we add too many qualifiers in some cases, e.g. here's a common pattern in clangd:

// foo.h
#include "Decl.h"
namespace clang::clangd { std::string printName(Decl*D); }
Thu, Sep 26, 5:01 AM · Restricted Project
ilya-biryukov added inline comments to D67695: [clangd] Implement getBeginning for overloaded operators..
Thu, Sep 26, 4:51 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D68077: [clangd][vscode] Turn on the semantic highlighting by default..

LGTM

Thu, Sep 26, 4:03 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D67964: [clangd] Update vscode lsp dependencies to pickup the new changes in LSP v3.15.0..

LGTM

Thu, Sep 26, 4:03 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D68024: [clangd] Implement GetEligiblePoints.

A few go-by comments from me, Haojian should have better context here!

Thu, Sep 26, 2:35 AM · Restricted Project

Wed, Sep 25

ilya-biryukov committed rG6648223faf39: Re-land r372863: [AST] Extract Decl::printNestedNameSpecifier helper from Decl… (authored by ilya-biryukov).
Re-land r372863: [AST] Extract Decl::printNestedNameSpecifier helper from Decl…
Wed, Sep 25, 8:48 AM
ilya-biryukov accepted D68027: [clangd] Change constness of parameters to findExplicitRefs.

@kadircet says we're using this pattern all over clangd. This somehow slipped my view before.
LGTM to be consistent with the rest of clangd (and functions in FindTarget.h).

Wed, Sep 25, 8:38 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D68027: [clangd] Change constness of parameters to findExplicitRefs.

Who are the callers? Selection tree?
FWIW, I'd rather pass non-const everywhere, the concept of const-ness is just not very useful for AST nodes.

Wed, Sep 25, 8:31 AM · Restricted Project, Restricted Project
ilya-biryukov committed rG71472a3eeceb: Revert r372863: [AST] Extract Decl::printNestedNameSpecifier helper from Decl… (authored by ilya-biryukov).
Revert r372863: [AST] Extract Decl::printNestedNameSpecifier helper from Decl…
Wed, Sep 25, 7:55 AM
ilya-biryukov committed rG1e36ed7fbcc7: [AST] Extract Decl::printNestedNameSpecifier helper from Decl… (authored by ilya-biryukov).
[AST] Extract Decl::printNestedNameSpecifier helper from Decl…
Wed, Sep 25, 6:09 AM
ilya-biryukov committed rGf96d2e175402: [clangd] A helper to find explicit references and their names (authored by ilya-biryukov).
[clangd] A helper to find explicit references and their names
Wed, Sep 25, 5:45 AM
ilya-biryukov updated the diff for D67826: [clangd] A helper to find explicit references and their names.
  • s/Decl* S/Decl* D/g
Wed, Sep 25, 5:29 AM · Restricted Project, Restricted Project
ilya-biryukov committed rGa3d337a9a7d0: Revert r372777: [libc++] Implement LWG 2510 and its follow-ups (authored by ilya-biryukov).
Revert r372777: [libc++] Implement LWG 2510 and its follow-ups
Wed, Sep 25, 2:11 AM
ilya-biryukov accepted D67973: [libTooling][NFC] Switch StencilTest.cpp to use EXPECT_THAT_EXPECTED.

LGTM

Wed, Sep 25, 1:48 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154).
Wed, Sep 25, 1:47 AM · Restricted Project

Tue, Sep 24

ilya-biryukov added a comment to D67826: [clangd] A helper to find explicit references and their names.

I believe all comments were addressed. PTAL and let me know if I missed something.

Tue, Sep 24, 10:08 AM · Restricted Project, Restricted Project