[PATCH] D54799: [clangd][WIP] textDocument/CursorInfo method

jkorous marked 7 inline comments as done. jkorous added a comment. Hi Sam. In https://reviews.llvm.org/D54799#1305470, @sammccall wrote: > >> I'd like to ask for early feedback - what's still missing is relevant client >> capability. > > I'd suggest leaving it out unless others feel strong

[PATCH] D54799: [clangd][WIP] textDocument/CursorInfo method

jkorous updated this revision to Diff 175069. jkorous marked 2 inline comments as done. jkorous added a comment. Changes based on review. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54799 Files: ClangdLSPServer.cpp ClangdLSPServer.h ClangdServer.cpp ClangdServer.h

[PATCH] D54799: [clangd][WIP] textDocument/CursorInfo method

jkorous marked 7 inline comments as done. jkorous added a comment. In https://reviews.llvm.org/D54799#1306585, @sammccall wrote: > So I think both SymbolID and USR are optional. No problem. I am just wondering if it make sense to include any symbol with empty name, empty USR and no ID in LSP r

[PATCH] D54799: [clangd][WIP] textDocument/CursorInfo method

jkorous updated this revision to Diff 175115. jkorous marked 2 inline comments as done. jkorous added a comment. Herald added a subscriber: mgorny. Couple minor changes based on discussion. - Move `SymbolID` to `index/SymbolID.h`. - Rename in `ClangdServer` - drop the verb from method name. - Rem

[PATCH] D54799: [clangd][WIP] textDocument/CursorInfo method

jkorous updated this revision to Diff 175137. jkorous added a comment. Multiple symbols per location. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54799/new/ https://reviews.llvm.org/D54799 Files: clangd/CMakeLists.txt clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd

[PATCH] D54796: [clangd] C++ API for emitting file status

jkorous added a comment. Hi @hokein, I am just keeping up to date with changes. Comment at: clangd/ClangdServer.h:39 +// FIXME: find a better name. class DiagnosticsConsumer { It would be unfortunate to have this name clashing with `clang::DiagnosticsConsum

[PATCH] D54799: [clangd][WIP] textDocument/CursorInfo method

jkorous marked 8 inline comments as done. jkorous added a comment. Thank you very much for the review Sam! I am going to write proper unit tests and then just wait for Alex and Ben to take a look. Comment at: clangd/XRefs.cpp:785 +} +Results.emplace_back(std::move(New

[PATCH] D54799: [clangd][WIP] textDocument/CursorInfo method

jkorous updated this revision to Diff 175229. jkorous marked an inline comment as done. jkorous added a comment. Addressed comments from the reivew. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54799/new/ https://reviews.llvm.org/D54799 Files: clangd/CMakeLists.txt clangd/ClangdLS

[PATCH] D54799: [clangd][WIP] textDocument/CursorInfo method

jkorous updated this revision to Diff 175230. jkorous added a comment. Removed empty line noise and fixed doxygen annotation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54799/new/ https://reviews.llvm.org/D54799 Files: clangd/CMakeLists.txt clangd/ClangdLSPServer.cpp clangd/Cl

[PATCH] D54799: [clangd][WIP] textDocument/SymbolInfo method

jkorous marked 5 inline comments as done. jkorous added inline comments. Comment at: clangd/ClangdServer.cpp:529 + + WorkScheduler.runWithAST("CursorInfo", File, Bind(Action, std::move(CB))); +} sammccall wrote: > sammccall wrote: > > nit: SymbolInfo > (This sti

[PATCH] D54799: [clangd][WIP] textDocument/SymbolInfo method

jkorous updated this revision to Diff 175314. jkorous marked an inline comment as done. jkorous added a comment. Adressed last comments + initial unit tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54799/new/ https://reviews.llvm.org/D54799 Files: CMakeLists.txt ClangdLSPServ

[PATCH] D54529: [clangd] Add USR to textDocument/definition response

jkorous abandoned this revision. jkorous added a comment. Morphed into https://reviews.llvm.org/D54799 Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54529/new/ https://reviews.llvm.org/D54529 ___ cfe-co

[PATCH] D54799: [clangd][WIP] textDocument/SymbolInfo method

jkorous updated this revision to Diff 175475. jkorous added a comment. Finished tests and fixed operator<<(SymbolDetails) for containerNames that aren't part of fully qualified name. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54799/new/ https://reviews.llvm.org/D54799 Files: clan

[PATCH] D54799: [clangd] textDocument/SymbolInfo method

jkorous added a comment. Going to split `SymbolID.h ` as a NFC commit per Alex's suggestion and land this. Thanks everyone! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54799/new/ https://reviews.llvm.org/D54799 ___ cfe-commits mailing li

[PATCH] D54799: [clangd] textDocument/SymbolInfo method

This revision was automatically updated to reflect the committed changes. Closed by commit rL347674: [clangd][NFC] Move SymbolID to a separate file (authored by jkorous, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D54799?vs=175475&i

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

jkorous updated this revision to Diff 175884. jkorous marked 19 inline comments as done. jkorous retitled this revision from "[clangd] XPC transport layer, framework, test-client" to "[clangd][WIP] XPC transport layer, framework, test-client". jkorous edited the summary of this revision. jkorous a

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

jkorous added reviewers: ilya-biryukov, ioeric. jkorous added a comment. Since AFAIK Sam is off until the end of the year I am adding more reviewers. Comment at: tool/ClangdMain.cpp:329 +if (getenv("CLANGD_AS_XPC_SERVICE")) + return newXPCransport(); +#endif --

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

jkorous marked 18 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) { jko

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

jkorous updated this revision to Diff 188423. jkorous marked 4 inline comments as done. jkorous added a comment. - Use RetryAfterSignal - Propagate events from inotify directly - Remove ModTime - Add assert for unknown event kinds CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/

[PATCH] D57628: [index] Improve indexing support for MSPropertyDecl.

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM Thanks for looking into this! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57628/new/ https://reviews.llvm.org/D57628 ___ cfe-c

[PATCH] D58659: [Sema] Fix assertion when `auto` parameter in lambda has an attribute.

jkorous added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:266 +auto *NewAttrTy = cast(T.getTypePtr()); +for (TypeAttrPair &A : AttrsForTypes) { + if (A.first == AttrTy) Do you think it would make sense to terminate the loop e

[PATCH] D58600: [clangd] Emit source to Diagnostic.

jkorous added inline comments. Comment at: clangd/Diagnostics.cpp:381 LastDiag = Diag(); +LastDiag->ID = Info.getID(); FillDiagBase(*LastDiag); Nit - is this really intended to be part of this patch? Comment at: clangd/Diagnostics

[PATCH] D58089: Add missing library dependencies in CMakeLists.txt

jkorous requested changes to this revision. jkorous added a comment. This revision now requires changes to proceed. Hi @comex, I totally missed this! Thanks for the patch. I am not entirely happy about this dependency though. I am trying to split off the only parts of clangDaemon that XpcTranspor

[PATCH] D58089: Add missing library dependencies in CMakeLists.txt

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. Reconsidered - let's fix the build first and I can finish my patch tomorrow. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58089/new/ https://r

[PATCH] D58089: Add missing library dependencies in CMakeLists.txt

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE354949: [clangd] Library dependencies in XPC (authored by jkorous, committed by ). Changed prior to commit: https://reviews.llvm.org/D58089?vs=186369&id=188494#toc Repository: rCTE Clang Tools Extr

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

jkorous updated this revision to Diff 188586. jkorous added a comment. - moved checks for CoreServices/inotify to cmake 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] D58478: [index-while-building] FileIndexRecord

jkorous added a comment. ping -c 1 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-commits

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

This revision was automatically updated to reflect the committed changes. Closed by commit rC355035: [clang][index-while-building] FileIndexRecord (authored by jkorous, committed by ). Changed prior to commit: https://reviews.llvm.org/D58478?vs=187863&id=188614#toc Repository: rC Clang CHAN

[PATCH] D58749: [index-while-building] IndexRecordHasher

jkorous created this revision. jkorous added reviewers: nathawes, akyrtzi, arphaman, dexonsmith, ioeric, malaperle. Herald added subscribers: cfe-commits, jdoerfert, mgorny. Herald added a project: clang. Another piece of index-while-building functionality. RFC: http://lists.llvm.org/pipermail/cf

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

jkorous added a comment. @xbolva00 I prefer to keep it here for now. I suggest we wait with upstreaming until an actual need for use in some other project arises. This is what we basically did with VFS. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/

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

jkorous updated this revision to Diff 188783. jkorous added a comment. - fix the linux implementation - clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 Files: clang/include/clang/DirectoryWatcher/DirectoryWatcher.h clang/lib/CMa

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

jkorous updated this revision to Diff 188808. 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 clang/lib/DirectoryWatcher/CMakeLists.txt clang/lib/Direc

[PATCH] D57965: Clean up ObjCPropertyDecl printing

jkorous added a comment. Hi David, I am just wondering - while you're here would you mind adding couple more tests? It would be great to have a test for each attribute. Also, what do you think about Ben's suggestion? I think it would be nice to be consistent with the style Objective-C documenta

[PATCH] D58922: [clang-format] broken after lambda with return type template with boolean literal

jkorous added a comment. Thanks for fixing this! The patch LGTM. I am just wondering what else are we missing here. I guess arithmetic and logical operators are also allowed: template struct foo {}; int main() { auto lambda1 = []() -> foo { return foo{}; }; auto lambda2 =

[PATCH] D58934: [clang-format] Fix lambdas returning template specialization that contains operator in parameter

jkorous created this revision. jkorous added reviewers: klimek, djasper, JonasToth, alexfh, krasimir, MyDeveloperDay. Herald added subscribers: cfe-commits, jdoerfert, dexonsmith. Herald added a project: clang. Inspired by https://reviews.llvm.org/D58922 Since this code compiles I assume we shou

[PATCH] D58922: [clang-format] broken after lambda with return type template with boolean literal

jkorous added a comment. I added couple other cases to your fix here: https://reviews.llvm.org/D58934 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58922/new/ https://reviews.llvm.org/D58922 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D58922: [clang-format] broken after lambda with return type template with boolean literal

jkorous added a comment. BTW I suggest you also add the original test case since the test cases you added fail the expectation at FormatTest.cpp:74 and the message is a bit unclear whether the testcase or the actual implementation is at fault. EXPECT_EQ(Expected.str(), format(Expected, Style)

[PATCH] D58941: [clang-format][docs][NFC] Fix example for Allman brace breaking style

jkorous created this revision. jkorous added reviewers: JDevlieghere, krasimir, benhamilton. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. I assume the example is wrong as it's clearly missing line-breaks before braces. That's about my level of understanding an

[PATCH] D58941: [clang-format][docs][NFC] Fix example for Allman brace breaking style

This revision was automatically updated to reflect the committed changes. Closed by commit rL355365: [clang-format][docs][NFC] Fix example for Allman brace breaking style (authored by jkorous, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D58749: [index-while-building] IndexRecordHasher

jkorous added reviewers: kadircet, gribozavr. jkorous added a comment. Adding clangd folks in case they want to take a look. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58749/new/ https://reviews.llvm.org/D58749 __

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

jkorous added reviewers: kadircet, gribozavr. jkorous added a comment. Adding clangd folks in case they want to take a look. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 ___ cfe-commits mailing li

[PATCH] D58922: [clang-format] broken after lambda with return type template with boolean literal

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. In D58922#1418029 , @MyDeveloperDay wrote: > do you mean this case? as this seems to work for me? > > verifyFormat("namespace bar {\n" >

[PATCH] D58934: [clang-format] Fix lambdas returning template specialization that contains operator in parameter

jkorous added a comment. In D58934#1418043 , @MyDeveloperDay wrote: > 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,foo I hear you :D > As there are

[PATCH] D58934: [clang-format] Fix lambdas returning template specialization that contains operator in parameter

This revision was automatically updated to reflect the committed changes. Closed by commit rL355434: [clang-format] Fix lambdas returning template specialization that contains… (authored by jkorous, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed pri

[PATCH] D39050: Add index-while-building support to Clang

jkorous abandoned this revision. jkorous added a comment. It's time to officially abandon these patches in favor of new push for upstreaming index-while-building. Current reviews in progress https://reviews.llvm.org/D58749 https://reviews.llvm.org/D58418 RFC http://lists.llvm.org/pipermail/cfe-

[PATCH] D39050: Add index-while-building support to Clang

jkorous added a comment. I'll address comments for this patch in the new set of patches. @gribozavr I haven't put up this part of code for the new round of review yet. I will keep this on mind. @mgrang This already landed in edbbe470f66

[PATCH] D58749: [index-while-building] IndexRecordHasher

jkorous updated this revision to Diff 190317. jkorous added a comment. Herald added a subscriber: jfb. Based on Kadir comment I refactored the code. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58749/new/ https://reviews.llvm.org/D58749 Files: clang/lib/Index

[PATCH] D58749: [index-while-building] IndexRecordHasher

jkorous added a comment. In D58749#1426270 , @gribozavr wrote: > 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, coul

[PATCH] D58749: [index-while-building] IndexRecordHasher

jkorous updated this revision to Diff 190342. jkorous marked 2 inline comments as done. jkorous added a comment. Addressed some of Dmitri's comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58749/new/ https://reviews.llvm.org/D58749 Files: clang/lib/Index/CMakeLists.txt clang

[PATCH] D58749: [index-while-building] IndexRecordHasher

jkorous marked 8 inline comments as done. jkorous added a comment. Addressed some comments, going to update the diff. Comment at: clang/lib/Index/IndexRecordHasher.cpp:291 + +hash_code IndexRecordHasher::hashImpl(const Decl *D) { + return DeclHashVisitor(*this).Visit(D); -

[PATCH] D58749: [index-while-building] IndexRecordHasher

jkorous added a comment. In D58749#1426769 , @kadircet > In D58749#1426778 , @gribozavr > I see what you mean now. That's a good idea. I'll add some unit tests. CHANGES SINCE LAST ACTION https://reviews.l

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

jkorous added a comment. Ping. 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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

jkorous updated this revision to Diff 190744. jkorous added a comment. Remove duplicate comment 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 clang/l

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

jkorous added a comment. In D58418#1430160 , @thakis wrote: > 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 wa

[PATCH] D59300: [clangd] Tune the fuzzy-matching algorithm

jkorous added a comment. @ilya-biryukov could you please give details about the quality metric you are using and some description of posted measurements? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59300/new/ https://reviews.llvm.org/D59300 _

[PATCH] D59377: Frontend: Remove CompilerInstance::VirtualFileSystem, NFC

jkorous added a comment. In D59377#1430002 , @dexonsmith wrote: > ... since I noticed that FileManager ... This kind of implies that we should move the comment from `FileManager` constructor implementation to the header thus making it an explicit part o

[PATCH] D59388: Basic: Return a reference from FileManager::getVirtualFileSystem, NFC

jkorous added a comment. Hi Duncan, thanks for working on better interfaces in clang! I am just wondering - is it safe to have the lifetime of a single object on heap managed by two different `IntrusiveRefCntPtr` instances? Comment at: clang/lib/Frontend/ASTUnit.cpp:1693

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

jkorous added a comment. In D58418#1430630 , @thakis wrote: > In D58418#1430490 , @jkorous wrote: > > > In D58418#1430160 , @thakis wrote: > > > > > Why is this needed for in

[PATCH] D59388: Basic: Return a reference from FileManager::getVirtualFileSystem, NFC

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. In D59388#1433233 , @dexonsmith wrote: > Yes, it's safe. The reference count is "intrusive", meaning it's stored in > the object itself (via inherit

[PATCH] D59605: [clangd] Introduce background-indexer

jkorous added inline comments. Comment at: clang-tools-extra/clangd/background-indexer/BackgroundIndexer.cpp:56 + // non-interactive tools like this one. + 24 * 60 * 60 * 1000); + llvm::SmallString<128> DummyFile(CompileCommandsDir); Nit: maybe we shou

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

jkorous marked an inline comment as done. jkorous added a comment. In D58418#1431765 , @thakis wrote: > In D58418#1431399 , @jkorous wrote: > > > In D58418#1430630 , @thakis

[PATCH] D57965: Clean up ObjCPropertyDecl printing

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM. Thanks for working on this! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57965/new/ https://reviews.llvm.org/D57965 ___

[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line

jkorous added a comment. @russellmcc the patch has been approved by @MyDeveloperDay https://reviews.llvm.org/D40988#1430502 Do you mean you don't have commit access and need someone to land the patch on your behalf? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40988/new/ https://revi

[PATCH] D59377: Frontend: Remove CompilerInstance::VirtualFileSystem, NFC

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

[PATCH] D60120: check-clang-tools: Actually build and run XPC test

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

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

jkorous created this revision. jkorous added reviewers: gribozavr, arphaman, dexonsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. - I assume we can cache comments for canonical declarations only, not for every redeclaration. - Caching that we didn't find comments see

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

jkorous marked an inline comment as done. jkorous added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:372 D = adjustDeclToTemplate(D); + const Decl* CanonicalDecl = D->getCanonicalDecl(); arphaman wrote: > Why are we now checking for the canonic

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

jkorous marked an inline comment as done. jkorous added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:372 D = adjustDeclToTemplate(D); + const Decl* CanonicalDecl = D->getCanonicalDecl(); gribozavr wrote: > jkorous wrote: > > arphaman wrote: > >

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

jkorous updated this revision to Diff 194604. jkorous added a comment. Second attempt on reducing the cache size and number of operations. I try in this order 1. cache lookup for the specific provided declaration 2. try to find comment attached to the specific provided declaration without using

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

jkorous updated this revision to Diff 194609. jkorous added a comment. rename RawCommentAndCacheFlags -> CommentAndOrigin CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60432/new/ https://reviews.llvm.org/D60432 Files: clang/include/clang/AST/ASTContext.h clang/lib/AST/ASTContext.cp

[PATCH] D58749: [index-while-building] IndexRecordHasher

jkorous added a comment. 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%. We ar

[PATCH] D60233: [clang-scan-deps] initial outline of the tool that runs preprocessor to find dependencies over a JSON compilation database

jkorous added inline comments. Comment at: tools/clang-scan-deps/ClangScanDeps.cpp:39 + bool runInvocation(std::shared_ptr Invocation, + FileManager *Files, + std::shared_ptr PCHContainerOps, `Files` -> `FileMgr` might be

[PATCH] D60523: [clang] Don't segfault on incorrect using directive (PR41400)

jkorous added a comment. 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. With current master (95c18c7beec

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

jkorous created this revision. jkorous added reviewers: gribozavr, arphaman. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. Loading external comments and sorting them is expensive - mostly due to getDecomposedLoc() begin expensive. For modules with very large nu

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

jkorous created this revision. jkorous added reviewers: gribozavr, arphaman. Herald added subscribers: cfe-commits, dexonsmith, mgrang. Herald added a project: clang. For large number of deserialized comments (~100k) the way how we try to attach them to declaration in completion comments is too e

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

jkorous marked 5 inline comments as done. jkorous added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:494 + // Explicitly not calling ExternalSource->ReadComments() as we're not + // interested in those. + ArrayRef RawComments = Comments.getComments();

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

jkorous updated this revision to Diff 196744. jkorous added a comment. - clang-format - comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61103/new/ https://reviews.llvm.org/D61103 Files: clang/include/clang/AST/ASTContext.h clang/lib/AST/ASTContext.cpp clang/lib/Sema/SemaDe

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

jkorous added a comment. 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? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D6110

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

jkorous added a comment. @MaskRay @juliehockett I'll take a look. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61187/new/ https://reviews.llvm.org/D61187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

jkorous added a comment. Patch with fix for XPC tests https://reviews.llvm.org/D61271 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61187/new/ https://reviews.llvm.org/D61187 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D61271: [clangd][xpc] Fix XPC unittests

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE359489: [clangd][xpc] Fix XPC unittests (authored by jkorous, committed by ). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rCTE Clang Tools Extra CHANGES SINCE L

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

jkorous planned changes to this revision. jkorous added a comment. @gribozavr thanks for the feedback. I'm rewriting the patch now as I figured out my detection of comments preceding declarations is unsound. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61104/new

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

jkorous added a comment. 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. CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D51867: [Diagnostics] Add error handling to FormatDiagnostic()

jkorous added a comment. Hi Volodymyr, do you think you might take another look? Repository: rC Clang https://reviews.llvm.org/D51867 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion

jkorous created this revision. jkorous added reviewers: arphaman, vsapsai, sammccall, ilya-biryukov. jkorous added a project: clang. Herald added subscribers: cfe-commits, dexonsmith, eraman. Destructors don't have return type "void", they don't have any return type at all. Repository: rC Cla

[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion

jkorous added a comment. You might be right - I am assuming here that we want consistent behaviour between constructors and destructors. IIUC ctors are currently skipped in code completions (in most cases) but they also don't have any type associated while result of their call definitely has s

[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion

jkorous added a comment. Sorry my bad. You are right, we aren't showing destructors in clangd for normal classes. The case where I noticed is kind of a corner case with template class. {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{

[PATCH] D52308: [Sema][CodeCompletion] Fix return type of C++ destructors in code completion

jkorous abandoned this revision. jkorous added a comment. Allright, I will remove destructor from clangd completion results which solves this particular issue. Also good point about return type being used in ranking - I incorrectly assumed it's just a presentational matter. Repository: rC C

[PATCH] D52495: [WIP][Sema][CodeCompletion] Add base type for double colon member completion results

jkorous created this revision. Herald added subscribers: cfe-commits, dexonsmith. Repository: rC Clang https://reviews.llvm.org/D52495 Files: Frontend/ASTUnit.cpp Sema/CodeCompleteConsumer.cpp Sema/SemaCodeComplete.cpp clang/Sema/CodeCompleteConsumer.h libclang/CIndexCodeCompletion.c

[PATCH] D52554: [WIP] [clangd] Tests for special methods code-completion

jkorous created this revision. jkorous added reviewers: sammccall, ilya-biryukov. jkorous added a project: clang-tools-extra. Herald added subscribers: cfe-commits, kadircet, arphaman, dexonsmith, MaskRay, ioeric. Created in order to check we agree on what are the requirements and would later wr

[PATCH] D58213: [FileManager] fillRealPathName even if we aren't opening the file

jkorous created this revision. jkorous added reviewers: sammccall, arphaman. Herald added subscribers: cfe-commits, kadircet, dexonsmith, ioeric, ilya-biryukov. Herald added a project: clang. This fixes an internal test failure in clangd - ClangdTests.GoToInclude.All rdar://47536127 Repository

[PATCH] D58213: [FileManager] fillRealPathName even if we aren't opening the file

jkorous added a comment. @sammccall I think you touched this part of the code recently in r352079 (revert of r347205). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58213/new/ https://reviews.llvm.org/D58213 ___ cfe

[PATCH] D58213: [FileManager] fillRealPathName even if we aren't opening the file

jkorous marked 2 inline comments as done. jkorous added a comment. Thanks for taking a look! Comment at: lib/Basic/FileManager.cpp:214 return nullptr; } If we can't stat the file we return here. Comment at: lib/Basic/FileManager.cpp

[PATCH] D58213: [FileManager] fillRealPathName even if we aren't opening the file

jkorous marked an inline comment as done. jkorous added inline comments. Comment at: unittests/Basic/FileManagerTest.cpp:349 +// rdar://problem/47536127 +TEST_F(FileManagerTest, getFileDontOpenRealPath) { arphaman wrote: > Please leave this comment out Sure, no

[PATCH] D58213: [FileManager] fillRealPathName even if we aren't opening the file

jkorous updated this revision to Diff 186906. jkorous added a comment. - comments - explicit openFile = false in test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58213/new/ https://reviews.llvm.org/D58213 Files: lib/Basic/FileManager.cpp unittests/Basic/FileManagerTest.cpp Inde

[PATCH] D58213: [FileManager] fillRealPathName even if we aren't opening the file

jkorous marked 4 inline comments as done. jkorous added a comment. Updated the patch. Comment at: unittests/Basic/FileManagerTest.cpp:349 +// rdar://problem/47536127 +TEST_F(FileManagerTest, getFileDontOpenRealPath) { arphaman wrote: > jkorous wrote: > > arph

[PATCH] D58213: [FileManager] fillRealPathName even if we aren't opening the file

This revision was automatically updated to reflect the committed changes. jkorous marked an inline comment as done. Closed by commit rC354075: [clang][FileManager] fillRealPathName even if we aren't opening the file (authored by jkorous, committed by ). Repository: rC Clang CHANGES SINCE LAST

[PATCH] D58185: [clangd] Handle unresolved scope specifier when fixing includes.

jkorous added a comment. Hi Eric, I have just couple details. Comment at: clangd/IncludeFixer.cpp:188 +// first character after the unresolved name in \p Code. For the example below, +// this returns "::X::Y" that is qualfied by unresolved name "clangd": +// clang::clangd::

[PATCH] D58291: [clangd] Include textual diagnostic ID as Diagnostic.code.

jkorous requested changes to this revision. jkorous added a comment. This revision now requires changes to proceed. Hi Sam, this looks good! I found just one small detail. Comment at: clangd/Diagnostics.cpp:281 +if (auto* Name = getDiagnosticCode(D.ID)) + Main.code =

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

jkorous created this revision. jkorous added reviewers: arphaman, dexonsmith, akyrtzi, nathawes, yvvan. Herald added subscribers: cfe-commits, jdoerfert, jfb, mgorny. Herald added a project: clang. This patch contains implementation of DirectoryWatcher from github.com/apple/swift-clang We are st

<    1   2   3   4   5   >