Page MenuHomePhabricator

jkorous (Jan Korous)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 18 2018, 2:22 AM (56 w, 4 d)

Recent Activity

Fri, May 17

jkorous updated the diff for D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher.

fix link libraries in cmake

Fri, May 17, 5:54 PM · Restricted Project

Wed, May 15

jkorous updated the diff for D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher.

A major clean-up.

Wed, May 15, 7:02 PM · Restricted Project

Mon, May 13

jkorous committed rG1652d8140191: [clang][ASTContext] Call setAttached for comments attached to a declaration (authored by jkorous).
[clang][ASTContext] Call setAttached for comments attached to a declaration
Mon, May 13, 10:53 AM
jkorous committed rC360607: [clang][ASTContext] Call setAttached for comments attached to a declaration.
[clang][ASTContext] Call setAttached for comments attached to a declaration
Mon, May 13, 10:52 AM
jkorous committed rL360607: [clang][ASTContext] Call setAttached for comments attached to a declaration.
[clang][ASTContext] Call setAttached for comments attached to a declaration
Mon, May 13, 10:52 AM
jkorous closed D61538: [clang][ASTContext] Call setAttached for comments attached to a declaration.
Mon, May 13, 10:52 AM · Restricted Project

Mon, May 6

jkorous added a comment to D28462: clang-format: Add new style option AlignConsecutiveMacros.

Hi @VelocityRa, just FYI - it's considered fine to ping your reviewers once per week here if you've addressed their comments and there's no activity in the review. Sometimes people just get distracted by other things.

Mon, May 6, 3:48 PM · Restricted Project

Fri, May 3

jkorous created D61538: [clang][ASTContext] Call setAttached for comments attached to a declaration.
Fri, May 3, 2:25 PM · Restricted Project

Thu, May 2

jkorous committed rGf90458b3895c: [clangd][xpc] Cannonicalize value of CLANGD_BUILD_XPC before caching (authored by jkorous).
[clangd][xpc] Cannonicalize value of CLANGD_BUILD_XPC before caching
Thu, May 2, 1:31 PM
jkorous committed rCTE359824: [clangd][xpc] Cannonicalize value of CLANGD_BUILD_XPC before caching.
[clangd][xpc] Cannonicalize value of CLANGD_BUILD_XPC before caching
Thu, May 2, 1:30 PM
jkorous committed rL359824: [clangd][xpc] Cannonicalize value of CLANGD_BUILD_XPC before caching.
[clangd][xpc] Cannonicalize value of CLANGD_BUILD_XPC before caching
Thu, May 2, 1:30 PM

Wed, May 1

jkorous planned changes to D61104: [clang][ASTContext] Try to avoid sorting comments for code completion.

@gribozavr thanks for the feedback. I'm rewriting the patch now as I figured out my detection of comments preceding declarations is unsound.

Wed, May 1, 2:53 PM · Restricted Project

Mon, Apr 29

jkorous committed rGfd76383d761c: [clangd][xpc] Fix XPC unittests (authored by jkorous).
[clangd][xpc] Fix XPC unittests
Mon, Apr 29, 12:40 PM
jkorous committed rL359489: [clangd][xpc] Fix XPC unittests.
[clangd][xpc] Fix XPC unittests
Mon, Apr 29, 12:39 PM
jkorous committed rCTE359489: [clangd][xpc] Fix XPC unittests.
[clangd][xpc] Fix XPC unittests
Mon, Apr 29, 12:39 PM
jkorous closed D61271: [clangd][xpc] Fix XPC unittests.
Mon, Apr 29, 12:39 PM · Restricted Project
jkorous added a comment to D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools..

Patch with fix for XPC tests https://reviews.llvm.org/D61271

Mon, Apr 29, 11:19 AM · Restricted Project, Restricted Project
jkorous created D61271: [clangd][xpc] Fix XPC unittests.
Mon, Apr 29, 11:19 AM · Restricted Project
jkorous added a comment to D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools..

@MaskRay @juliehockett I'll take a look.

Mon, Apr 29, 10:23 AM · Restricted Project, Restricted Project

Fri, Apr 26

jkorous updated the diff for D61102: [clang][ASTContext][NFCi] Refactor ASTContext::getRawCommentForDeclNoCache.
  • moved assert
Fri, Apr 26, 10:29 AM · Restricted Project

Thu, Apr 25

jkorous added inline comments to D61102: [clang][ASTContext][NFCi] Refactor ASTContext::getRawCommentForDeclNoCache.
Thu, Apr 25, 5:30 PM · Restricted Project
jkorous updated the diff for D61102: [clang][ASTContext][NFCi] Refactor ASTContext::getRawCommentForDeclNoCache.
  • renames
  • if => assert
Thu, Apr 25, 5:09 PM · Restricted Project
jkorous abandoned D60432: [clang][ASTContext] Simplify caching for declaration-related comments.
Thu, Apr 25, 4:05 PM · Restricted Project
jkorous added a comment to D61103: [clang] Add tryToAttachCommentsToDecls method to ASTContext.

Also, IIUC the test case that I deleted wasn't actually supposed to produce any diagnostics and the fact that it did was a bug. We could keep it as a regression test but I think it has a rather low value. WDYT?

Thu, Apr 25, 3:52 PM · Restricted Project
jkorous updated the diff for D61103: [clang] Add tryToAttachCommentsToDecls method to ASTContext.
  • clang-format
  • comments
Thu, Apr 25, 3:50 PM · Restricted Project
jkorous added inline comments to D61103: [clang] Add tryToAttachCommentsToDecls method to ASTContext.
Thu, Apr 25, 3:46 PM · Restricted Project
jkorous abandoned D60494: [clang][ASTContext] Don't load external comments from Sema::ActOnDocumentableDecls.

Abandonned in favor of https://reviews.llvm.org/D61103

Thu, Apr 25, 1:24 PM

Wed, Apr 24

jkorous added a parent revision for D61104: [clang][ASTContext] Try to avoid sorting comments for code completion: D61102: [clang][ASTContext][NFCi] Refactor ASTContext::getRawCommentForDeclNoCache.
Wed, Apr 24, 4:43 PM · Restricted Project
jkorous added a child revision for D61102: [clang][ASTContext][NFCi] Refactor ASTContext::getRawCommentForDeclNoCache: D61104: [clang][ASTContext] Try to avoid sorting comments for code completion.
Wed, Apr 24, 4:43 PM · Restricted Project
jkorous added a child revision for D61102: [clang][ASTContext][NFCi] Refactor ASTContext::getRawCommentForDeclNoCache: D61103: [clang] Add tryToAttachCommentsToDecls method to ASTContext.
Wed, Apr 24, 4:42 PM · Restricted Project
jkorous added a parent revision for D61103: [clang] Add tryToAttachCommentsToDecls method to ASTContext: D61102: [clang][ASTContext][NFCi] Refactor ASTContext::getRawCommentForDeclNoCache.
Wed, Apr 24, 4:42 PM · Restricted Project
jkorous updated the summary of D61102: [clang][ASTContext][NFCi] Refactor ASTContext::getRawCommentForDeclNoCache.
Wed, Apr 24, 4:42 PM · Restricted Project
jkorous created D61104: [clang][ASTContext] Try to avoid sorting comments for code completion.
Wed, Apr 24, 4:42 PM · Restricted Project
jkorous created D61103: [clang] Add tryToAttachCommentsToDecls method to ASTContext.
Wed, Apr 24, 4:33 PM · Restricted Project
jkorous created D61102: [clang][ASTContext][NFCi] Refactor ASTContext::getRawCommentForDeclNoCache.
Wed, Apr 24, 4:32 PM · Restricted Project
jkorous accepted D61093: Fix spelling error. NFC.
Wed, Apr 24, 3:25 PM · Restricted Project
jkorous accepted D61096: posix_spawn should retry upon EINTR.

Hmm, we have this utility function RetryAfterSignal() but it doesn't have maxRetries.

Wed, Apr 24, 3:24 PM · Restricted Project

Tue, Apr 23

jkorous added a comment to D60523: [clang] Don't segfault on incorrect using directive (PR41400).

I can't really comment on correctness of your fix but had been willing to do the work I'd suggest making ASTContext::getDependentNameType and DependentNameType::DependentNameType interface more robust.

Tue, Apr 23, 6:43 PM · Restricted Project
jkorous accepted D61038: Add "const" in GetUnderlyingObjects.

Seems like a NFC.
Looks awesome to me - const correctness for the win!

Tue, Apr 23, 5:50 PM · Restricted Project
jkorous abandoned D48559: [clangd] refactoring for XPC transport layer [NFCI].
Tue, Apr 23, 5:32 PM · Restricted Project, Restricted Project
jkorous abandoned D48560: [clangd] JSON <-> XPC conversions.
Tue, Apr 23, 5:32 PM · Restricted Project, Restricted Project

Apr 16 2019

jkorous added inline comments to D60233: [clang-scan-deps] initial outline of the tool that runs preprocessor to find dependencies over a JSON compilation database.
Apr 16 2019, 3:51 PM · Restricted Project
jkorous added inline comments to D60779: [ADT] llvm::bsearch, binary search for mere mortals.
Apr 16 2019, 2:05 PM · Restricted Project

Apr 12 2019

jkorous abandoned D58749: [index-while-building] IndexRecordHasher.
Apr 12 2019, 2:42 PM · Restricted Project

Apr 11 2019

jkorous added a comment to D58749: [index-while-building] IndexRecordHasher.

We did performance tests of alternative approach - just hashing the serialized bit code representation. There's a performance regression in the sense that while the current implementation costs approx. extra 2.2% in build time the alternative approach costs 3.8%.

Apr 11 2019, 8:08 PM · Restricted Project

Apr 10 2019

jkorous added a comment to D60494: [clang][ASTContext] Don't load external comments from Sema::ActOnDocumentableDecls.

I agree and will try to decrease the impact on interfaces.

Apr 10 2019, 4:17 PM
jkorous updated the diff for D60432: [clang][ASTContext] Simplify caching for declaration-related comments.

rename RawCommentAndCacheFlags -> CommentAndOrigin

Apr 10 2019, 4:12 PM · Restricted Project
jkorous updated the diff for D60432: [clang][ASTContext] Simplify caching for declaration-related comments.

Second attempt on reducing the cache size and number of operations.

Apr 10 2019, 4:03 PM · Restricted Project
jkorous added inline comments to D60432: [clang][ASTContext] Simplify caching for declaration-related comments.
Apr 10 2019, 3:56 PM · Restricted Project
jkorous committed rG6644d014dd98: [clang][ASTContext] Try to exit early before loading serialized comments from… (authored by jkorous).
[clang][ASTContext] Try to exit early before loading serialized comments from…
Apr 10 2019, 1:23 PM
jkorous committed rC358133: [clang][ASTContext] Try to exit early before loading serialized comments from….
[clang][ASTContext] Try to exit early before loading serialized comments from…
Apr 10 2019, 1:23 PM
jkorous committed rL358133: [clang][ASTContext] Try to exit early before loading serialized comments from….
[clang][ASTContext] Try to exit early before loading serialized comments from…
Apr 10 2019, 1:23 PM
jkorous closed D60493: [clang][ASTContext] Try to exit early before loading serialize comments from AST files.
Apr 10 2019, 1:23 PM · Restricted Project
jkorous added a comment to D60493: [clang][ASTContext] Try to exit early before loading serialize comments from AST files.

Thanks @gribozavr!

Apr 10 2019, 1:23 PM · Restricted Project

Apr 9 2019

jkorous created D60494: [clang][ASTContext] Don't load external comments from Sema::ActOnDocumentableDecls.
Apr 9 2019, 6:45 PM
jkorous created D60493: [clang][ASTContext] Try to exit early before loading serialize comments from AST files.
Apr 9 2019, 6:34 PM · Restricted Project
jkorous added inline comments to D60432: [clang][ASTContext] Simplify caching for declaration-related comments.
Apr 9 2019, 6:29 PM · Restricted Project

Apr 8 2019

jkorous created D60432: [clang][ASTContext] Simplify caching for declaration-related comments.
Apr 8 2019, 5:45 PM · Restricted Project

Apr 4 2019

jkorous accepted D60120: check-clang-tools: Actually build and run XPC test.

LGTM. Thanks for fixing this!

Apr 4 2019, 1:03 PM · Restricted Project

Mar 25 2019

jkorous committed rG2d000e395ecc: [clangd][xpc][cmake] Respect explicit value of CLANGD_BUILD_XPC (authored by jkorous).
[clangd][xpc][cmake] Respect explicit value of CLANGD_BUILD_XPC
Mar 25 2019, 8:48 PM
jkorous committed rCTE356974: [clangd][xpc][cmake] Respect explicit value of CLANGD_BUILD_XPC.
[clangd][xpc][cmake] Respect explicit value of CLANGD_BUILD_XPC
Mar 25 2019, 8:48 PM
jkorous committed rL356974: [clangd][xpc][cmake] Respect explicit value of CLANGD_BUILD_XPC.
[clangd][xpc][cmake] Respect explicit value of CLANGD_BUILD_XPC
Mar 25 2019, 8:48 PM
jkorous closed D59808: [clangd][xpc][cmake] Respect explicit value of CLANGD_BUILD_XPC.
Mar 25 2019, 8:48 PM · Restricted Project
jkorous created D59808: [clangd][xpc][cmake] Respect explicit value of CLANGD_BUILD_XPC.
Mar 25 2019, 8:09 PM · Restricted Project

Mar 21 2019

jkorous accepted D59377: Frontend: Remove CompilerInstance::VirtualFileSystem, NFC.

LGTM

Mar 21 2019, 8:23 PM
jkorous added a comment to D40988: Clang-format: add finer-grained options for putting all arguments on one line.

@russellmcc the patch has been approved by @MyDeveloperDay
https://reviews.llvm.org/D40988#1430502

Mar 21 2019, 4:21 PM · Restricted Project
jkorous accepted D57965: Clean up ObjCPropertyDecl printing.

LGTM. Thanks for working on this!

Mar 21 2019, 4:13 PM · Restricted Project, Restricted Project
jkorous added a comment to D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher.

Why is this needed for index-while-building? My mental model for index-while-building is that that streams out build index metadata as part of the regular compile. Why does that require watching directories?

You're right that this isn't necessary for the indexing phase. But we also provide API so clients can consume the index. This functionality is used for getting notifications about index data changes.

You can see it for example here:
https://github.com/apple/swift-clang/blob/stable/lib/IndexDataStore/IndexDataStore.cpp#L111

Is that code going to live in clang? This seems more like a tool built on top of the compiler rather than something core to the compiler itself (like the actual index-while-building feature). Maybe this could be in clang-tools-extra?

It actually is part of the feature as the serialized format of the index isn't meant as a stable interface, that's what the API is for. DirectoryWatcher isn't a tool, it's just part of implementation of the IndexStore API.

Maybe I misunderstand what the client of the IndexStore API is. That's not code that will be in the clang binary, right?

Mar 21 2019, 4:09 PM · Restricted Project

Mar 20 2019

jkorous added inline comments to D59605: [clangd] Introduce background-indexer.
Mar 20 2019, 11:05 AM · Restricted Project

Mar 19 2019

jkorous accepted D59388: Basic: Return a reference from FileManager::getVirtualFileSystem, NFC.

Yes, it's safe. The reference count is "intrusive", meaning it's stored in the object itself (via inheritance from RefCountedBase). As a result, all the instances of IntrusiveRefCntPtr that reference at the same object will implicitly share their count.

Mar 19 2019, 11:28 AM

Mar 18 2019

jkorous accepted D59130: [llvm][Support] Provide interface to set thread priorities.

Gotchas and symptoms are IMO dependent on the use-case. Since this is a generic API I'd say that documenting the behavior is the best we can do here.

Mar 18 2019, 11:31 AM · Restricted Project

Mar 15 2019

jkorous added a comment to D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher.

Why is this needed for index-while-building? My mental model for index-while-building is that that streams out build index metadata as part of the regular compile. Why does that require watching directories?

You're right that this isn't necessary for the indexing phase. But we also provide API so clients can consume the index. This functionality is used for getting notifications about index data changes.

You can see it for example here:
https://github.com/apple/swift-clang/blob/stable/lib/IndexDataStore/IndexDataStore.cpp#L111

Is that code going to live in clang? This seems more like a tool built on top of the compiler rather than something core to the compiler itself (like the actual index-while-building feature). Maybe this could be in clang-tools-extra?

Mar 15 2019, 1:29 PM · Restricted Project
jkorous added a comment to D59388: Basic: Return a reference from FileManager::getVirtualFileSystem, NFC.

Hi Duncan, thanks for working on better interfaces in clang!

Mar 15 2019, 12:46 PM
jkorous added a comment to D59377: Frontend: Remove CompilerInstance::VirtualFileSystem, NFC.

... since I noticed that FileManager ...

Mar 15 2019, 11:56 AM
jkorous added a comment to D59300: [clangd] Tune the fuzzy-matching algorithm.

@ilya-biryukov could you please give details about the quality metric you are using and some description of posted measurements?

Mar 15 2019, 10:53 AM · Restricted Project, Restricted Project
jkorous added a comment to D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher.

Why is this needed for index-while-building? My mental model for index-while-building is that that streams out build index metadata as part of the regular compile. Why does that require watching directories?

Mar 15 2019, 12:24 AM · Restricted Project

Mar 14 2019

jkorous updated the diff for D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher.

Remove duplicate comment

Mar 14 2019, 3:36 PM · Restricted Project

Mar 12 2019

jkorous added a comment to D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher.

Ping.

Mar 12 2019, 2:34 PM · Restricted Project
jkorous added a comment to D58749: [index-while-building] IndexRecordHasher.
Mar 12 2019, 2:18 PM · Restricted Project
jkorous updated the diff for D58749: [index-while-building] IndexRecordHasher.

Addressed some of Dmitri's comments.

Mar 12 2019, 2:16 PM · Restricted Project
jkorous added a comment to D58749: [index-while-building] IndexRecordHasher.

Addressed some comments, going to update the diff.

Mar 12 2019, 2:16 PM · Restricted Project
jkorous added a comment to D58749: [index-while-building] IndexRecordHasher.

I left some comments, but it is difficult for me to review without understanding what the requirements for this hasher are, why some information is hashed in, and some is left out, could you clarify? See detailed comments.

Mar 12 2019, 1:31 PM · Restricted Project
jkorous updated the diff for D58749: [index-while-building] IndexRecordHasher.

Based on Kadir comment I refactored the code.

Mar 12 2019, 1:24 PM · Restricted Project
jkorous added a comment to D59130: [llvm][Support] Provide interface to set thread priorities.

We are already using PRIO_DARWIN_BG in couple other places - libclang and CrashRecoveryContext in llvm. According to the linked man page this priority causes I/O to be "throttled" but not halted on macOS.

Mar 12 2019, 11:11 AM · Restricted Project

Mar 6 2019

jkorous added a comment to D41407: Add index-while-building support to Clang - Part 3.

I'll address comments for this patch in the new set of patches and I'll add original reviewers.

Mar 6 2019, 10:59 AM
jkorous added a comment to D39050: Add index-while-building support to Clang.

I'll address comments for this patch in the new set of patches.

Mar 6 2019, 10:59 AM
jkorous added a comment to D40992: Add index-while-building support to Clang - Part 2.

I'll address comments for this patch in the new set of patches and I'll add original reviewers.

Mar 6 2019, 10:59 AM
jkorous abandoned D41407: Add index-while-building support to Clang - Part 3.

It's time to officially abandon these patches in favor of new push for upstreaming index-while-building.

Mar 6 2019, 10:46 AM
jkorous abandoned D40992: Add index-while-building support to Clang - Part 2.

It's time to officially abandon these patches in favor of new push for upstreaming index-while-building.

Mar 6 2019, 10:46 AM
jkorous commandeered D41407: Add index-while-building support to Clang - Part 3.
Mar 6 2019, 10:45 AM
jkorous commandeered D40992: Add index-while-building support to Clang - Part 2.
Mar 6 2019, 10:45 AM
jkorous commandeered D39050: Add index-while-building support to Clang.
Mar 6 2019, 10:45 AM
jkorous abandoned D39050: Add index-while-building support to Clang.

It's time to officially abandon these patches in favor of new push for upstreaming index-while-building.

Mar 6 2019, 10:45 AM

Mar 5 2019

jkorous committed rG88e15140ee54: [clang-format] Fix lambdas returning template specialization that contains… (authored by jkorous).
[clang-format] Fix lambdas returning template specialization that contains…
Mar 5 2019, 11:27 AM
jkorous committed rC355434: [clang-format] Fix lambdas returning template specialization that contains….
[clang-format] Fix lambdas returning template specialization that contains…
Mar 5 2019, 11:26 AM
jkorous committed rL355434: [clang-format] Fix lambdas returning template specialization that contains….
[clang-format] Fix lambdas returning template specialization that contains…
Mar 5 2019, 11:26 AM
jkorous closed D58934: [clang-format] Fix lambdas returning template specialization that contains operator in parameter.
Mar 5 2019, 11:26 AM · Restricted Project, Restricted Project
jkorous added a comment to D58934: [clang-format] Fix lambdas returning template specialization that contains operator in parameter.

I'm not sure I personally would ever write code like that ;-) , but if its legal C++ and it compiles we should handle it the same as foo<1>,foo<true>,foo<!true>

I hear you :D

As there are a number of reviews out there for formatting Lambdas I think its a good idea for us to add corner cases like this to the unit tests, but it does get me thinking if this shouldn't be handled by a piece of code which knows about trailing return types (template or otherwise) and not be in the general Lambda parsing code

I suspect that given that the switch statement handles

tok::identifier, tok::less, tok::numeric_constant, tok::greater
foo            <                          1              >

We are effectively just consuming the return type tokens.

But to handle what you have here it LGTM and handles more use cases that https://bugs.llvm.org/show_bug.cgi?id=40910 would throw up.

Thanks for helping out

Mar 5 2019, 11:20 AM · Restricted Project, Restricted Project
jkorous accepted D58922: [clang-format] broken after lambda with return type template with boolean literal.

do you mean this case? as this seems to work for me?

verifyFormat("namespace bar {\n"
               "auto foo{[]() -> foo<false> { ; }};\n"
               "} // namespace bar");
Mar 5 2019, 11:20 AM · Restricted Project, Restricted Project

Mar 4 2019

jkorous added reviewers for D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher: kadircet, gribozavr.

Adding clangd folks in case they want to take a look.

Mar 4 2019, 5:52 PM · Restricted Project