Page MenuHomePhabricator

ilya-biryukov (Ilya Biryukov)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 6 2017, 1:42 AM (124 w, 2 d)

Recent Activity

Yesterday

ilya-biryukov added inline comments to D66637: [clangd] Support multifile edits as output of Tweaks.
Fri, Aug 23, 5:48 AM · Restricted Project
ilya-biryukov added inline comments to D66637: [clangd] Support multifile edits as output of Tweaks.
Fri, Aug 23, 5:21 AM · Restricted Project
ilya-biryukov added a comment to D66125: [clang] Generalize WrapperFrontendAction to support multiple actions.

BTW It just occurred to me that multiplexing ASTConsumer::shouldSkipFunctionBody must have hit the same issue as FrontendAction multiplexing we considered above.

bool MultiplexConsumer::shouldSkipFunctionBody(Decl *D) {
  bool Skip = true;
  for (auto &Consumer : Consumers)
    Skip = Skip && Consumer->shouldSkipFunctionBody(D);
  return Skip;
}
Fri, Aug 23, 1:44 AM

Thu, Aug 22

ilya-biryukov added inline comments to D66516: [clangd] Added highlighting to types dependant on templates..
Thu, Aug 22, 9:32 AM · Restricted Project
ilya-biryukov accepted D66592: [clangd] Send suppported codeActionKinds to the client..

LGTM

Thu, Aug 22, 7:42 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D66578: Remove \brief commands from doxygen comments..

LGTM. Kill it with fire

Thu, Aug 22, 2:30 AM · Restricted Project, Restricted Project

Wed, Aug 21

ilya-biryukov added a comment to D66530: [clangd] Don't collect locations that implicitly refer to the target decl..

This actually seems useful. If one starts the search at operator bool, it's nice to see its implicit calls too.
What are the cons of having this?

Wed, Aug 21, 6:55 AM · Restricted Project
ilya-biryukov added inline comments to D66516: [clangd] Added highlighting to types dependant on templates..
Wed, Aug 21, 4:46 AM · Restricted Project
ilya-biryukov accepted D66478: [clangd] Ignore implicit conversion-operator nodes in find refs..

LGTM, thanks!

Wed, Aug 21, 2:56 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D66516: [clangd] Added highlighting to types dependant on templates..
Wed, Aug 21, 2:23 AM · Restricted Project

Tue, Aug 20

ilya-biryukov accepted D66349: [clangd] Fix one testcase in XRefsTests..

LGTM

Tue, Aug 20, 6:52 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D66470: [Syntax] Added function to get macro expansion tokens to TokenBuffer..

LGTM

Tue, Aug 20, 6:01 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D66349: [clangd] Fix one testcase in XRefsTests..
Tue, Aug 20, 4:22 AM · Restricted Project, Restricted Project
ilya-biryukov committed rG30c86b64da71: [clangd] Skip function bodies inside processed files while indexing (authored by ilya-biryukov).
[clangd] Skip function bodies inside processed files while indexing
Tue, Aug 20, 1:55 AM
ilya-biryukov updated the diff for D66226: [clangd] Skip function bodies inside processed files while indexing.
  • Change / to
Tue, Aug 20, 1:48 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D66349: [clangd] Fix one testcase in XRefsTests..
Tue, Aug 20, 1:47 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D66226: [clangd] Skip function bodies inside processed files while indexing.
Tue, Aug 20, 1:32 AM · Restricted Project, Restricted Project
ilya-biryukov updated the diff for D66226: [clangd] Skip function bodies inside processed files while indexing.
  • Expose the existing helper instead of introducing a new one
Tue, Aug 20, 1:32 AM · Restricted Project, Restricted Project

Mon, Aug 19

ilya-biryukov accepted D64741: [clangd] Added highlighting for tokens that are macro arguments..

LGTM from my side (and a few NIT, but up to you whether to apply them)

Mon, Aug 19, 9:12 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D66349: [clangd] Fix one testcase in XRefsTests..
Mon, Aug 19, 8:09 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D64741: [clangd] Added highlighting for tokens that are macro arguments..
Mon, Aug 19, 3:04 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D64741: [clangd] Added highlighting for tokens that are macro arguments..
Mon, Aug 19, 3:04 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D66349: [clangd] Fix one testcase in XRefsTests..
Mon, Aug 19, 1:44 AM · Restricted Project, Restricted Project

Fri, Aug 16

ilya-biryukov added inline comments to D66349: [clangd] Fix one testcase in XRefsTests..
Fri, Aug 16, 8:44 AM · Restricted Project, Restricted Project
ilya-biryukov committed rG12864001a652: [clangd] Simplify code of ClangdLSPServer::onCommand (authored by ilya-biryukov).
[clangd] Simplify code of ClangdLSPServer::onCommand
Fri, Aug 16, 5:50 AM
ilya-biryukov created D66343: [clangd] Simplify code of ClangdLSPServer::onCommand.
Fri, Aug 16, 5:24 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D66335: [clangd] Added special HighlightingKind for function parameters..

LGTM

Fri, Aug 16, 3:28 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D66221: [clangd] Added highlighting for non type templates..

LGTM

Fri, Aug 16, 2:27 AM · Restricted Project, Restricted Project
ilya-biryukov committed rGb3c2f5d2ee61: [clangd] Remove Bind, use C++14 lambda captures instead. NFC (authored by ilya-biryukov).
[clangd] Remove Bind, use C++14 lambda captures instead. NFC
Fri, Aug 16, 2:21 AM
ilya-biryukov added inline comments to D66221: [clangd] Added highlighting for non type templates..
Fri, Aug 16, 1:48 AM · Restricted Project, Restricted Project

Wed, Aug 14

ilya-biryukov added inline comments to D66221: [clangd] Added highlighting for non type templates..
Wed, Aug 14, 9:27 AM · Restricted Project, Restricted Project
ilya-biryukov updated the summary of D66226: [clangd] Skip function bodies inside processed files while indexing.
Wed, Aug 14, 9:19 AM · Restricted Project, Restricted Project
ilya-biryukov created D66226: [clangd] Skip function bodies inside processed files while indexing.
Wed, Aug 14, 9:16 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D66215: [clangd] Print qualifiers of out-of-line definitions in document outline.
Wed, Aug 14, 6:47 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D66211: [clangd][vscode] Surface the error when applying tweaks fails.

Thanks for providing examples of error messages, also agree they look fine.
If we find bad error messages later, we could revisit the presentation strategy.
LGTM

Wed, Aug 14, 5:56 AM · Restricted Project, Restricted Project
ilya-biryukov committed rG38fa1a916865: [clangd] Print qualifiers of out-of-line definitions in document outline (authored by ilya-biryukov).
[clangd] Print qualifiers of out-of-line definitions in document outline
Wed, Aug 14, 5:51 AM
ilya-biryukov updated the diff for D66215: [clangd] Print qualifiers of out-of-line definitions in document outline.
  • Rename the helper function
  • Remove mention of 'out-of-line definition' from a comment
Wed, Aug 14, 5:41 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D66211: [clangd][vscode] Surface the error when applying tweaks fails.

Surfacing errors to the users in those cases is definitely something we need to do, thanks!
How do they look in practice? In particular, should we add more context information (either in clangd or in the VSCode extension) to those errors, now that we know they are shown to the users?
Something like Failed to apply a fix: input range is invalid vs input range is invalid.

Wed, Aug 14, 5:25 AM · Restricted Project, Restricted Project
ilya-biryukov created D66215: [clangd] Print qualifiers of out-of-line definitions in document outline.
Wed, Aug 14, 5:08 AM · Restricted Project, Restricted Project
ilya-biryukov committed rG928bf19b65f3: [clangd] Fix typos and grammar in a comment. NFC (authored by ilya-biryukov).
[clangd] Fix typos and grammar in a comment. NFC
Wed, Aug 14, 3:51 AM
ilya-biryukov added inline comments to D66172: [clang][Modules] Serialize decl to comment mapping to speed up code completion..
Wed, Aug 14, 2:39 AM
ilya-biryukov added a comment to D66172: [clang][Modules] Serialize decl to comment mapping to speed up code completion..

This patch mentions there is code currently that "reconstructs the mapping". Why not remove it in this patch? Where is it used if we actually serialize the mapping?
Do we have any measurements? How faster is this in particular use-cases? How bigger

Wed, Aug 14, 2:36 AM

Tue, Aug 13

ilya-biryukov added a comment to D65591: [AST] Add a flag indicating if any subexpression had errors.

I did some experiments with adding something similar to the ErrorExpr Aaron suggest. It has this flag set and does not get removed from the AST.
I believe the best way to address Aaron's comments is to add tests mentioning this expression instead of trying to catch TypoExpr, which normally get removed from the AST.

Tue, Aug 13, 9:38 AM · Restricted Project
ilya-biryukov added a comment to D65591: [AST] Add a flag indicating if any subexpression had errors.

The problem is: those bits are not infinite and we only have a handful left until bumping the allocation size; is this use case critical enough to use one of those bits? I don't think it will be -- it seems like premature optimization. Also, by calculating rather than using a bit, you don't have to touch every Expr constructor, which reduces the complexity of the patch.

Alternatively, we could keep an equivalent of set<Expr*> InvalidExprs in ASTContext. Gives the same computational complexity, but has a higher constant overhead factor.
Does that look reasonable?

Tue, Aug 13, 1:25 AM · Restricted Project

Mon, Aug 12

ilya-biryukov added a comment to D66087: [clangd] Preserve line break when rendering text chunks of markdown.

The heuristics for properly rendering the comments are probably a good way to go.
I'd say this change is still a step in the right direction - text blocks in formatted strings should be properly escaped to avoid being interpreted like markdown constructs.
Any structure we want to have should be explicit.

Mon, Aug 12, 7:47 AM · Restricted Project
ilya-biryukov committed rG119d1c278cf8: [clangd] Separate chunks with a space when rendering markdown (authored by ilya-biryukov).
[clangd] Separate chunks with a space when rendering markdown
Mon, Aug 12, 7:37 AM
ilya-biryukov added a comment to D65591: [AST] Add a flag indicating if any subexpression had errors.

It seems that these two options are not exactly the same right ? The ContainsError bit is useful to quickly answer "Does this expression contains an invalid sub-expression" without doing the search, while adding an ErrorExpr node is useful to note that this sub-expression is invalid (and as Aaron says the hypothetical ErrorExpr node can carry more info about the error).

That's true. I had figured that answering "does this expression contain an invalid sub-expression" could be implemented with a walk of the expression tree rather than consuming a bit. To consumers of containsErrors(), there shouldn't be much difference aside from performance (and that may be sufficient reason to go with a bit, but I think I'd like to see performance measurements that show the bit is necessary).

Are expression bits scarce?
We don't any checks if expressions contain errors now, we simply drop too many invalid expressions and never put them into the AST.
It's impossible to do the measurements at this point, but it would be nice if adding those checks was cheap.

Mon, Aug 12, 7:10 AM · Restricted Project
ilya-biryukov added a comment to D65591: [AST] Add a flag indicating if any subexpression had errors.

It seems that these two options are not exactly the same right ? The ContainsError bit is useful to quickly answer "Does this expression contains an invalid sub-expression" without doing the search, while adding an ErrorExpr node is useful to note that this sub-expression is invalid (and as Aaron says the hypothetical ErrorExpr node can carry more info about the error).

Exactly right, thanks for summing this up!

Mon, Aug 12, 6:43 AM · Restricted Project
ilya-biryukov created D66087: [clangd] Preserve line break when rendering text chunks of markdown.
Mon, Aug 12, 6:29 AM · Restricted Project
ilya-biryukov created D66086: [clangd] Separate chunks with a space when rendering markdown.
Mon, Aug 12, 6:09 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D66083: [clangd] Remove highlightings coming from non topLevelDecls from included files..

LGTM. Great example for the test case! It will definitely stay valid even if we fix all problems caused by RecursiveASTVisitor!

Mon, Aug 12, 5:25 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D65591: [AST] Add a flag indicating if any subexpression had errors.

Gentle ping. @rsmith, please take a look when you have time.

Mon, Aug 12, 4:59 AM · Restricted Project
ilya-biryukov updated subscribers of D65752: [Sema] Refactor LookupVisibleDecls. NFC.

This looks reasonable to me, there are a couple of variations you might think about:

  • also treat QualifiedNameLookup as an option, and override in places with an RAII pattern like ScopedOverride Unqual(QualifiedNameLookup, false) (why don't we have a class like that?). This would further reduce args.

I actually think it's good to have it as an explicit option. Qualified and unqualified lookup are so vastly different use-cases, that having the flag that discriminates them as a function parameter looks like a better trade-off.
Besides, RAII objects are more code and more complicated.

Mon, Aug 12, 2:53 AM · Restricted Project
ilya-biryukov accepted D66074: [clangd] Drop diags from non-written #include..

LGTM, thanks!
We should also merge this into the release branch if it's not too late yet.

Mon, Aug 12, 2:01 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D65373: [clangd] Update features table in the docs with links to LSP extension proposals.
Mon, Aug 12, 1:25 AM · Restricted Project, Restricted Project

Wed, Aug 7

ilya-biryukov accepted D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls..

LGTM, but please print the function template arguments uniformly before landing this (to avoid different forms of outputs inside the same test).

Wed, Aug 7, 11:05 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls..
Wed, Aug 7, 7:48 AM · Restricted Project, Restricted Project
ilya-biryukov committed rG843280bfe30e: [unittests] Mark private gmock headers with IWYU pragmas. NFC (authored by ilya-biryukov).
[unittests] Mark private gmock headers with IWYU pragmas. NFC
Wed, Aug 7, 1:46 AM
ilya-biryukov added inline comments to D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls..
Wed, Aug 7, 1:32 AM · Restricted Project, Restricted Project
ilya-biryukov created D65849: [unittests] Mark private gmock headers with IWYU pragmas. NFC.
Wed, Aug 7, 1:25 AM · Restricted Project

Tue, Aug 6

ilya-biryukov committed rGbfbf6b6cab9b: [Syntax] Do not add a node for 'eof' into the tree (authored by ilya-biryukov).
[Syntax] Do not add a node for 'eof' into the tree
Tue, Aug 6, 10:10 AM
ilya-biryukov committed rG4b03364d72a5: [AST] Traverse attributes inside DEF_TRAVERSE_DECL macro (authored by ilya-biryukov).
[AST] Traverse attributes inside DEF_TRAVERSE_DECL macro
Tue, Aug 6, 8:48 AM
ilya-biryukov added a comment to D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls..

One important comment about somehow distinguishing multiple decls with the same name.

Tue, Aug 6, 7:33 AM · Restricted Project, Restricted Project
ilya-biryukov committed rG56bdb0c50825: [clangd] Compute scopes eagerly in IncludeFixer (authored by ilya-biryukov).
[clangd] Compute scopes eagerly in IncludeFixer
Tue, Aug 6, 4:40 AM
ilya-biryukov added inline comments to D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls..
Tue, Aug 6, 3:23 AM · Restricted Project, Restricted Project
ilya-biryukov updated the diff for D64907: [AST] Traverse attributes inside DEF_TRAVERSE_DECL macro.
  • Add the missing newline
Tue, Aug 6, 3:04 AM · Restricted Project, Restricted Project
ilya-biryukov updated the diff for D65796: [clangd] Compute scopes eagerly in IncludeFixer.
  • Make sure the test actually runs IncludeFixer.cpp
Tue, Aug 6, 2:59 AM · Restricted Project, Restricted Project
ilya-biryukov created D65796: [clangd] Compute scopes eagerly in IncludeFixer.
Tue, Aug 6, 2:48 AM · Restricted Project, Restricted Project

Mon, Aug 5

ilya-biryukov added a comment to D65752: [Sema] Refactor LookupVisibleDecls. NFC.

This is just a proposal, there are probably other ways to reach better readability, e.g. group some of those parameters into a struct.
But let me know what you think, happy to refactor in a slightly different manner or simply drop this revision.

Mon, Aug 5, 8:26 AM · Restricted Project
ilya-biryukov updated the diff for D65752: [Sema] Refactor LookupVisibleDecls. NFC.
  • Remove accidental change from revision
Mon, Aug 5, 8:22 AM · Restricted Project
ilya-biryukov created D65752: [Sema] Refactor LookupVisibleDecls. NFC.
Mon, Aug 5, 8:20 AM · Restricted Project
ilya-biryukov added inline comments to D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls..
Mon, Aug 5, 7:51 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls..
Mon, Aug 5, 7:46 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D65735: [AST] Fix RecursiveASTVisitor visiting implicit constructor initializers..

LGTM from my side, a few optional NITs.
Feel free to land as soon as @hokein stamps.

Mon, Aug 5, 2:44 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D65735: [AST] Fix RecursiveASTVisitor visiting implicit constructor initializers..

The fix looks good.

Mon, Aug 5, 1:37 AM · Restricted Project, Restricted Project

Fri, Aug 2

ilya-biryukov added a comment to D65517: [Preprocessor] Always discard body of #define if we failed to parse it.

Thanks!

Fri, Aug 2, 9:35 AM · Restricted Project, Restricted Project
ilya-biryukov committed rG25082817eb6b: [clangd] Fix a crash when presenting values for Hover (authored by ilya-biryukov).
[clangd] Fix a crash when presenting values for Hover
Fri, Aug 2, 8:28 AM
ilya-biryukov updated subscribers of D65655: [clangd] Fix a crash when presenting values for Hover.

@hans, could we merge this commit into the release branch?

Fri, Aug 2, 8:27 AM · Restricted Project, Restricted Project
ilya-biryukov created D65655: [clangd] Fix a crash when presenting values for Hover.
Fri, Aug 2, 6:28 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D65525: [clangd] Add new helpers to make tweak tests scale better. Convert most tests. NFC.

LGTM

Fri, Aug 2, 2:08 AM · Restricted Project, Restricted Project
ilya-biryukov updated subscribers of D65625: [clangd] Allow the client to specify multiple compilation databases in the 'initialize' request.

Could this be handled outside clangd? If not, what are the complications?

Fri, Aug 2, 1:53 AM · Restricted Project
ilya-biryukov added inline comments to D65525: [clangd] Add new helpers to make tweak tests scale better. Convert most tests. NFC.
Fri, Aug 2, 1:39 AM · Restricted Project, Restricted Project

Thu, Aug 1

ilya-biryukov added a child revision for D65591: [AST] Add a flag indicating if any subexpression had errors: D65592: [Parser] Avoid spurious 'missing template' error in presence of typos.
Thu, Aug 1, 10:29 AM · Restricted Project
ilya-biryukov added a parent revision for D65592: [Parser] Avoid spurious 'missing template' error in presence of typos: D65591: [AST] Add a flag indicating if any subexpression had errors.
Thu, Aug 1, 10:29 AM · Restricted Project
ilya-biryukov created D65592: [Parser] Avoid spurious 'missing template' error in presence of typos.
Thu, Aug 1, 10:29 AM · Restricted Project
ilya-biryukov created D65591: [AST] Add a flag indicating if any subexpression had errors.
Thu, Aug 1, 10:29 AM · Restricted Project
ilya-biryukov added a comment to D65525: [clangd] Add new helpers to make tweak tests scale better. Convert most tests. NFC.

Neat, thanks for the cleanup! A few suggestions from me, no major comments.

Thu, Aug 1, 5:58 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls..
Thu, Aug 1, 5:45 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D65517: [Preprocessor] Always discard body of #define if we failed to parse it.

SG, just wanted to make sure it's on your list ;-)

Thu, Aug 1, 4:23 AM · Restricted Project, Restricted Project
ilya-biryukov committed rG2fe0a14b5a27: [clangd] Add missing braces to completion tests. NFC (authored by ilya-biryukov).
[clangd] Add missing braces to completion tests. NFC
Thu, Aug 1, 4:06 AM
ilya-biryukov updated subscribers of D65517: [Preprocessor] Always discard body of #define if we failed to parse it.

@hans, could you please merge rL367530 into the release?

Thu, Aug 1, 2:43 AM · Restricted Project, Restricted Project
ilya-biryukov committed rGb455fc429fe9: [Preprocessor] Always discard body of #define if we failed to parse it (authored by ilya-biryukov).
[Preprocessor] Always discard body of #define if we failed to parse it
Thu, Aug 1, 2:15 AM
ilya-biryukov updated the diff for D65517: [Preprocessor] Always discard body of #define if we failed to parse it.
  • Update a comment
Thu, Aug 1, 2:00 AM · Restricted Project, Restricted Project

Wed, Jul 31

ilya-biryukov updated the summary of D65517: [Preprocessor] Always discard body of #define if we failed to parse it.
Wed, Jul 31, 9:54 AM · Restricted Project, Restricted Project
ilya-biryukov added inline comments to D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls..
Wed, Jul 31, 9:54 AM · Restricted Project, Restricted Project
ilya-biryukov accepted D64475: [clangd] Duplicate lines of semantic highlightings sent removed..

LGTM from my side

Wed, Jul 31, 9:01 AM · Restricted Project, Restricted Project
ilya-biryukov updated the diff for D64907: [AST] Traverse attributes inside DEF_TRAVERSE_DECL macro.
  • Reformat
Wed, Jul 31, 8:45 AM · Restricted Project, Restricted Project
ilya-biryukov updated the diff for D64907: [AST] Traverse attributes inside DEF_TRAVERSE_DECL macro.
  • Add a test
Wed, Jul 31, 8:43 AM · Restricted Project, Restricted Project
ilya-biryukov created D65517: [Preprocessor] Always discard body of #define if we failed to parse it.
Wed, Jul 31, 8:00 AM · Restricted Project, Restricted Project
ilya-biryukov added a comment to D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU.

An experiment with popping on expression evaluation context failed, I couldn't find a good way to fix the problems described above.
Typo correction can and will run after the evaluation context where expression created is popped and the diagnostic we produce depends on the results of typo correction.
Emitting diagnostics on some, but not all context pops could be an option, but classifying those seems to be hard, would require looking closely at every expr eval context push.

Wed, Jul 31, 3:40 AM · Restricted Project