Page MenuHomePhabricator

hokein (Haojian Wu)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 19 2015, 3:38 AM (173 w, 2 d)

Recent Activity

Today

hokein added a comment to D55185: [Clangd] Index main-file symbols (bug 39761).

! In D55185#1330328, @nridge wrote:

Thanks!

So, on the clangd codebase, I got the following measurements for indexing with this patch compared to indexing without this patch:

 
- Size of index on disk increased by 3.2%
- Memory usage as reported by `dexp` increased by 3.6%
Fri, Dec 14, 6:38 AM
hokein accepted D55705: [dexp] Change FuzzyFind to also print scope of symbols.
Fri, Dec 14, 6:19 AM
hokein added a comment to D55044: [clang-tidy] check for Abseil make_unique.

In D55044#1329604, @hokein wrote:
In fact, the existing modernize-make-unique can be configured to support absl::make_unique, you'd just need to configure the check option MakeSmartPtrFunction to absl::make_unique (this is what we do inside google).

See https://reviews.llvm.org/D52670#change-eVDYJhWSHWeW (the getModuleOptions() part) on how to do that

Fri, Dec 14, 6:17 AM · Restricted Project
hokein committed rL349148: [clangd] Use buildCompilerInvocation to simplify the HeadersTests, NFC..
[clangd] Use buildCompilerInvocation to simplify the HeadersTests, NFC.
Fri, Dec 14, 5:52 AM
hokein committed rCTE349148: [clangd] Use buildCompilerInvocation to simplify the HeadersTests, NFC..
[clangd] Use buildCompilerInvocation to simplify the HeadersTests, NFC.
Fri, Dec 14, 5:52 AM
hokein committed rCTE349145: [clangd] Fix memory leak in ClangdTests..
[clangd] Fix memory leak in ClangdTests.
Fri, Dec 14, 5:22 AM
hokein committed rL349145: [clangd] Fix memory leak in ClangdTests..
[clangd] Fix memory leak in ClangdTests.
Fri, Dec 14, 5:22 AM
hokein closed D55702: [clangd] Fix memory leak in ClangdTests..
Fri, Dec 14, 5:22 AM
hokein committed rCTE349144: [clangd] Fix an assertion failure in background index..
[clangd] Fix an assertion failure in background index.
Fri, Dec 14, 4:42 AM
hokein committed rL349144: [clangd] Fix an assertion failure in background index..
[clangd] Fix an assertion failure in background index.
Fri, Dec 14, 4:42 AM
hokein closed D55650: [clangd] Fix an assertion failure in background index..
Fri, Dec 14, 4:42 AM
hokein added a comment to D55650: [clangd] Fix an assertion failure in background index..

Thanks for the review.

Fri, Dec 14, 4:39 AM
hokein updated subscribers of D55702: [clangd] Fix memory leak in ClangdTests..
Fri, Dec 14, 4:38 AM
hokein created D55702: [clangd] Fix memory leak in ClangdTests..
Fri, Dec 14, 4:37 AM
hokein added inline comments to D55650: [clangd] Fix an assertion failure in background index..
Fri, Dec 14, 3:14 AM
hokein added a comment to D55650: [clangd] Fix an assertion failure in background index..

I think the description is misleading, you are saying that we were triggering an assertion and you are fixing that. But in reality, we were not triggering any assertions, this patch is adding the assertions right?

Fri, Dec 14, 1:41 AM
hokein updated the diff for D55650: [clangd] Fix an assertion failure in background index..

Keep the old behavior.

Fri, Dec 14, 1:41 AM

Yesterday

hokein added a comment to D55545: Allow IncludeSorter to use #import for Objective-C files.

Please add a unittest for it, you can add it in unittests/clang-tidy/IncludeInserterTest.cpp.

Thu, Dec 13, 10:06 AM
hokein added a reviewer for D55545: Allow IncludeSorter to use #import for Objective-C files: benhamilton.
Thu, Dec 13, 10:05 AM
hokein added a reviewer for D55545: Allow IncludeSorter to use #import for Objective-C files: hokein.
Thu, Dec 13, 9:59 AM
hokein committed rL349033: [clangd] Refine the way of checking a declaration is referenced by the written….
[clangd] Refine the way of checking a declaration is referenced by the written…
Thu, Dec 13, 5:21 AM
hokein committed rCTE349033: [clangd] Refine the way of checking a declaration is referenced by the written….
[clangd] Refine the way of checking a declaration is referenced by the written…
Thu, Dec 13, 5:21 AM
hokein closed D55191: [clangd] Refine the way of checking a declaration is referenced by the written code..
Thu, Dec 13, 5:21 AM
hokein committed rCTE349032: [clangd] Avoid emitting Queued status when we are able to acquire the Barrier..
[clangd] Avoid emitting Queued status when we are able to acquire the Barrier.
Thu, Dec 13, 5:13 AM
hokein committed rL349032: [clangd] Avoid emitting Queued status when we are able to acquire the Barrier..
[clangd] Avoid emitting Queued status when we are able to acquire the Barrier.
Thu, Dec 13, 5:13 AM
hokein closed D55359: [clangd] Avoid emitting Queued status when we are able to acquire the Barrier..
Thu, Dec 13, 5:13 AM
hokein committed rCTE349031: [clangd] Move the utility function to anonymous namespace, NFC..
[clangd] Move the utility function to anonymous namespace, NFC.
Thu, Dec 13, 5:13 AM
hokein committed rL349031: [clangd] Move the utility function to anonymous namespace, NFC..
[clangd] Move the utility function to anonymous namespace, NFC.
Thu, Dec 13, 5:10 AM
hokein added a comment to D55044: [clang-tidy] check for Abseil make_unique.

In fact, the existing modernize-make-unique can be configured to support absl::make_unique, you'd just need to configure the check option MakeSmartPtrFunction to absl::make_unique (this is what we do inside google).

Thu, Dec 13, 5:06 AM · Restricted Project
hokein accepted D55649: [clangd] Enable cross-namespace completions by default in clangd.
Thu, Dec 13, 4:56 AM
hokein created D55650: [clangd] Fix an assertion failure in background index..
Thu, Dec 13, 4:54 AM

Wed, Dec 12

hokein added a reviewer for D55185: [Clangd] Index main-file symbols (bug 39761): hokein.
Wed, Dec 12, 2:10 AM
hokein added a comment to D55185: [Clangd] Index main-file symbols (bug 39761).

AFAIK, the symbol (section) is not the largest part of index (we have 30K symbols), would be interesting to know the (index, memory) size (including symbols in anonymous namespace).

Do you have a suggestion for how to measure memory usage? Should I just look at the memory usage of the clangd process in top?

Wed, Dec 12, 2:10 AM

Fri, Dec 7

hokein added inline comments to D55359: [clangd] Avoid emitting Queued status when we are able to acquire the Barrier..
Fri, Dec 7, 7:49 AM
hokein updated the diff for D55359: [clangd] Avoid emitting Queued status when we are able to acquire the Barrier..

Address review comment.

Fri, Dec 7, 7:49 AM
hokein created D55437: [Index] Index declarations in lambda expression..
Fri, Dec 7, 7:44 AM
hokein added a comment to D55191: [clangd] Refine the way of checking a declaration is referenced by the written code..

What are the cases we're trying to filter out? Only implicit constructor or anything else?

Fri, Dec 7, 5:42 AM
hokein updated the diff for D55191: [clangd] Refine the way of checking a declaration is referenced by the written code..

Update based on our offline discussion.

Fri, Dec 7, 5:39 AM
hokein committed rCTE348583: [clang-tidy] Remove duplicated getText implementation, NFC.
[clang-tidy] Remove duplicated getText implementation, NFC
Fri, Dec 7, 3:28 AM
hokein committed rL348583: [clang-tidy] Remove duplicated getText implementation, NFC.
[clang-tidy] Remove duplicated getText implementation, NFC
Fri, Dec 7, 3:28 AM
hokein updated subscribers of D53830: [clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'.

+ @hwright who have implemented a bunch of absl-duration-* checks, you might be interested in this as well.

Fri, Dec 7, 2:58 AM · Restricted Project
hokein added a reviewer for D55409: [clang-tidy] check for using declarations not in an anonymous namespace when there exists one: ahedberg.
Fri, Dec 7, 2:36 AM
hokein accepted D55245: [clang-tidy] Add the abseil-duration-subtraction check.

The check looks good from my side, except one nit.

Fri, Dec 7, 2:34 AM · Restricted Project

Thu, Dec 6

hokein updated the diff for D55363: [clangd] Expose FileStatus in LSP..

Update.

Thu, Dec 6, 8:24 AM
hokein updated the summary of D55374: [clangd] Show FileStatus in vscode-clangd..
Thu, Dec 6, 8:23 AM
hokein created D55374: [clangd] Show FileStatus in vscode-clangd..
Thu, Dec 6, 8:21 AM
hokein added a child revision for D55363: [clangd] Expose FileStatus in LSP.: D55374: [clangd] Show FileStatus in vscode-clangd..
Thu, Dec 6, 8:21 AM
hokein created D55363: [clangd] Expose FileStatus in LSP..
Thu, Dec 6, 4:45 AM
hokein created D55359: [clangd] Avoid emitting Queued status when we are able to acquire the Barrier..
Thu, Dec 6, 2:31 AM
hokein committed rCTE348478: [clangd] Update the test code.
[clangd] Update the test code
Thu, Dec 6, 2:27 AM
hokein committed rL348478: [clangd] Update the test code.
[clangd] Update the test code
Thu, Dec 6, 2:27 AM
hokein closed D54796: [clangd] C++ API for emitting file status.
Thu, Dec 6, 2:23 AM
hokein added a comment to D54796: [clangd] C++ API for emitting file status.

Forgot to associate this patch to the actual commit, committed in rL348475.

Thu, Dec 6, 2:23 AM
hokein committed rCTE348475: [clangd] C++ API for emitting file status..
[clangd] C++ API for emitting file status.
Thu, Dec 6, 1:44 AM
hokein committed rL348475: [clangd] C++ API for emitting file status..
[clangd] C++ API for emitting file status.
Thu, Dec 6, 1:44 AM
hokein added inline comments to D54796: [clangd] C++ API for emitting file status.
Thu, Dec 6, 1:26 AM
hokein updated the summary of D54796: [clangd] C++ API for emitting file status.
Thu, Dec 6, 1:26 AM
hokein updated the diff for D54796: [clangd] C++ API for emitting file status.

Address comments.

Thu, Dec 6, 1:25 AM
hokein committed rCTE348467: [clangd] Fix a typo in TUSchedulerTests.
[clangd] Fix a typo in TUSchedulerTests
Thu, Dec 6, 12:59 AM
hokein committed rL348467: [clangd] Fix a typo in TUSchedulerTests.
[clangd] Fix a typo in TUSchedulerTests
Thu, Dec 6, 12:59 AM
hokein closed D55312: [clangd] Fix a typo in TUSchedulerTests.
Thu, Dec 6, 12:59 AM

Wed, Dec 5

hokein accepted D55275: [clangd] Dont provide locations for non-existent files..

thanks!

Wed, Dec 5, 1:24 AM
hokein created D55312: [clangd] Fix a typo in TUSchedulerTests.
Wed, Dec 5, 1:24 AM
hokein added inline comments to D55250: [clangd] Enhance macro hover to see full definition.
Wed, Dec 5, 12:21 AM
hokein added inline comments to D55275: [clangd] Dont provide locations for non-existent files..
Wed, Dec 5, 12:05 AM

Tue, Dec 4

hokein updated the diff for D55256: [clangd] Support clang-tidy configuration in clangd..

Minor cleanup

Tue, Dec 4, 3:25 AM
hokein added inline comments to D54796: [clangd] C++ API for emitting file status.
Tue, Dec 4, 3:10 AM
hokein updated the diff for D54796: [clangd] C++ API for emitting file status.

Address review comments

  • remove Unknown enum type
  • make TUState only accessed by the worker thread.
Tue, Dec 4, 3:10 AM
hokein created D55256: [clangd] Support clang-tidy configuration in clangd..
Tue, Dec 4, 1:53 AM

Mon, Dec 3

hokein committed rCTE348133: [clangd] Fix a stale comment, NFC..
[clangd] Fix a stale comment, NFC.
Mon, Dec 3, 5:19 AM
hokein committed rL348133: [clangd] Fix a stale comment, NFC..
[clangd] Fix a stale comment, NFC.
Mon, Dec 3, 5:19 AM
hokein added a comment to D55185: [Clangd] Index main-file symbols (bug 39761).

This is a first attempt at a patch for this.

There are some cases this doesn't handle, which I was thinking of handling in follow-up commits (but we could do them here if desired):

  • Entities in anonymous namespaces are still not indexed, There is a comment in SymbolCollector::shouldCollectSymbol() which says "Also we skip the symbols in anonymous namespace as the qualifier names of these symbols are like foo::<anonymous>::bar, which need a special handling." It's not clear to me how that should be handled, suggestions are welcome.
Mon, Dec 3, 5:06 AM
hokein committed rCTE348130: [clangd] Get rid of AST matchers in CodeComplete, NFC.
[clangd] Get rid of AST matchers in CodeComplete, NFC
Mon, Dec 3, 5:00 AM
hokein committed rL348130: [clangd] Get rid of AST matchers in CodeComplete, NFC.
[clangd] Get rid of AST matchers in CodeComplete, NFC
Mon, Dec 3, 4:57 AM
hokein closed D55206: [clangd] Get rid of AST matchers in CodeComplete, NFC.
Mon, Dec 3, 4:57 AM
hokein updated the diff for D55206: [clangd] Get rid of AST matchers in CodeComplete, NFC.

Using break.

Mon, Dec 3, 4:55 AM
hokein created D55206: [clangd] Get rid of AST matchers in CodeComplete, NFC.
Mon, Dec 3, 2:48 AM
hokein created D55191: [clangd] Refine the way of checking a declaration is referenced by the written code..
Mon, Dec 3, 2:09 AM

Fri, Nov 30

hokein added inline comments to D54796: [clangd] C++ API for emitting file status.
Fri, Nov 30, 6:43 AM
hokein updated the diff for D54796: [clangd] C++ API for emitting file status.

Address comments and fix a bug in the test.

Fri, Nov 30, 6:41 AM
hokein added inline comments to D54796: [clangd] C++ API for emitting file status.
Fri, Nov 30, 5:26 AM
hokein updated the diff for D54796: [clangd] C++ API for emitting file status.

Fix nits.

Fri, Nov 30, 5:25 AM
hokein accepted D55061: [clangd] Penalize destructor and overloaded operators in code completion..
Fri, Nov 30, 2:00 AM
hokein added a comment to D55052: Fix junk output in clangd vscode plugin.

Yeah, there are still modes where clangd behaves badly there. Note that the output from that pane is rarely useful anyway as clangd keeps producing errors about accessing non-open files, which would confuse people even more.
Overall we never designed this output to be readable by the users, so it's not really useful anyway. There are ways to communicate with the users in the protocol, we should be using them instead to be more user-friendly

Fri, Nov 30, 1:30 AM · Restricted Project
hokein committed rL347970: Fix a use-after-scope bug..
Fix a use-after-scope bug.
Fri, Nov 30, 1:26 AM
hokein committed rC347970: Fix a use-after-scope bug..
Fix a use-after-scope bug.
Fri, Nov 30, 1:26 AM
hokein committed rCTE347969: [clangd] Bump vscode-clangd v0.0.8.
[clangd] Bump vscode-clangd v0.0.8
Fri, Nov 30, 1:21 AM
hokein committed rL347969: [clangd] Bump vscode-clangd v0.0.8.
[clangd] Bump vscode-clangd v0.0.8
Fri, Nov 30, 1:21 AM
hokein committed rCTE347968: [clangd] Fix junk output in clangd vscode plugin.
[clangd] Fix junk output in clangd vscode plugin
Fri, Nov 30, 1:19 AM
hokein committed rL347968: [clangd] Fix junk output in clangd vscode plugin.
[clangd] Fix junk output in clangd vscode plugin
Fri, Nov 30, 1:19 AM
hokein closed D55052: Fix junk output in clangd vscode plugin.
Fri, Nov 30, 1:18 AM · Restricted Project

Thu, Nov 29

hokein added a comment to D55052: Fix junk output in clangd vscode plugin.

+1 to the change, this is annoying for me too.

@jfindley I'd like to understand how do these log messages noise you? Does the window prompt up to you automatically (this only happens when there is an error in clangd)? These messages only get print to the clangd-vscode output panel which is used for debugging purposes (normal users won't open this window by their own).

It's fine to have this open for debugging purposes, the real problem is that it pops up whenever clangd send an error response. We do this a lot now that we support cancellation and the default policy is to show an output window in that case as well.

Thu, Nov 29, 6:42 AM · Restricted Project
hokein added a comment to D55052: Fix junk output in clangd vscode plugin.

@jfindley I'd like to understand how do these log messages noise you? Does the window prompt up to you automatically (this only happens when there is an error in clangd)? These messages only get print to the clangd-vscode output panel which is used for debugging purposes (normal users won't open this window by their own).

Thu, Nov 29, 6:17 AM · Restricted Project
hokein added a comment to D54796: [clangd] C++ API for emitting file status.

@ilya-biryukov, I hope the current patch is not too big for you to review, happy to chat offline if you want (sam and I had a lot of discussions before he is OOO).

Thu, Nov 29, 5:50 AM
hokein updated the diff for D54796: [clangd] C++ API for emitting file status.
  • address review comments
  • drop the LSP change, only focus on TUScheduler in this patch
Thu, Nov 29, 5:45 AM

Wed, Nov 28

hokein committed rL347753: [clangd] Build and test IndexBenchmark in check-clangd.
[clangd] Build and test IndexBenchmark in check-clangd
Wed, Nov 28, 5:34 AM
hokein committed rCTE347753: [clangd] Build and test IndexBenchmark in check-clangd.
[clangd] Build and test IndexBenchmark in check-clangd
Wed, Nov 28, 5:34 AM
hokein closed D54998: [clangd] Build and test IndexBenchmark in check-clangd.
Wed, Nov 28, 5:33 AM
hokein committed rL347751: Fix a typo..
Fix a typo.
Wed, Nov 28, 5:24 AM
hokein committed rLLD347751: Fix a typo..
Fix a typo.
Wed, Nov 28, 5:24 AM