Page MenuHomePhabricator

ArcsinX (Aleksandr Platonov)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 13 2019, 11:28 AM (50 w, 2 d)

Recent Activity

Thu, Nov 26

ArcsinX committed rG1ca174b6420a: [clangd][query-driver] Extract target (authored by ArcsinX).
[clangd][query-driver] Extract target
Thu, Nov 26, 4:09 AM
ArcsinX closed D92012: [clangd][query-driver] Extract target.
Thu, Nov 26, 4:09 AM · Restricted Project
ArcsinX added a comment to D92012: [clangd][query-driver] Extract target.

Thanks for review!

Thu, Nov 26, 4:02 AM · Restricted Project
ArcsinX updated the summary of D92012: [clangd][query-driver] Extract target.
Thu, Nov 26, 3:57 AM · Restricted Project
ArcsinX added inline comments to D92012: [clangd][query-driver] Extract target.
Thu, Nov 26, 12:38 AM · Restricted Project
ArcsinX updated the diff for D92012: [clangd][query-driver] Extract target.

Do not override existing target.
Fix clang-tidy warning.
Address review comments.

Thu, Nov 26, 12:30 AM · Restricted Project

Wed, Nov 25

ArcsinX added inline comments to D92012: [clangd][query-driver] Extract target.
Wed, Nov 25, 4:56 AM · Restricted Project
ArcsinX updated the diff for D92012: [clangd][query-driver] Extract target.

Add structure to hold system includes and target.
Use state machine to go though lines of the driver output.
Handle invalid target
Use '--target=X' instead of '-target X'

Wed, Nov 25, 4:49 AM · Restricted Project

Tue, Nov 24

ArcsinX added reviewers for D92012: [clangd][query-driver] Extract target: sammccall, kadircet.

I'm not sure if this solution is elegant enough. I will be happy to hear advices.

Tue, Nov 24, 1:30 AM · Restricted Project
ArcsinX requested review of D92012: [clangd][query-driver] Extract target.
Tue, Nov 24, 1:27 AM · Restricted Project

Wed, Nov 11

ArcsinX committed rGdad804a193ed: [clangd] Improve clangd-indexer performance (authored by ArcsinX).
[clangd] Improve clangd-indexer performance
Wed, Nov 11, 3:39 AM
ArcsinX closed D91051: [clangd] Improve clangd-indexer performance.
Wed, Nov 11, 3:39 AM · Restricted Project

Tue, Nov 10

ArcsinX updated the diff for D91051: [clangd] Improve clangd-indexer performance.

Don't check that AbsPath is not in Files twice.

Tue, Nov 10, 11:00 AM · Restricted Project

Mon, Nov 9

ArcsinX added a comment to D90116: [llvm] CMake: Force MSVC to read code as UTF-8.
Mon, Nov 9, 11:37 PM · Restricted Project, Restricted Project
ArcsinX committed rG1bbf87e22a73: [clangd][remote] Check an index file correctly (authored by ArcsinX).
[clangd][remote] Check an index file correctly
Mon, Nov 9, 10:43 AM
ArcsinX closed D91049: [clangd][remote] Check an index file correctly.
Mon, Nov 9, 10:43 AM · Restricted Project
ArcsinX added a comment to D91051: [clangd] Improve clangd-indexer performance.

Thanks you all for your comments.

Mon, Nov 9, 4:57 AM · Restricted Project
ArcsinX updated the diff for D91049: [clangd][remote] Check an index file correctly.

Do not use unique pointer for Index

Mon, Nov 9, 12:34 AM · Restricted Project
ArcsinX added reviewers for D91051: [clangd] Improve clangd-indexer performance: sammccall, kadircet, hokein.
Mon, Nov 9, 12:16 AM · Restricted Project
ArcsinX requested review of D91051: [clangd] Improve clangd-indexer performance.
Mon, Nov 9, 12:15 AM · Restricted Project
ArcsinX added reviewers for D91049: [clangd][remote] Check an index file correctly: kbobyrev, sammccall, kadircet.
Mon, Nov 9, 12:01 AM · Restricted Project

Sun, Nov 8

ArcsinX requested review of D91049: [clangd][remote] Check an index file correctly.
Sun, Nov 8, 11:59 PM · Restricted Project
ArcsinX added inline comments to D90116: [llvm] CMake: Force MSVC to read code as UTF-8.
Sun, Nov 8, 11:26 PM · Restricted Project, Restricted Project

Tue, Nov 3

ArcsinX added inline comments to D90654: [clangd] Add index server request logging.
Tue, Nov 3, 3:22 AM · Restricted Project

Mon, Nov 2

ArcsinX added a comment to D90116: [llvm] CMake: Force MSVC to read code as UTF-8.

Hmm, I see. From the looks of it, the solution for several projects would be

add_compile_options("$<$<C_COMPILER_ID:MSVC>:/utf-8>")
add_compile_options("$<$<CXX_COMPILER_ID:MSVC>:/utf-8>")

But I'm not sure if it makes sense in our case and I don't see many add_compile_options in LLVM. Also, I don't have a way to test it out.

Mon, Nov 2, 5:12 AM · Restricted Project, Restricted Project

Oct 29 2020

ArcsinX added inline comments to D90291: [clangd] Add lit tests for remote index.
Oct 29 2020, 7:51 AM · Restricted Project
ArcsinX added inline comments to D90291: [clangd] Add lit tests for remote index.
Oct 29 2020, 4:12 AM · Restricted Project

Oct 25 2020

ArcsinX added a comment to D90116: [llvm] CMake: Force MSVC to read code as UTF-8.

I have face this problem long time ago.
This happens only:

  • on non-English Windows (and could be fixed via system settings: Control Panel -> Clock,Language,and Region -> Region and Language -> Administrative ->Language for non-Unicode programs -> Change system locale -> English).
  • with Visual Studio Compiler
Oct 25 2020, 10:49 AM · Restricted Project, Restricted Project

Oct 23 2020

ArcsinX added a comment to D90016: [clangd] NFC: Add using directives to avoid spelling out llvm::sys::path.

The ones you proposed would not work without using llvm::sys::path::* which I avoid :( They have to have an argument from llvm::sys::path:: in order for the lookup to be performed correctly.

Oct 23 2020, 3:05 AM · Restricted Project
ArcsinX added inline comments to D90016: [clangd] NFC: Add using directives to avoid spelling out llvm::sys::path.
Oct 23 2020, 2:54 AM · Restricted Project
ArcsinX added inline comments to D89852: [clangd] Get rid of llvm::Optional in Remote- and LocalIndexRoot; NFC.
Oct 23 2020, 2:27 AM · Restricted Project
ArcsinX added inline comments to D89852: [clangd] Get rid of llvm::Optional in Remote- and LocalIndexRoot; NFC.
Oct 23 2020, 2:13 AM · Restricted Project

Oct 20 2020

ArcsinX committed rGd99b2a976a37: [clangd][remote] Add Windows paths support (authored by ArcsinX).
[clangd][remote] Add Windows paths support
Oct 20 2020, 3:05 AM
ArcsinX closed D89529: [clangd][remote] Add Windows paths support.
Oct 20 2020, 3:05 AM · Restricted Project

Oct 19 2020

ArcsinX added a comment to D89529: [clangd][remote] Add Windows paths support.

Thank you for the patch! Ready to land it now.

Thank you for review!

Oct 19 2020, 8:19 AM · Restricted Project
ArcsinX updated the summary of D89529: [clangd][remote] Add Windows paths support.
Oct 19 2020, 8:09 AM · Restricted Project
ArcsinX updated the diff for D89529: [clangd][remote] Add Windows paths support.

Comments fixes

Oct 19 2020, 6:16 AM · Restricted Project
ArcsinX added inline comments to D89529: [clangd][remote] Add Windows paths support.
Oct 19 2020, 5:27 AM · Restricted Project
ArcsinX updated the diff for D89529: [clangd][remote] Add Windows paths support.

auto => StringRef
add comment for llvm::sys::path::native() call
Result.str().str() => std::string(Result)
add back removed test

Oct 19 2020, 5:20 AM · Restricted Project
ArcsinX added inline comments to D89529: [clangd][remote] Add Windows paths support.
Oct 19 2020, 3:25 AM · Restricted Project
ArcsinX added inline comments to D89529: [clangd][remote] Add Windows paths support.
Oct 19 2020, 2:55 AM · Restricted Project

Oct 18 2020

ArcsinX added a comment to D89529: [clangd][remote] Add Windows paths support.

The solution would be to just convert to the POSIX path instead of asserting it.

I have updated the patch according to this advice.
It seems to me it looks more consistent now, thank you.

Oct 18 2020, 4:05 AM · Restricted Project
ArcsinX updated the diff for D89529: [clangd][remote] Add Windows paths support.

Convert RemoteIndexRoot and LocalIndexRoot to the POSIX style.

Oct 18 2020, 3:55 AM · Restricted Project

Oct 16 2020

ArcsinX added a comment to D89529: [clangd][remote] Add Windows paths support.

Hi, thanks for flagging the issue! Could you please give more information where exactly the tests fail (e.g. logs with assertions)? This way I could understand where exactly the failure happens.

With current implementation I could not pass Windows path (e.g. C:\a\b\c) to Marshaller constructor because of assert(RemoteIndexRoot == llvm::sys::path::convert_to_slash(RemoteIndexRoot));, because on Windows llvm::sys::path::convert_to_slash(RemoteIndexRoot) converts C:\a\b\c to C:/a/b/c

Yes, I mentioned it above. The solution would be to just convert to the POSIX path instead of asserting it.

Ok, got your point. I will think about this.
But for me it's hard to see the reason why we need to store local paths in POSIX style instead of native. From my point of view, we need to ensure that paths inside the wire are in POSIX style and nothing more. E.g. clangd converts paths extracted from LSP to native style.

Oct 16 2020, 5:52 AM · Restricted Project
ArcsinX added a comment to D89529: [clangd][remote] Add Windows paths support.
D:\work\llvm-project\build>ninja check-clangd
[6/7] Running the Clangd regression tests
llvm-lit.py: D:\work\llvm-project\build\bin\..\..\llvm\utils\lit\lit\llvm\config.py:347: note: using clang: d:\work\llvm-project\build\bin\clang.exe
-- Testing: 905 tests, 8 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70
FAIL: Clangd Unit Tests :: ./ClangdTests.exe/RemoteMarshallingTest.IncludeHeaderURIs (665 of 905)
******************** TEST 'Clangd Unit Tests :: ./ClangdTests.exe/RemoteMarshallingTest.IncludeHeaderURIs' FAILED ********************
Note: Google Test filter = RemoteMarshallingTest.IncludeHeaderURIs
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from RemoteMarshallingTest
[ RUN      ] RemoteMarshallingTest.IncludeHeaderURIs
D:\work\llvm-project\clang-tools-extra\clangd\unittests\remote\MarshallingTests.cpp(256): error: Value of: bool(Serialized)
  Actual: false
Expected: true
[  FAILED  ] RemoteMarshallingTest.IncludeHeaderURIs (1 ms)
[----------] 1 test from RemoteMarshallingTest (1 ms total)
Oct 16 2020, 4:57 AM · Restricted Project
ArcsinX added a comment to D89529: [clangd][remote] Add Windows paths support.

Hi, thanks for flagging the issue! Could you please give more information where exactly the tests fail (e.g. logs with assertions)? This way I could understand where exactly the failure happens.

With current implementation I could not pass Windows path (e.g. C:\a\b\c) to Marshaller constructor because of assert(RemoteIndexRoot == llvm::sys::path::convert_to_slash(RemoteIndexRoot));, because on Windows llvm::sys::path::convert_to_slash(RemoteIndexRoot) converts C:\a\b\c to C:/a/b/c

Oct 16 2020, 4:56 AM · Restricted Project
ArcsinX added reviewers for D89529: [clangd][remote] Add Windows paths support: kbobyrev, sammccall, kadircet.
Oct 16 2020, 2:49 AM · Restricted Project
ArcsinX requested review of D89529: [clangd][remote] Add Windows paths support.
Oct 16 2020, 2:48 AM · Restricted Project

Oct 2 2020

ArcsinX added a comment to D88721: [clangd][lit] Update document-link.test to respect custom resource-dir locations.

As far as Windows accepts forward and back slashes

Note we don't rely on windows itself for this support.
All access through llvm::sys::fs APIs on windows ultimately goes through widenPath to convert to UTF-16, and this substitutes slashes.
So LLVM tools do always support / on windows and it's fairly common to rely on this for tests (abstraction is hard in lit tests).

I am not sure that understood you correctly, but widenPath does nothing for me, if I pass mixed-slash path to it.
Code:

std::string From("C:\\a/b\\c");
SmallVector<wchar_t, 128> To;
llvm::sys::windows::widenPath(From, To);
std::wcerr << To.data();

Output:

C:\a/b\c

So, if we imagine that Windows does not support /, then paths with / could not be opened with llvm::sys::fs API.

Oct 2 2020, 5:05 AM · Restricted Project
ArcsinX added a comment to D88721: [clangd][lit] Update document-link.test to respect custom resource-dir locations.

Just a question.
On Windows %resource_dir/.. looks a bit inconsistent after substitution (d:\llvm-project\build\lib\clang\12.0.0\include/..), but this test passes.
As far as Windows accepts forward and back slashes, I expect something like this %/resource_dir/.. to be consistent across all platforms.
Are there any plans to add %/resource_dir substitution as it was done for %T, %s, etc?

Oct 2 2020, 3:34 AM · Restricted Project

Sep 29 2020

ArcsinX closed D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines.

Don't know why this didn't close automatically
Commit: https://reviews.llvm.org/rGd8ba6b4ab3eceb6bbcdf4371d4ffaab9d1a5cebe

Sep 29 2020, 10:24 AM · Restricted Project
ArcsinX updated the diff for D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines.

Fix possible UB at bitwise shift.
WordGain => MaxDistance.
Fix LineMin and LineMax values.
Fix comment.

Sep 29 2020, 3:27 AM · Restricted Project

Sep 28 2020

ArcsinX added inline comments to D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines.
Sep 28 2020, 12:31 AM · Restricted Project
ArcsinX updated the diff for D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines.

Use SourceManager::translateLineCol(), code simplifications.

Sep 28 2020, 12:24 AM · Restricted Project

Sep 24 2020

ArcsinX added inline comments to D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines.
Sep 24 2020, 1:30 AM · Restricted Project
ArcsinX added a comment to D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines.

The biggest problem I see is, this is not changing the fact that we are still traversing the whole file:

  • You do one traversal over the whole file to find FileLines
  • Then possibly two more to find OffsetMin and OffsetMax

Seems you are right, but we do not compare strings during traversal to find FileLines.

Sep 24 2020, 12:44 AM · Restricted Project

Sep 23 2020

ArcsinX added a comment to D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines.

I feel like I'm doing something totally wrong here :)
Could someone give me an advice?

Sep 23 2020, 11:23 PM · Restricted Project
ArcsinX updated the diff for D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines.

std::pow => bitwise shift.
Take care about integers overflow.

Sep 23 2020, 12:57 AM · Restricted Project

Sep 18 2020

ArcsinX updated the diff for D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines.

Fix format

Sep 18 2020, 3:09 AM · Restricted Project
ArcsinX added reviewers for D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines: sammccall, kadircet, adamcz.
Sep 18 2020, 1:56 AM · Restricted Project
ArcsinX requested review of D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines.
Sep 18 2020, 1:55 AM · Restricted Project

Sep 16 2020

ArcsinX committed rGd427df6369f1: [clangd] Don't use zlib when it's unavailable. (authored by ArcsinX).
[clangd] Don't use zlib when it's unavailable.
Sep 16 2020, 1:06 AM
ArcsinX closed D87673: [clangd] Don't use zlib when it's unavailable..
Sep 16 2020, 1:06 AM · Restricted Project
ArcsinX abandoned D87745: [clangd] Config: handle Index block.

Sorry, it's already here https://reviews.llvm.org/D87710

Sep 16 2020, 12:50 AM · Restricted Project
ArcsinX added reviewers for D87745: [clangd] Config: handle Index block: sammccall, kadircet, adamcz.
Sep 16 2020, 12:42 AM · Restricted Project
ArcsinX requested review of D87745: [clangd] Config: handle Index block.
Sep 16 2020, 12:41 AM · Restricted Project

Sep 15 2020

ArcsinX updated the summary of D87673: [clangd] Don't use zlib when it's unavailable..
Sep 15 2020, 11:30 PM · Restricted Project
ArcsinX updated the diff for D87673: [clangd] Don't use zlib when it's unavailable..

Remove {}

Sep 15 2020, 11:28 PM · Restricted Project
ArcsinX updated the diff for D87673: [clangd] Don't use zlib when it's unavailable..

Fix build

Sep 15 2020, 1:24 AM · Restricted Project
ArcsinX added reviewers for D87673: [clangd] Don't use zlib when it's unavailable.: sammccall, kadircet, adamcz.

Unsure about test for this

Sep 15 2020, 1:03 AM · Restricted Project
ArcsinX requested review of D87673: [clangd] Don't use zlib when it's unavailable..
Sep 15 2020, 1:00 AM · Restricted Project

Aug 26 2020

ArcsinX committed rGceffd6993c35: [Support][Windows] Fix incorrect GetFinalPathNameByHandleW() return value check… (authored by ArcsinX).
[Support][Windows] Fix incorrect GetFinalPathNameByHandleW() return value check…
Aug 26 2020, 12:12 PM
ArcsinX closed D86564: [Support][Windows] Fix incorrect GetFinalPathNameByHandleW() return value check in realPathFromHandle().
Aug 26 2020, 12:12 PM · Restricted Project
ArcsinX added a comment to D86564: [Support][Windows] Fix incorrect GetFinalPathNameByHandleW() return value check in realPathFromHandle().

I know it's not part of this patch, but Phabricator is showing a clang-tidy lint error on the inclusion of <io.h>. While Windows does have one of these, it's only a compatibility hack, and it doesn't seem to be needed. Can you yank that #include <io.h> line before submitting?

Aug 26 2020, 9:11 AM · Restricted Project
ArcsinX updated the diff for D86564: [Support][Windows] Fix incorrect GetFinalPathNameByHandleW() return value check in realPathFromHandle().

Remove #include <io.h>

Aug 26 2020, 9:10 AM · Restricted Project
ArcsinX added a comment to D86564: [Support][Windows] Fix incorrect GetFinalPathNameByHandleW() return value check in realPathFromHandle().

Thank you for review.

Aug 26 2020, 7:46 AM · Restricted Project
ArcsinX updated the diff for D86564: [Support][Windows] Fix incorrect GetFinalPathNameByHandleW() return value check in realPathFromHandle().

Buffer.data() => Buffer.begin()

Aug 26 2020, 7:44 AM · Restricted Project
ArcsinX added inline comments to D86564: [Support][Windows] Fix incorrect GetFinalPathNameByHandleW() return value check in realPathFromHandle().
Aug 26 2020, 6:40 AM · Restricted Project
ArcsinX added inline comments to D86564: [Support][Windows] Fix incorrect GetFinalPathNameByHandleW() return value check in realPathFromHandle().
Aug 26 2020, 6:19 AM · Restricted Project
ArcsinX updated the diff for D86564: [Support][Windows] Fix incorrect GetFinalPathNameByHandleW() return value check in realPathFromHandle().

Check GetFinalPathNameByHandleW() return value for non-zero first

Aug 26 2020, 6:18 AM · Restricted Project
ArcsinX updated the diff for D86564: [Support][Windows] Fix incorrect GetFinalPathNameByHandleW() return value check in realPathFromHandle().

Check GetFinalPathNameByHandleW() return value for zero first

Aug 26 2020, 6:09 AM · Restricted Project
ArcsinX added inline comments to D86564: [Support][Windows] Fix incorrect GetFinalPathNameByHandleW() return value check in realPathFromHandle().
Aug 26 2020, 3:07 AM · Restricted Project
ArcsinX updated the diff for D86564: [Support][Windows] Fix incorrect GetFinalPathNameByHandleW() return value check in realPathFromHandle().

Handle Buffer.capacity() == 0 case correcly.

Aug 26 2020, 3:06 AM · Restricted Project

Aug 25 2020

ArcsinX added reviewers for D86564: [Support][Windows] Fix incorrect GetFinalPathNameByHandleW() return value check in realPathFromHandle(): andrewng, rnk, sammccall, bd1976llvm.
Aug 25 2020, 11:59 AM · Restricted Project
ArcsinX requested review of D86564: [Support][Windows] Fix incorrect GetFinalPathNameByHandleW() return value check in realPathFromHandle().
Aug 25 2020, 11:52 AM · Restricted Project

Aug 20 2020

ArcsinX committed rG445739826567: [clangd] Don't crash on `#pragma clang __debug parser_crash` (authored by ArcsinX).
[clangd] Don't crash on `#pragma clang __debug parser_crash`
Aug 20 2020, 5:54 AM
ArcsinX closed D86279: [clangd] Don't crash on `#pragma clang __debug parser_crash`.
Aug 20 2020, 5:54 AM · Restricted Project
ArcsinX added reviewers for D86279: [clangd] Don't crash on `#pragma clang __debug parser_crash`: sammccall, kadircet, hokein.
Aug 20 2020, 3:51 AM · Restricted Project
ArcsinX requested review of D86279: [clangd] Don't crash on `#pragma clang __debug parser_crash`.
Aug 20 2020, 3:50 AM · Restricted Project

Aug 17 2020

ArcsinX committed rGbc5c9df62182: [clangd] Fix Windows build when remote index is enabled. (authored by ArcsinX).
[clangd] Fix Windows build when remote index is enabled.
Aug 17 2020, 6:55 AM
ArcsinX closed D86052: [clangd] Fix Windows build when remote index is enabled..
Aug 17 2020, 6:55 AM · Restricted Project
ArcsinX added a comment to D86052: [clangd] Fix Windows build when remote index is enabled..

Looks good to me, thank you very much!

I'm curious why this works on Linux though, I guess Threads are not aliased as Threads::Threads somehow?

I assume you are able to build this on Windows with this patch, right?

Aug 17 2020, 6:39 AM · Restricted Project
ArcsinX added reviewers for D86052: [clangd] Fix Windows build when remote index is enabled.: kbobyrev, sammccall.

I am not sure that this fix is correct, I took this solution from here https://github.com/grpc/grpc/blob/master/examples/cpp/helloworld/CMakeLists.txt#L30

Aug 17 2020, 2:59 AM · Restricted Project
ArcsinX requested review of D86052: [clangd] Fix Windows build when remote index is enabled..
Aug 17 2020, 2:57 AM · Restricted Project

Aug 14 2020

ArcsinX committed rG399e4593431c: [clangd] Fix find_program() result check when searching for gRPC (authored by ArcsinX).
[clangd] Fix find_program() result check when searching for gRPC
Aug 14 2020, 1:34 AM
ArcsinX closed D85958: [clangd] Fix find_program() result check when searching for gRPC.
Aug 14 2020, 1:34 AM · Restricted Project
ArcsinX updated the diff for D85958: [clangd] Fix find_program() result check when searching for gRPC.

Rebase

Aug 14 2020, 1:31 AM · Restricted Project
ArcsinX added a comment to D85958: [clangd] Fix find_program() result check when searching for gRPC.

Do you want me to land the patch for you?

I already have commit access, thank you.

Aug 14 2020, 1:23 AM · Restricted Project
ArcsinX added reviewers for D85958: [clangd] Fix find_program() result check when searching for gRPC: kbobyrev, sammccall.
Aug 14 2020, 12:31 AM · Restricted Project
ArcsinX requested review of D85958: [clangd] Fix find_program() result check when searching for gRPC.
Aug 14 2020, 12:31 AM · Restricted Project