[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous updated this revision to Diff 187623. jkorous added a comment. [DirectoryWatcher] Fix detection of FSEvents for iOS [DirectoryWatcher][NFC] Doxygen CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 Files: clang/include/clang/DirectoryWat

[PATCH] D58478: [index-while-building] FileIndexRecord

jkorous created this revision. jkorous added reviewers: dexonsmith, nathawes, akyrtzi, arphaman, ioeric, malaperle. Herald added subscribers: cfe-commits, jdoerfert, mgorny. Herald added a project: clang. Basic data structures for index Tests are missing from this patch - will be covered properl

[PATCH] D58478: [index-while-building] FileIndexRecord

jkorous updated this revision to Diff 187848. jkorous marked 5 inline comments as done. jkorous added a comment. - license - remove auto CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58478/new/ https://reviews.llvm.org/D58478 Files: clang/include/clang/Index/DeclOccurrence.h clang/

[PATCH] D58478: [index-while-building] FileIndexRecord

jkorous added a comment. Thanks Eugene. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58478/new/ https://reviews.llvm.org/D58478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous updated this revision to Diff 187850. jkorous added a comment. - license - end-of-include-guard comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 Files: clang/include/clang/DirectoryWatcher/DirectoryWatcher.h clang/lib/CMakeLi

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous updated this revision to Diff 187860. jkorous marked 2 inline comments as done. jkorous added a comment. - handle EINTR - assert we are reading whole INotifyEvents CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 Files: clang/include/cl

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous added a comment. Thanks for taking a look Michał! I'll try to address the rest of your comments asap. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 ___ cfe-commits mailing list cfe-commits

[PATCH] D58478: [index-while-building] FileIndexRecord

jkorous updated this revision to Diff 187863. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58478/new/ https://reviews.llvm.org/D58478 Files: clang/include/clang/Index/DeclOccurrence.h clang/lib/Index/CMakeLists.txt clang/lib/Index/FileIndexRecord.cpp clang/lib/Index/FileIndexRec

[PATCH] D58478: [index-while-building] FileIndexRecord

jkorous marked 3 inline comments as done. jkorous added a comment. Done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58478/new/ https://reviews.llvm.org/D58478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous updated this revision to Diff 187868. jkorous added a comment. - handle errors returned by close CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 Files: clang/include/clang/DirectoryWatcher/DirectoryWatcher.h clang/lib/CMakeLists.txt

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous marked an inline comment as done. jkorous added a comment. Fixed closing of FD. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:/

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous marked 4 inline comments as done. jkorous added inline comments. Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher-linux.inc.h:116 + + DirectoryWatcher::EventKind K = DirectoryWatcher::EventKind::Added; + if (ievt->mask & IN_MODIFY) { mgor

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous marked 20 inline comments as done. jkorous added inline comments. Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:40 +// FDRead. +// Currently used just for one-off termination signal. +struct SemaphorPipe { gribozavr wrote: > grib

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous updated this revision to Diff 202467. jkorous marked 2 inline comments as done. jkorous added a comment. Addressed comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 Files: clang/include/clang/DirectoryWatcher/DirectoryWatcher.h

[PATCH] D61729: [docs] Fix example for Allman brace breaking style

jkorous edited reviewers, added: owenpan; removed: llvm-commits. jkorous added a subscriber: owenpan. jkorous added a comment. You're right, thanks for letting me know. It was changed in 806d5741aa7f . @owenpan was that intend

[PATCH] D61729: [docs] Fix example for Allman brace breaking style

jkorous updated this revision to Diff 202827. jkorous added a comment. Fix in the header file from which the documentation is actually generated. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61729/new/ https://reviews.llvm.org/D61729 Files: clang/docs/ClangFormatStyleOptions.rst c

[PATCH] D61729: [docs] Fix example for Allman brace breaking style

jkorous added a comment. Thanks, I didn't know it's generated. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61729/new/ https://reviews.llvm.org/D61729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/m

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous marked 3 inline comments as done. jkorous added a comment. I fixed the rest. There are still some questions you raised that I just responded to and kept them as not Done. Feel free to take a look. If nothing comes up I'll commit this on Wednesday. Comment at: clang/l

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous updated this revision to Diff 202833. jkorous added a comment. linux implementation - factory method for SemaphorePipe - *_CLOEXEC flags CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 Files: clang/include/clang/DirectoryWatcher/Direc

[PATCH] D61729: [docs] Fix example for Allman brace breaking style

This revision was automatically updated to reflect the committed changes. Closed by commit rL362646: [clang-format][NFC] Fix BS_Allman style example in the header docs are… (authored by jkorous, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior t

[PATCH] D63613: [clang-tidy] Fail gracefully upon empty database fields

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. Herald added a subscriber: dexonsmith. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63613/new/ https://reviews.llvm.org/D63613 __

[PATCH] D63961: [clangd][xpc] pass the LSP value using data instead of string

jkorous added a comment. I'd add a test with non-empty non-LSP dictionary to specifically test that we're ignoring the content. I like const-correctness but that's up to you. Otherwise LGTM. Comment at: clang-tools-extra/clangd/unittests/xpc/ConversionTests.cpp:34 +TEST(JsonX

[PATCH] D61104: [clang][ASTContext] Try to avoid sorting comments for code completion

jkorous abandoned this revision. jkorous added a comment. Abandoned in favor of https://reviews.llvm.org/D65301 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61104/new/ https://reviews.llvm.org/D61104 ___ cfe-commits

[PATCH] D61103: [clang] Add tryToAttachCommentsToDecls method to ASTContext

jkorous abandoned this revision. jkorous added a comment. Abandoned in favor of https://reviews.llvm.org/D65301 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61103/new/ https://reviews.llvm.org/D61103 ___ cfe-commits mailing list cfe-commit

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

jkorous added a comment. Hi @jdenny, thanks for the warning! I think having the test disabled is fine - the main upside I see is that we won't check it fails over and over on our CI systems. Could you please mention the test/reproducer in those FIXMEs? Otherwise LGTM. Comment

[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

jkorous added a comment. Hi @hokein, Do you have any thoughts on how to handle situation when client registers callback but doesn't send a request with registered ID later? Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:317 +std::lock_guard Lock(ReplyCallbacksMut

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

jkorous added a comment. In D61466#1602928 , @jdenny wrote: > In D61466#1602917 , @jkorous wrote: > > > > > > In an inline comment, you also mentioned the alternative of replacing > `EXPECT_EQ` with `EXPECT_NE`. N

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

jkorous added inline comments. Comment at: clang/include/clang/Basic/FileManager.h:143 /// - llvm::StringMap SeenDirEntries; + llvm::StringMap, llvm::BumpPtrAllocator> + SeenDirEntries; Maybe we could replace this with some type that has hard-to-use-incorre

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous added a comment. In D58418#1609138 , @plotfi wrote: > @jkorous DirectoryWatcherTests causes ninja check-clang to hang on my Ubuntu > 18.04 computer. check-clang will not finish and I am forced to killall -9 > DirectoryWatcherTests. My system has

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

jkorous added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:240 +auto File = SourceMgr.getFileManager().getFile(FilePath); +if (!File) + return SourceLocation(); Previously we'd hit the assert in `translateFile()` called fro

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous added a comment. Hi Puyan, I failed to reproduce with llvm.org/master (5faa533e47b0e54b04166b0257c5ebb48e6ffcaa ) on Ubuntu 18.04.1 LTS both in debug and release build. Since it sounds like you can reproduce "reliabl

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

jkorous accepted this revision. jkorous marked an inline comment as done. jkorous added a comment. This revision is now accepted and ready to land. LGTM Thanks for all the work here! Comment at: clang/lib/ARCMigrate/FileRemapper.cpp:156 + auto newE = FileMgr->getFile(temp

[PATCH] D65545: Handle some fs::remove failures

jkorous added inline comments. Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:107-111 TemporaryFiles::~TemporaryFiles() { llvm::MutexGuard Guard(Mutex); for (const auto &File : Files) -llvm::sys::fs::remove(File.getKey()); +if (std::error_code EC = llvm::sy

[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

jkorous added a comment. Thanks for the patch! Comment at: clang/include/clang/DirectoryWatcher/DirectoryWatcher.h:102 /// Returns nullptr if \param Path doesn't exist or isn't a directory. /// Returns nullptr if OS kernel API told us we can't start watching. In such --

[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

jkorous added a comment. Please just update the comment, otherwise LGTM. Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:283 /*waitForInitialSync=*/true); + if (!DW) return; jkorous wrote: > IIUC this is silently dropping errors.

[PATCH] D65708: [NFC][DirectoryWatchedTests] Unlocking mutexes before signaling condition variable.

jkorous added a comment. LGTM. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65708/new/ https://reviews.llvm.org/D65708 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

jkorous added inline comments. Comment at: cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h:102 - /// Returns nullptr if \param Path doesn't exist or isn't a directory. - /// Returns nullptr if OS kernel API told us we can't start watching. In such - /// case it's

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

jkorous added a comment. I was about to suggest something similar. SGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65829/new/ https://reviews.llvm.org/D65829 ___ cfe-commits mailing list cfe-commit

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

jkorous added a comment. Thanks Puyan! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65829/new/ https://reviews.llvm.org/D65829 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma

[PATCH] D66156: Removed dead code from clang/tools/libclang/CXIndexDataConsumer.{cpp,h}

jkorous added a reviewer: akyrtzi. jkorous added a subscriber: akyrtzi. jkorous added a comment. LGTM but let's wait for @akyrtzi to confirm. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66156/new/ https://reviews.llvm.org/D66156 __

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61466/new/ https://reviews.llvm.org/D61466 ___ cfe-commits mailing list cf

[PATCH] D63603: [ASTImporter] Propagate error from ImportDeclContext

jkorous added a comment. I don't have the insight to LGTM the whole change - just a nit about implementation detail. Comment at: clang/lib/AST/ASTImporter.cpp:1724 + }; + DefinitionCompleter CompleterRAII(To); You might consider using just a lambda with `l

[PATCH] D63600: [test][Driver] Fix Clang :: Driver/cl-response-file.c

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. Herald added a subscriber: dexonsmith. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63600/new/ https://reviews.llvm.org/D63600 _

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

This revision was automatically updated to reflect the committed changes. Closed by commit rL365574: [clang] DirectoryWatcher (authored by jkorous, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D58418?vs=

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous added a comment. Thanks for the revert. There's actually one more problem - seems like ninja doesn't like the generated build.ninja file on Linux. ninja: error: build.ninja:52390: bad $-escape (literal $ must be written as $$) http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu/

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous updated this revision to Diff 199712. jkorous added a comment. A major clean-up. changelog = **general** - specification, documentation, tests - returning only filenames instead of paths (or empty string when the event is related to the watched dir itself) - removed unsound eve

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous updated this revision to Diff 200123. jkorous added a comment. fix link libraries in cmake CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 Files: clang/cmake/modules/AddClang.cmake clang/include/clang/DirectoryWatcher/DirectoryWatche

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous marked 2 inline comments as done. jkorous added a comment. Thanks for taking a look @gribozavr! I briefly scanned the rest of your comments and I agree with you (or don't really have a strong opinion) in all cases. I'll work on that today. Comment at: clang/lib/Direct

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous marked 36 inline comments as done. jkorous added a comment. I addressed most of the comments. What is left: - rewrite test with gmock matchers - write test for metadata of a file in watched dir being changed Comment at: clang/include/clang/DirectoryWatcher/DirectoryWa

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous updated this revision to Diff 200389. jkorous marked 4 inline comments as done. jkorous added a comment. Addressed comments. Changed semantics of one of std::atomic in linux implementation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous marked 9 inline comments as done. jkorous added a comment. Thanks for taking a look Kadir! After yesterday's discussion with Dmitri I removed all those busy waits. Seems like the code is not much more complex now. I am going to update the diff and off to fixing the tests.

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous updated this revision to Diff 200810. jkorous marked an inline comment as done. jkorous added a comment. Remove busy waits. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 Files: clang/cmake/modules/AddClang.cmake clang/include/clang

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous updated this revision to Diff 200811. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 Files: clang/cmake/modules/AddClang.cmake clang/include/clang/DirectoryWatcher/DirectoryWatcher.h clang/lib/CMakeLists.txt clang/lib/DirectoryWat

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous updated this revision to Diff 201114. jkorous added a comment. Reimplemented tests with std::futures which allowed to use more generous timeout while not slowing down the happy paths. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 File

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous marked 6 inline comments as done. jkorous added inline comments. Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:131 + // the async event handling picks them up. Can make this test flaky. + std::this_thread::sleep_for(std::chrono::milliseconds(100)

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous updated this revision to Diff 201321. jkorous added a comment. Specify what "file modified" means and add a test for metadata change CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 Files: clang/cmake/modules/AddClang.cmake clang/incl

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous updated this revision to Diff 201338. jkorous added a comment. - simplify link libraries in cmake - fix Release build (messed-up asserts) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 Files: clang/include/clang/DirectoryWatcher/Direc

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous updated this revision to Diff 201744. jkorous added a comment. Remove DirectoryWatcher::Event::EventKind::Added CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 Files: clang/include/clang/DirectoryWatcher/DirectoryWatcher.h clang/lib/

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

jkorous added a comment. One more thing. On macOS FSEvents are coalesced more or less at will and it became quite apparent when I was creating automatic tests - I was for example receiving coalesced events Added & Modified & Removed. We had a discussion about how to deal with this and it turne

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

jkorous added a comment. In D55363#1337376 , @hokein wrote: > In D55363#1336315 , @ilya-biryukov > wrote: > > > Maybe consider sending an update after some period of time, e.g. `0.5s`? (I > > expect this particula

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

jkorous added a comment. This looks like a work around LSP imperfection indeed. Are you going to push for change in LSP? Something like `CompletionOptions/triggerCharacters` -> `CompletionOptions/triggerStrings`? export interface CompletionOptions { /** * The server provides

[PATCH] D56226: [clang-format] square parens that are followed by '=' are not Objective-C message sends

jkorous added a comment. Hi Alex, thanks for fixing this. I assume chance of testing an Objective-C header with assignment to method result without any other specifically Objective-C (detectable) construct is significantly lower than chance of testing a C99 header with designated initializers.

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

jkorous added a comment. @hokein I think you are correct. I meant that it would be possible to make LSP more generic by using trigger strings instead of trigger characters.. @ilya-biryukov Yes, that's true. I would still expect some performance gain in more restrictive filtering on client's side

[PATCH] D56226: [clang-format] square parens that are followed by '=' are not Objective-C message sends

jkorous added a comment. I was thinking the same (that we might be able to make the detection more precise) but I got the impression these cases are handled later (L524 and below). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56226/new/ https://reviews.llvm.or

[PATCH] D54428: [clangd] XPC transport layer, framework, test-client

jkorous updated this revision to Diff 180640. jkorous retitled this revision from "[clangd][WIP] XPC transport layer, framework, test-client" to "[clangd] XPC transport layer, framework, test-client". jkorous edited the summary of this revision. jkorous added a comment. added one more assert R

[PATCH] D54428: [clangd] XPC transport layer, framework, test-client

jkorous added a comment. @arphaman or @sammccall, could you please take the final look? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54428/new/ https://reviews.llvm.org/D54428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://

[PATCH] D54428: [clangd] XPC transport layer, framework, test-client

jkorous updated this revision to Diff 180809. jkorous edited the summary of this revision. jkorous added a comment. Fixed the synchronous handling of events. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54428/new/ https://reviews.llvm.org/D54428 Files: CMakeLists.txt clangd/CMakeL

[PATCH] D56523: Improve a -Wunguarded-availability note

jkorous added a reviewer: jkorous. jkorous added a comment. Hi Erik, this looks neat! Comment at: clang/test/Sema/availability-guard-format.mm:6 @interface foo -- (void) method_bar __attribute__((availability(macosx, introduced = 10_12))); // expected-note {{'method_bar' has

[PATCH] D67584: [Support] Replace function with function_ref in writeFileAtomically. NFC

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. Good point! Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67584/new/ https://reviews.llvm.org/D67584 __

[PATCH] D67522: [clang-scan-deps] Verbose mode

This revision was automatically updated to reflect the committed changes. Closed by commit rG1b87364f511a: [clang-scan-deps] Add verbose mode (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D67522?vs

[PATCH] D45569: [Sema] Disable built-in increment operator for bool in overload resolution in C++17

jkorous-apple created this revision. jkorous-apple added a reviewer: rsmith. Herald added a subscriber: cfe-commits. Following: https://llvm.org/svn/llvm-project/cfe/trunk@329804 For C++17 the wording of [over.built] p4 excluded bool: For every pair (T , vq), where T is an arithmetic type other

[PATCH] D45620: [clangd][tests] Fix handling of EOF in delimited input

jkorous-apple created this revision. Herald added subscribers: cfe-commits, MaskRay, ioeric, ilya-biryukov. Request in delimited input ended by EOF shouldn't be an error state. Comments should be allowed at the end of test files. Input mirroring should work for the last request in delimited test f

[PATCH] D45621: [clangd][tests] Fix delimiter handling

jkorous-apple created this revision. jkorous-apple added a project: clang-tools-extra. Herald added subscribers: cfe-commits, MaskRay, ioeric, ilya-biryukov. jkorous-apple edited the summary of this revision. Empty line shouldn't be considered a delimiter Following https://reviews.llvm.org/D45620

[PATCH] D41897: Fixing a crash in Sema.

jkorous-apple closed this revision. jkorous-apple added a comment. Landed as git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@322438 91177308-0d34-0410-b5e6-96231b3b80d8 https://reviews.llvm.org/D41897 ___ cfe-commits mailing list cfe-commit

[PATCH] D45763: [clangd][tests] Fix handling of EOF in delimited input

jkorous created this revision. jkorous added a project: clang-tools-extra. Herald added subscribers: cfe-commits, MaskRay, ioeric, jkorous-apple, ilya-biryukov. Request in delimited input ended by EOF shouldn't be an error state. Comments should be allowed at the end of test files. Input mirrorin

[PATCH] D45764: [clangd][tests] Fix delimiter handling

jkorous created this revision. jkorous added a project: clang-tools-extra. Herald added subscribers: cfe-commits, MaskRay, ioeric, jkorous-apple, ilya-biryukov. Empty line shouldn't be considered a delimiter. Following https://reviews.llvm.org/D45763 Repository: rCTE Clang Tools Extra https

[PATCH] D45763: [clangd][tests] Fix handling of EOF in delimited input

jkorous added a reviewer: sammccall. jkorous added a comment. Hi Sam, could you please take a look at this minor fix? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45763 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:

[PATCH] D45764: [clangd][tests] Fix delimiter handling

jkorous added a reviewer: sammccall. jkorous added a comment. Hi Sam, could you please take a look at this minor fix? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:

[PATCH] D45569: [Sema] Disable built-in increment operator for bool in overload resolution in C++17

This revision was automatically updated to reflect the committed changes. Closed by commit rC330254: [Sema] Disable built-in increment operator for bool in overload resolution in… (authored by jkorous, committed by ). Changed prior to commit: https://reviews.llvm.org/D45569?vs=142167&id=142929#

[PATCH] D45763: [clangd][tests] Fix handling of EOF in delimited input

jkorous updated this revision to Diff 143079. jkorous added a comment. Include the correct test. https://reviews.llvm.org/D45763 Files: JSONRPCDispatcher.cpp clangd/delimited-input-comment-at-the-end.test Index: clangd/delimited-input-comment-at-the-end.test ==

[PATCH] D45763: [clangd][tests] Fix handling of EOF in delimited input

jkorous added a comment. By "Comments should be allowed at the end of test files." I meant that we shouldn't log it as an exceptional state. Should've worded that better. I included a wrong test in the patch. I had realized the same thing as you and had written another test but messed up. htt

[PATCH] D45764: [clangd][tests] Fix delimiter handling

jkorous added a comment. [ultranit] Sorry, just to be sure - did you really mean any number of dashes if (LineRef.startswith("-") && LineRef.find_first_not_of('-') == llvm::StringRef::npos) or at least three? if (LineRef.startswith("---") && LineRef.find_first_not_of('-') == llvm::StringR

[PATCH] D45884: [Sema] Fix parsing of anonymous union in language linkage specification

jkorous created this revision. jkorous added reviewers: vsapsai, arphaman, rsmith. jkorous added a project: clang. Herald added a subscriber: cfe-commits. C++17 [dcl.link]p4: A linkage specification does not establish a scope. C++17 [class.union.anon]p2: Namespace level anonymous unions shall be

[PATCH] D45764: [clangd][tests] Fix delimiter handling

This revision was automatically updated to reflect the committed changes. Closed by commit rL330609: [clangd][tests] Fix delimiter handling (authored by jkorous, committed by ). Herald added subscribers: llvm-commits, klimek. Changed prior to commit: https://reviews.llvm.org/D45764?vs=142918&id

[PATCH] D45763: [clangd][tests] Fix handling of EOF in delimited input

jkorous closed this revision. jkorous added a comment. I forgot to mention review in commit message. git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@330608 91177308-0d34-0410-b5e6-96231b3b80d8 https://reviews.llvm.org/D45763

[PATCH] D45884: [Sema] Fix parsing of anonymous union in language linkage specification

jkorous updated this revision to Diff 143736. jkorous added a comment. Addressed comments + proposal for refactoring of the namespace-related logic. https://reviews.llvm.org/D45884 Files: Sema/SemaDecl.cpp SemaCXX/anonymous-union-export.cpp SemaCXX/anonymous-union.cpp Index: SemaCXX/ano

[PATCH] D45884: [Sema] Fix parsing of anonymous union in language linkage specification

jkorous marked an inline comment as done. jkorous added a comment. Volodymyr, could you please confirm that the non-anonymous vs non-inline logic makes sense to you? Comment at: Sema/SemaDecl.cpp:4659-4661 + (isa(OwnerScope) || + (isa(OwnerScope) && +

[PATCH] D45884: [Sema] Fix parsing of anonymous union in language linkage specification

jkorous added inline comments. Comment at: Sema/SemaDecl.cpp:4651-4653 + + DeclContext *OwnerScope = Owner->getRedeclContext(); + vsapsai wrote: > I think the code style favours avoiding excessive vertical whitespace and I > don't feel like this statement i

[PATCH] D48559: [clangd] refactoring for XPC transport layer [NFCI]

jkorous created this revision. jkorous added a project: clang-tools-extra. Herald added subscribers: cfe-commits, MaskRay, ioeric, ilya-biryukov, mgorny. Hi all, We finally finished a self-contained first version of our implementation of alternative transport layer for macOS based on XPC. To en

[PATCH] D48560: [clangd] JSON <-> XPC conversions

jkorous created this revision. jkorous added a project: clang-tools-extra. Herald added subscribers: cfe-commits, MaskRay, ioeric, ilya-biryukov, mgorny. This is a self-contained pair of utility functions for the XPC transport layer. It's not dependent on but following the refactoring patch: http

[PATCH] D48562: [clangd] XPC transport layer

jkorous created this revision. jkorous added a project: clang-tools-extra. Herald added subscribers: cfe-commits, MaskRay, ioeric, ilya-biryukov, mgorny. Implementation of alternative transport layer for macOS based on XPC. Based on these two other patches: https://reviews.llvm.org/D48559 https:/

[PATCH] D48559: [clangd] refactoring for XPC transport layer [NFCI]

jkorous added a comment. Followed by these two patches: [clangd] JSON <-> XPC conversions https://reviews.llvm.org/D48560 [clangd] XPC transport layer https://reviews.llvm.org/D48562 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48559

[PATCH] D48562: [clangd] XPC transport layer

jkorous added inline comments. Comment at: xpc/test-client/ClangdXPCTestClient.cpp:51 + dlHandle, "clangd_xpc_get_bundle_identifier"); + xpc_connection_t conn = + xpc_connection_create(clangd_xpc_get_bundle_identifier(), NULL); arphaman wrote: > We

[PATCH] D48562: [clangd] XPC transport layer

jkorous updated this revision to Diff 152888. jkorous added a comment. Two changes in test client based on internal review by XPC folks: - Removed cleanup code at the end of main() as dispatch_main() never returns. - Removed check for conn as xpc_connection_create() is guaranteed to succeed. ht

[PATCH] D48559: [clangd] refactoring for XPC transport layer [NFCI]

jkorous added a comment. Ping. Added reviewers suggestion. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48559 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48560: [clangd] JSON <-> XPC conversions

jkorous marked an inline comment as done. jkorous added a comment. Hi Sam, no worries! Thanks for making time to take a look! I am going to rebase all my patches on top of JSON library moved to llvm/Support and process your comments. Comment at: clangd/xpc/CMakeLists.txt:1 +s

[PATCH] D48559: [clangd] refactoring for XPC transport layer [NFCI]

jkorous added a comment. Hi Sam, thanks for your feedback! In general I agree that explicitly factoring out the transport layer and improving layering would be a good thing. Unfortunately it's highly probable that we'd like to drop JSON completely from XPC dispatch (XPC -> JSON -> ProtocolCall

[PATCH] D48562: [clangd] XPC transport layer

jkorous added a reviewer: sammccall. jkorous added a comment. Hi Sam, I am definitely open to discussion about the right abstraction. I will push patches rebased on TOT and changes based on your comments today or tomorrow and I am trying to figure out how could we use your Transport abstraction

[PATCH] D48786: [Preprocessor] Stop entering included files after hitting a fatal error.

jkorous added a reviewer: jkorous. jkorous added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:1875 + // Stop further preprocessing if a fatal error has occurred. Any diagnostics + // we might have raised will not be visible. + if (ShouldEnter && Diags->hasFatalE

[PATCH] D48786: [Preprocessor] Stop entering included files after hitting a fatal error.

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM with just a nit about comment wording. Thanks for the patch! Comment at: clang/lib/Lex/PPDirectives.cpp:1875 + // Stop further preprocessing if a fatal error has occ

<    1   2   3   4   5   >