[PATCH] D50452: [WIP] clangd XPC adapter

jkorous created this revision. jkorous added reviewers: arphaman, sammccall, ilya-biryukov, simark. Herald added subscribers: cfe-commits, dexonsmith, MaskRay, ioeric, mgorny. Based on our internal discussions we decided to change our direction with XPC support for clangd. We did some extra measu

[PATCH] D49548: [clangd] XPC WIP

jkorous abandoned this revision. jkorous added a comment. We decided to abandon this direction in favor of simpler solution. https://reviews.llvm.org/D50452 One of the reasons is that there's a design fault - handling of server-originated messages (notifications) in this patch. Repository: r

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

jkorous added a comment. Hi, we decided to go with a different solution: https://reviews.llvm.org/D50452 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48559 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.

[PATCH] D48562: [clangd] XPC transport layer

jkorous abandoned this revision. jkorous added a comment. We decided to go with a different solution: https://reviews.llvm.org/D50452 https://reviews.llvm.org/D48562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-b

[PATCH] D50641: [clangd][test] Fix exit messages in tests

jkorous created this revision. jkorous added reviewers: sammccall, ilya-biryukov. jkorous added a project: clang-tools-extra. Herald added subscribers: cfe-commits, arphaman, dexonsmith, MaskRay, ioeric. There's a small typo in tests - causing that we aren't sending exit LSP message to clangd but

[PATCH] D50641: [clangd][test] Fix exit messages in tests

jkorous added a comment. I think that by introducing different codepath for testing purposes we might end up with fewer bugs in tests but the actual production code could become less tested. Actually, even the -lit-test itself might be not the theoretically most correct approach but I do see th

[PATCH] D50641: [clangd][test] Fix exit messages in tests

jkorous added a comment. I can see the value of getting more information in case of unexpected test behaviour but I still don't really like to have separate codepath for testing purposes. Anyway, it's not a big deal and it looks like you guys are all in agreement about this. I created a patch

[PATCH] D50641: [clangd][test] Fix exit messages in tests

This revision was automatically updated to reflect the committed changes. Closed by commit rL339781: [clangd][tests] Fix typo in tests - invalid LSP exit message (authored by jkorous, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D506

[PATCH] D50785: [clangd][tests] Add exit(EXIT_FAILURE) in case of JSON parsing failure in TestMode

jkorous created this revision. jkorous added reviewers: arphaman, ilya-biryukov. jkorous added a project: clang-tools-extra. Herald added subscribers: cfe-commits, dexonsmith, MaskRay, ioeric. This is meant to cause a visible fail when clangd gets invalid JSON payload in -lit-test mode. Based on

[PATCH] D50462: Try building complete AST after a fatal error was emitted if further diagnostics are expected

jkorous added a reviewer: vsapsai. jkorous added a comment. Adding Volodymyr who landed related patch recently. Repository: rC Clang https://reviews.llvm.org/D50462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[PATCH] D50740: [SourceManager] isPointWithin: avoid using isBeforeInTranslationUnit, compare buffer offsets directly for lexical correctness

jkorous added a comment. Hi Alex, nice work! I am just wondering if it would be beneficial to assert Loc and End are in the same file. It might help to catch bugs. I also stumbled upon this function but not sure it makes things significantly cleaner here: https://clang.llvm.org/doxygen/SourceL

[PATCH] D50785: [clangd][tests] Add exit(EXIT_FAILURE) in case of JSON parsing failure in TestMode

jkorous added a comment. Oh, I thought that what everyone wanted was test-specific behaviour. I like both approaches you propose much more! If we go for the generic option we can effectively start "checking stderr" in tests by using the flag. That would be nice. Just to be sure we are all on th

[PATCH] D46747: [Sema] Use dotted form of macOS version for unguarded availability FixIts

jkorous created this revision. jkorous added reviewers: arphaman, erik.pilkington, vsapsai. jkorous added a project: clang. Herald added a subscriber: cfe-commits. E. g. use "10.11" instead of "10_11". This is just a consistency fix - the dotted format is already used everywhere else. rdar://pr

[PATCH] D46747: [Sema] Use dotted form of macOS version for unguarded availability FixIts

jkorous planned changes to this revision. jkorous added a comment. Sorry, my bad. I tried to get rid of dependency on Foundation.h and didn't check the test is relevant for the fix after that. #import @interface foo - (void) method NS_AVAILABLE_MAC(10_12); @end int main() {

[PATCH] D46747: [Sema] Use dotted form of macOS version for unguarded availability FixIts

jkorous updated this revision to Diff 146605. jkorous added a comment. Fixed the test. It turned out that version component separator for availability is set during parsing which is why all other tests (including my bad one) are passing. The only way how to set underscore as a separator is thro

[PATCH] D46747: [Sema] Use dotted form of macOS version for unguarded availability FixIts

jkorous added a comment. I am not 100% sure it's the best thing to set printing style at the point of parsing version tuples but I am not sure it's bad either. Unless someone convinces me otherwise I would rather not do any major changes. https://reviews.llvm.org/D46747

[PATCH] D46834: [Sema][Cxx17] Error message for C++17 static_assert(pred) without string literal

jkorous created this revision. jkorous added reviewers: arphaman, dexonsmith, rsmith, doug.gregor. jkorous added a project: clang. Herald added a subscriber: cfe-commits. C++17 adds form of static_assert without string literal. static_assert ( bool_constexpr ) Currently clang produces diagnost

[PATCH] D46485: Add python tool to dump and construct header maps

jkorous added a comment. Hi Bruno, I just noticed couple of implementation details. Comment at: utils/hmaptool/hmaptool:55 +# The number of buckets must be a power of two. +if num_buckets == 0 or (num_buckets & num_buckets - 1) != 0: +rai

[PATCH] D46747: [Sema] Use dotted form of macOS version for unguarded availability FixIts

jkorous planned changes to this revision. jkorous added a comment. Sorry for me being dense here - since the output format is determined by input source code there's more work to do. I tried to change format of introduced version in couple of tests and the output mirrors the change. That basica

[PATCH] D46747: [Sema] Use dotted form of macOS version for unguarded availability FixIts

jkorous added a comment. Eric, thanks for the context. I am now clarifying internally and if possible would like to simplify the whole format business by using just one delimiter everywhere. Would that make sense to you or do you think we should respect delimiter used in sources? https://revi

[PATCH] D46834: [Sema][Cxx17] Error message for C++17 static_assert(pred) without string literal

jkorous abandoned this revision. jkorous added a comment. We reconsidered this in light of the policy - thanks for pointing that out Richard! Just to be sure that I understand it - the policy is meant for CLI and not serialized diagnostics, right? Repository: rC Clang https://reviews.llvm.

[PATCH] D46485: Add python tool to dump and construct header maps

jkorous added a comment. LGTM but my review was fairly superficial. Comment at: utils/hmaptool/hmaptool:55 +# The number of buckets must be a power of two. +if num_buckets == 0 or (num_buckets & num_buckets - 1) != 0: +raise SystemExit("e

[PATCH] D46747: [Sema] Use dotted form of macOS version for unguarded availability FixIts

jkorous updated this revision to Diff 147184. jkorous added a comment. This revision is now accepted and ready to land. After some internal discussion we agreed that we can simplify things and get consistent behaviour by using dot everywhere in diagnostics. This is pretty much revert of r219124

[PATCH] D46747: [Sema] Use dotted form of macOS version for unguarded availability FixIts

This revision was automatically updated to reflect the committed changes. Closed by commit rL332598: Use dotted format of version tuple for availability diagnostics (authored by jkorous, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D

[PATCH] D38643: PR13575: Fix USR mangling for fixed-size arrays.

jkorous-apple created this revision. Added array type mangling to USR generation. Included test from bug report. Repository: rL LLVM https://reviews.llvm.org/D38643 Files: lib/Index/USRGeneration.cpp test/Index/USR/array-type.cpp Index: test/Index/USR/array-type.cpp ===

[PATCH] D38643: PR13575: Fix USR mangling for fixed-size arrays.

jkorous-apple updated this revision to Diff 118066. jkorous-apple added a comment. Uploaded full diff. https://reviews.llvm.org/D38643 Files: lib/Index/USRGeneration.cpp test/Index/USR/array-type.cpp Index: test/Index/USR/array-type.cpp

[PATCH] D38643: PR13575: Fix USR mangling for fixed-size arrays.

jkorous-apple updated this revision to Diff 118104. jkorous-apple added a comment. Ammenended as suggested. https://reviews.llvm.org/D38643 Files: lib/Index/USRGeneration.cpp test/Index/USR/array-type.cpp Index: test/Index/USR/array-type.cpp ===

[PATCH] D38643: PR13575: Fix USR mangling for fixed-size arrays.

jkorous-apple added a comment. Replied and going to delete the end delimiter. Comment at: lib/Index/USRGeneration.cpp:819 } +if (const ArrayType *const AT = dyn_cast(T)) { + Out << "{"; arphaman wrote: > You can use `const auto *` here. That's wha

[PATCH] D38643: PR13575: Fix USR mangling for fixed-size arrays.

jkorous-apple updated this revision to Diff 118115. jkorous-apple added a comment. Solved issues from CR. https://reviews.llvm.org/D38643 Files: lib/Index/USRGeneration.cpp test/Index/USR/array-type.cpp Index: test/Index/USR/array-type.cpp =

[PATCH] D38643: PR13575: Fix USR mangling for fixed-size arrays.

jkorous-apple updated this revision to Diff 118231. jkorous-apple added a comment. clang-format https://reviews.llvm.org/D38643 Files: lib/Index/USRGeneration.cpp test/Index/USR/array-type.cpp Index: test/Index/USR/array-type.cpp ===

[PATCH] D38643: PR13575: Fix USR mangling for fixed-size arrays.

jkorous-apple updated this revision to Diff 118247. jkorous-apple added a comment. Single char constants don't need to be c-strings. https://reviews.llvm.org/D38643 Files: lib/Index/USRGeneration.cpp test/Index/USR/array-type.cpp Index: test/Index/USR/array-type.cpp ==

[PATCH] D38707: PR13575: Fix USR mangling for functions taking function pointers as arguments.

jkorous-apple created this revision. https://reviews.llvm.org/D38707 Files: lib/Index/USRGeneration.cpp test/Index/USR/func-type.cpp Index: test/Index/USR/func-type.cpp === --- /dev/null +++ test/Index/USR/func-type.cpp @@ -0,0

[PATCH] D38707: PR13575: Fix USR mangling for functions taking function pointers as arguments.

jkorous-apple updated this revision to Diff 118287. jkorous-apple added a comment. added another test https://reviews.llvm.org/D38707 Files: lib/Index/USRGeneration.cpp test/Index/USR/func-type.cpp Index: test/Index/USR/func-type.cpp ===

[PATCH] D38707: PR13575: Fix USR mangling for functions taking function pointers as arguments.

jkorous-apple added inline comments. Comment at: lib/Index/USRGeneration.cpp:757 VisitType(FT->getReturnType()); - for (const auto &I : FT->param_types()) + Out << '('; + for (const auto &I : FT->param_types()) { arphaman wrote: > I believe

[PATCH] D38711: typos in documentation?

jkorous-apple created this revision. I have found two possible typos in documentation. Since English is not my first language I would like to ask for verification. https://reviews.llvm.org/D38711 Files: docs/InternalsManual.rst Index: docs/InternalsManual.rst ==

[PATCH] D38755: Fixed crash during indexing default template template param

jkorous-apple created this revision. fix + testcase https://reviews.llvm.org/D38755 Files: lib/Index/IndexDecl.cpp test/Index/index-template-template-param.cpp Index: test/Index/index-template-template-param.cpp === --- /dev/

[PATCH] D38863: Typos in tutorial

jkorous-apple created this revision. https://reviews.llvm.org/D38863 Files: www/hacking.html Index: www/hacking.html === --- www/hacking.html +++ www/hacking.html @@ -246,9 +246,9 @@ For example: - python C:\Tool\llvm\u

[PATCH] D51485: [Sema][NFC] Trivial cleanup in ActOnCallExpr

jkorous created this revision. jkorous added reviewers: rsmith, doug.gregor, JDevlieghere, thegameg. jkorous added a project: clang. Herald added subscribers: cfe-commits, dexonsmith. Use logical or operator instead of a bool variable and if/else. Repository: rC Clang https://reviews.llvm.org

[PATCH] D51485: [Sema][NFC] Trivial cleanup in ActOnCallExpr

This revision was automatically updated to reflect the committed changes. Closed by commit rL341074: [Sema][NFC] Trivial cleanup in ActOnCallExpr (authored by jkorous, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D51485?vs=163304&id=

[PATCH] D51488: [Sema][NFC] Small cleanup - remove dead code from ActOnCallExpr() ?

jkorous created this revision. jkorous added reviewers: rsmith, vsapsai, JDevlieghere. jkorous added a project: clang. Herald added subscribers: cfe-commits, dexonsmith. Since there's no change to ArgExprs between here and the previous early exit starting at L 5333 it's effectively a dead code.

[PATCH] D51543: [Sema] Fix uninitialized OverloadCandidate::FoundDecl member

jkorous created this revision. jkorous added reviewers: arphaman, vsapsai, dexonsmith, rsmith. jkorous added a project: clang. Herald added subscribers: cfe-commits, mgorny. It's rather error-prone to leave that member variable uninitialized. The DeclAccessPair seems to be intentionally POD and i

[PATCH] D51488: [Sema][NFC] Small cleanup - remove dead code from ActOnCallExpr() ?

jkorous added a comment. Sorry for the delay - I was busy with other things for past two weeks. Comment at: Sema/SemaExpr.cpp:5338 Context.DependentTy, VK_RValue, RParenLoc); } else { JDevlieghere wrote: > While you're at it you might as w

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

jkorous created this revision. jkorous added reviewers: arphaman, vsapsai. jkorous added a project: clang. Herald added subscribers: cfe-commits, dexonsmith. There seem to be couple implicit assumptions that might be better represented explicitly by asserts. Repository: rC Clang https://revi

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

jkorous planned changes to this revision. jkorous added a comment. Hi Volodymyr, Thanks for the feedback - interesting points! I see your point regarding NFC - I am going to drop it as you are right. Two things about sanitizers come to mind: 1. You'd need to guarantee that you have all possibl

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

jkorous added a comment. I tried to come up with some input that breaks current implementation so I could add the test. Problem is that invalid memory read doesn't guarantee deterministic crash. E. g. with this input the current implementation is definitely reading way past the buffer: Sma

[PATCH] D51488: [Sema][NFC] Small cleanup - remove dead code from ActOnCallExpr() ?

jkorous added a comment. I got some test failure with the patch, still investigating the issue. Repository: rC Clang https://reviews.llvm.org/D51488 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/lis

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

jkorous marked 12 inline comments as done. jkorous added inline comments. Comment at: xpc/XPCJSONConversions.cpp:62 + if (objType == XPC_TYPE_UINT64) +return json::Expr(xpc_uint64_get_value(xpcObj)); + if (objType == XPC_TYPE_STRING) sammccall wrote: > hmm,

[PATCH] D48562: [clangd] XPC transport layer

jkorous marked 7 inline comments as done. 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(), NU

[PATCH] D49548: [clangd] XPC WIP

jkorous created this revision. jkorous added reviewers: sammccall, arphaman. Herald added subscribers: cfe-commits, dexonsmith, MaskRay, ioeric, ilya-biryukov, mgorny. Combined, rebased and modified original XPC patches. - https://reviews.llvm.org/D48559 [clangd] refactoring for XPC transport la

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

jkorous added a comment. Sam, just out of curiosity - would it be possible for you to share any relevant experience gained by using porting clangd to protobuf-based transport layer? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48559

[PATCH] D49523: [clangd] Add support for per-file override compilation command

jkorous added a comment. Hi Marc-Andre, what is a structure of data you are passing as parameter of didChangeConfiguration message? All we need is to pass per-file compilation command to clangd. Maybe we could send didChangeConfiguration message right after didOpen. Repository: rCTE Clang T

[PATCH] D49523: [clangd] Add support for per-file override compilation command

jkorous added a comment. Alex, I am just wondering if we shouldn't rather create another implementation of GlobalCompilationDatabase interface (something like InMemoryGlobalCompilationDatabase), add it to ClangdServer and use it as the first place to be searched in ClangdServer::getCompileComma

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

jkorous added a comment. Hi Sam, we discussed a bit more with Alex offline. Ultimately we don't mind using JSON as part of the generic interface but we'd like to optimize specific cases in the future (thinking only about code completion so far) which might need to "work-around" the abstraction.

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

jkorous added a comment. BTW Just for the record - I put the rebased & updated patch here: [clangd] XPC WIP https://reviews.llvm.org/D49548 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48560 ___ cfe-commits mailing list cfe-commi

[PATCH] D48562: [clangd] XPC transport layer

jkorous added a comment. BTW Just for the record - I put the rebased & updated patch here: [clangd] XPC WIP https://reviews.llvm.org/D49548 https://reviews.llvm.org/D48562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.or

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

jkorous added a comment. BTW Just for the record - I put the rebased & updated patch here: [clangd] XPC WIP https://reviews.llvm.org/D49548 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48559 ___ cfe-commits mailing list cfe-commi

[PATCH] D49523: [clangd] Add support for per-file override compilation command

jkorous added a comment. BTW it looks like we already had kind of support for compilation command before (extra flags). commit 5ec1f7ca32eb85077a22ce81d41aa02a017d4852 Author: Krasimir Georgiev Date: Thu Jul 6 08:44:54 2017 + [clangd] Add support for per-file extra flags There is even

[PATCH] D49736: [Basic] Emit warning flag suggestion only in case there's *similar* existing flag for given unknown one

jkorous created this revision. jkorous added reviewers: arphaman, vsapsai, bruno, JDevlieghere. Herald added subscribers: cfe-commits, dexonsmith. In case user specified non-existent warning flag clang was suggesting an existing on that was in some cased very different. This patch should ensure

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

jkorous planned changes to this revision. jkorous added a comment. Hi Sam, we are still discussing internally how to fit clangd and XPC together. Please bear with us and ignore our patches until we decide. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48559

[PATCH] D49736: [Basic] Emit warning flag suggestion only in case there's existing flag *similar* to the unknown one

jkorous added a comment. I like that idea! It looks like it's living in a wrong place anyway as it doesn't need access to any of implementation details (private members) of DiagnosticID. I would still like to preserve it as a function so this block of code has clear semantics and interface. Ho

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

jkorous-apple added inline comments. Comment at: clangd/Protocol.h:190 /// (see exit notification) its process. - llvm::Optional processId; + llvm::Optional processId = 0; Sorry if I am asking stupid question but since the type is Optional<> isn't it's de

[PATCH] D43331: [WIP][Parser][FixIt] Better diagnostics for omitted template keyword (type dependent template names)

jkorous-apple created this revision. What I would love to have is FixIt for this kind of situation: template void foo() { T::bar(); /* missing template keyword */ } which currently gets not that helpful diagnostics: > clang misleading.cpp misleading.cpp:2:15: error: expected '('

[PATCH] D41306: [clangd] Update documentation page with new features, instructions

jkorous-apple accepted this revision. jkorous-apple added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41306 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41897: Fixing a crash in Sema.

jkorous-apple created this revision. jkorous-apple added a reviewer: arphaman. The original code is checking for inaccessible base classes but does not expect inheriting from template parameters (or dependent types in general) as these are not modelled by CXXRecord. Issue was at this line since

[PATCH] D41897: Fixing a crash in Sema.

jkorous-apple updated this revision to Diff 129284. jkorous-apple added a comment. Changes based on Aaron's feedback. https://reviews.llvm.org/D41897 Files: Sema/SemaDeclCXX.cpp SemaCXX/base-class-ambiguity-check.cpp Index: SemaCXX/base-class-ambiguity-check.cpp ==

[PATCH] D42938: [Sema] Emit -Winteger-overflow for arguments in function calls, ObjC messages.

jkorous-apple added a comment. Maybe a stupid idea but in case it makes sense to add these expression types could we also add integer template arguments? https://reviews.llvm.org/D42938 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D42938: [Sema] Emit -Winteger-overflow for arguments in function calls, ObjC messages.

jkorous-apple added a comment. Sorry, my bad, I tried just older clang version which didn't produce any error. With reasonably up to date clang I get these: > cat tmpl_overflow.hpp template struct foo { short a; }; template<> struct foo<100 * 10240> { bool a; };

[PATCH] D44088: [clangd] Extract ClangdServer::Options struct.

jkorous-apple added inline comments. Comment at: clang-tools-extra/trunk/clangd/ClangdServer.h:131 + // Features like indexing must be enabled if desired. + static Options optsForTest(); + Shouldn't this be better implemented as a utility function in tests? Al

[PATCH] D44575: [clangd][nfc] Give name to a magic constant

jkorous-apple created this revision. jkorous-apple added a project: clang-tools-extra. Herald added subscribers: cfe-commits, ioeric, ilya-biryukov. Since I was reading this code I decided I might just as well polish it a little. It is just preliminary commit for a bug-fix. Repository: rCTE C

[PATCH] D44576: [clangd] Fix undefined behavior due to misaligned type cast

jkorous-apple created this revision. jkorous-apple added a project: clang-tools-extra. Herald added subscribers: cfe-commits, ioeric, ilya-biryukov. The current code is casting pointer to a misaligned type which is undefined behavior. Found by compiling with Undefined Behavior Sanitizer and runn

[PATCH] D36918: [Sema] Take into account the current context when checking the accessibility of a member function pointer

jkorous added a comment. I don't particularly like that between setting the DeclContext (SemaTemplateDeduction.cpp:3814) and actually using it (CheckAccess() in SemaAccess.cpp:1459) are some 20 stack frames but it looks like you already tried fixing this "locally" in your initial approach. I a

[PATCH] D36918: [Sema] Take into account the current context when checking the accessibility of a member function pointer

jkorous accepted this revision. jkorous added a comment. LGTM. Thank you for the explanation! Repository: rC Clang https://reviews.llvm.org/D36918 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

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

This revision was automatically updated to reflect the committed changes. Closed by commit rC334062: [Sema] Fix parsing of anonymous union in language linkage specification (authored by jkorous, committed by ). Changed prior to commit: https://reviews.llvm.org/D45884?vs=143736&id=150069#toc Re

[PATCH] D53290: [WIP] clangd Transport layer

jkorous created this revision. jkorous added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, jfb, arphaman, dexonsmith, MaskRay, ioeric, ilya-biryukov, mgorny. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53290 Files: clangd/CMakeLists.txt clangd/Cl

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

jkorous added a comment. Hi Ilya, this seems really useful for people learning how to implement their custom actions! Comment at: clangd/refactor/actions/SwapIfBranches.cpp:36 + + bool VisitIfStmt(IfStmt *If) { +auto R = toHalfOpenFileRange(Ctx.getSourceManager(), Ctx.get

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

jkorous added a comment. Hi Ilya, I got here from reading your other patch (https://reviews.llvm.org/D56611). I'm wondering if we could make those range utility functions more understandable. Comment at: clangd/SourceCode.h:64 +/// Turns a token range into a half-open range

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

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE351280: [clangd] XPC transport layer (authored by jkorous, committed by ). Changed prior to commit: https://reviews.llvm.org/D54428?vs=180809&id=181925#toc Repository: rCTE Clang Tools Extra CHANG

[PATCH] D50452: [WIP] clangd XPC adapter

jkorous added a comment. Abandonned in favor of https://reviews.llvm.org/D54428 Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D50452/new/ https://reviews.llvm.org/D50452 ___ cfe-commits mailing list cfe-

[PATCH] D56561: [Preprocessor] For missing file in framework add note about framework location.

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM with nits about doxygen annotations. Comment at: clang/include/clang/Lex/DirectoryLookup.h:175 + /// \param [out] IsFrameworkFound For a framework directory set to tr

[PATCH] D57057: [clangd] Log clang-tidy configuration, NFC

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57057/new/ https://reviews.llvm.org/D57057 __

[PATCH] D57228: [clangd] Make USRs for macros to be position independent

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. Thanks for fixing this Kadir! LGTM Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57228/new/ https://reviews.llvm.org/D57228 ___

[PATCH] D49736: [Basic] Emit warning flag suggestion only in case there's existing flag *similar* to the unknown one

jkorous requested review of this revision. jkorous added a comment. I tried to move the getNearestOption() to it's only client - EmitUnknownDiagWarning() but it turned out to be a significant change because of clang/Basic/DiagnosticGroups.inc use in DiagnosticIDs.cpp. I suggest we leave that for

[PATCH] D50926: [SourceManager] Extract 'findFileIDsForFile' from 'translateFile' to allow mapping from one file entry to multiple FileIDs

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM and it seems like all of Eric's comments were answered too. Repository: rC Clang https://reviews.llvm.org/D50926 ___ cfe-commits mailin

[PATCH] D50801: [rename] Use isPointWithin when looking for a declaration at location

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM Thanks for fixing this! Repository: rC Clang https://reviews.llvm.org/D50801 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

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

jkorous marked 6 inline comments as done. jkorous added inline comments. Comment at: clangd/CodeCompleteTests.cpp:2043 +TEST(CompletionTestNoExplicitMembers, Struct) { + clangd::CodeCompleteOptions Opts; sammccall wrote: > sammccall wrote: > > (Should this be

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

jkorous updated this revision to Diff 172951. jkorous marked 4 inline comments as done. jkorous added a comment. Rewritten tests to shared implementation different cases. https://reviews.llvm.org/D52554 Files: unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.c

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

jkorous created this revision. jkorous added reviewers: arphaman, sammccall. Herald added subscribers: cfe-commits, kadircet, dexonsmith, MaskRay, ioeric, ilya-biryukov, mgorny. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54428 Files: CMakeLists.txt Features.inc.in clan

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

jkorous created this revision. jkorous added reviewers: sammccall, ilya-biryukov, arphaman, benlangmuir. jkorous added a project: clang-tools-extra. Herald added subscribers: cfe-commits, kadircet, dexonsmith, MaskRay, ioeric. We need a way for given code position to get the best definition/declar

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

jkorous added a comment. One more thing - shall I create new client capability flag for this? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin

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

jkorous added a comment. Hi Sam! I am aware of clangd using SymbolID. We basically need USR for integration with our external indexing service. We don't plan to systematically use it in the protocol and I am not aware of any other requirement for using USR in LSP - will double check that to be

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

jkorous added a comment. We are also discussing creating separate method as we'll likely want to add other data to the response in the future. Would you prefer USR not to be in the standard part of LSP but only in our extension? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D5

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

jkorous added a comment. Correction - I asked offline about our intended use of USR in LSP and it seems we might want to receive it as part of other responses too. Nothing specific for now but it's probable that it won't be just this singular case. Repository: rCTE Clang Tools Extra https:/

[PATCH] D53900: [CodeComplete] Penalize inherited ObjC properties for auto-completion

jkorous added reviewers: arphaman, benlangmuir. jkorous added a comment. This looks reasonable to me but asked people with more context to take a look. Repository: rC Clang https://reviews.llvm.org/D53900 ___ cfe-commits mailing list cfe-commits@

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

jkorous planned changes to this revision. jkorous added a comment. In https://reviews.llvm.org/D54428#1297147, @sammccall wrote: > A question about the high-level build target setup (I don't know much about > XPC services/frameworks, bear with me...): > > This is set up so that the clangd binary

[PATCH] D54047: Check TUScope is valid before use

jkorous added a comment. Could you please add a test? I'd suggest minimizing the testcase you linked and placing it to `clang/test`. Repository: rC Clang https://reviews.llvm.org/D54047 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D49736: [Basic] Emit warning flag suggestion only in case there's existing flag *similar* to the unknown one

jkorous added a comment. Ping. Repository: rC Clang https://reviews.llvm.org/D49736 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53807: Create a diagnostic group for warn_call_to_pure_virtual_member_function_from_ctor_dtor, so it can be turned into an error using Werror

jkorous added a reviewer: jkorous. jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM. Thanks for the patch! Repository: rC Clang https://reviews.llvm.org/D53807 ___ cfe-commits mailing

[PATCH] D54047: Check TUScope is valid before use

jkorous added reviewers: rjmccall, doug.gregor. jkorous added subscribers: rjmccall, doug.gregor. jkorous added a comment. Adding @rjmccall and @doug.gregor who might have some insight. Honestly, I don't know how IWYU works and my familiarity with Sema is limited so bear with me. From what I se

[PATCH] D54693: [clangd] Store source file hash in IndexFile{In,Out}

jkorous added a comment. Hi Kadir, I have one small nit otherwise LGTM. Comment at: clangd/index/Serialization.cpp:368 +Reader Hash(Chunks.lookup("hash")); +llvm::StringRef Digest = Hash.consume(20); +Result.Digest.emplace(); Nit: Maybe we could use

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

jkorous created this revision. jkorous added reviewers: sammccall, arphaman, benlangmuir. Herald added subscribers: cfe-commits, kadircet, dexonsmith, MaskRay, ioeric, ilya-biryukov. Hi, I implemented `textDocument/cursorInfo` method based on consensus in https://reviews.llvm.org/D54529. I'd li

  1   2   3   4   5   >