[PATCH] D67720: [clangd] Add semantic selection to ClangdLSPServer.

hokein added a comment. mostly good, a few nits. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1131 +Callback> Reply) { + if (Params.positions.size() != 1) { +elog("{0} positions provided to SelectionRange. Supports exactly one " usaxena95 w

[PATCH] D66937: [clangd] Fix the stale documentation about background indexing.

hokein updated this revision to Diff 221523. hokein marked 2 inline comments as done. hokein added a comment. address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66937/new/ https://reviews.llvm.org/D66937 Files: clang-tools-extra/doc

[PATCH] D66937: [clangd] Fix the stale documentation about background indexing.

hokein added inline comments. Comment at: clang-tools-extra/docs/clangd/Installation.rst:365 +- background indexing canb be disable by the ``--background-index=false`` flag; + if it is disabled, clangd doesn't have a global view of the whole project, it + only has a view on sym

[PATCH] D67720: [clangd] Add semantic selection to ClangdLSPServer.

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. thanks, looks good. Comment at: clang-tools-extra/clangd/Protocol.cpp:18 #include "llvm/ADT/Hashing.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" --

[PATCH] D67960: [clangd] Move the existing heder-source-switch implemenation out of clangdServer.

hokein created this revision. hokein added a reviewer: kadircet. Herald added subscribers: usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. This is a NFC change. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D67960 Files: clang-tools-extra

[PATCH] D67964: [clangd] Update vscode lsp dependencies to pickup the new changes in LSP v3.15.0.

hokein created this revision. hokein added a reviewer: usaxena95. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. This would enable the newly-added semantic selection feature in vscode. Repository: rG LLVM Github Monorepo https://r

[PATCH] D66937: [clangd] Fix the stale documentation about background indexing.

hokein updated this revision to Diff 221687. hokein marked an inline comment as done. hokein added a comment. Fix a typo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66937/new/ https://reviews.llvm.org/D66937 Files: clang-tools-extra/docs/clan

[PATCH] D66937: [clangd] Fix the stale documentation about background indexing.

This revision was automatically updated to reflect the committed changes. Closed by commit rL372825: [clangd] Fix the stale documentation about background indexing. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154)

hokein added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:181 + addToken(E->getMemberLoc(), E->getQualifier() + ? HighlightingKind::StaticField + : HighlightingKind::F

[PATCH] D67960: [clangd] Move the existing heder-source-switch implemenation out of clangdServer.

hokein updated this revision to Diff 221703. hokein marked 3 inline comments as done. hokein added a comment. Herald added a subscriber: mgorny. address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67960/new/ https://reviews.llvm.org/D67

[PATCH] D67960: [clangd] Move the existing heder-source-switch implemenation out of clangdServer.

This revision was automatically updated to reflect the committed changes. Closed by commit rL372841: [clangd] Move the existing heder-source-switch implemenation out of… (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to co

[PATCH] D67907: [clangd] Implement a smart version of HeaderSource switch.

hokein updated this revision to Diff 221724. hokein added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67907/new/ https://reviews.llvm.org/D67907 Files: clang-tools-extra/clangd/HeaderSourceSwitch.cpp clang-tools-extra/clang

[PATCH] D68020: [clangd] Fix parseNamespaceEvents to parse the last token

hokein accepted this revision. hokein added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp:429 +// Parse until EOF +namespace bar{})cpp", + {""}, ---

[PATCH] D68026: [clangd] Add missing header guard, NFC.

hokein created this revision. hokein added a reviewer: gribozavr. Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. [clang-tidy] Make llvm-header-guard work on llvm git monorepo. Repository: rG LLVM Github Monorepo https:

[PATCH] D68026: [clangd] Add missing header guard, NFC.

hokein updated this revision to Diff 221765. hokein added a comment. upload correct diff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68026/new/ https://reviews.llvm.org/D68026 Files: clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.cpp cl

[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

hokein updated this revision to Diff 221903. hokein added a comment. Rewrite the getBeignningOfIdentifier function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67695/new/ https://reviews.llvm.org/D67695 Files: clang-tools-extra/clangd/SourceCo

[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

hokein updated this revision to Diff 221904. hokein added a comment. Fix a typo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67695/new/ https://reviews.llvm.org/D67695 Files: clang-tools-extra/clangd/SourceCode.cpp clang-tools-extra/clangd/u

[PATCH] D68026: [clang-tidy] Make llvm-header-guard work on llvm git monorepo

This revision was automatically updated to reflect the committed changes. Closed by commit rL372953: [clang-tidy] Make llvm-header-guard work on llvm git monorepo (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D68077: [clangd][vscode] Turn on the semantic highlighting by default.

hokein created this revision. hokein added a reviewer: ilya-biryukov. Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. We have turned on the flag manually and used it for a while, and don't see any major issues, let's enable it by default.

[PATCH] D68077: [clangd][vscode] Turn on the semantic highlighting by default.

This revision was automatically updated to reflect the committed changes. Closed by commit rL372969: [clangd][vscode] Turn on the semantic highlighting by default. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D67964: [clangd] Update vscode lsp dependencies to pickup the new changes in LSP v3.15.0.

This revision was automatically updated to reflect the committed changes. Closed by commit rL372970: [clangd][vscode] Update vscode lsp dependencies to pickup the new changes in… (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed pr

[PATCH] D68080: [clangd][vscode] Add npm helper commands to package/release the extension.

hokein created this revision. hokein added a reviewer: ilya-biryukov. Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. Help to fix https://github.com/clangd/clangd/issues/159. Repository: rG LLVM Github Monorepo https://reviews.llvm.org

[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

hokein updated this revision to Diff 221935. hokein marked 2 inline comments as done. hokein added a comment. Simplify the logic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67695/new/ https://reviews.llvm.org/D67695 Files: clang-tools-extra/c

[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

hokein updated this revision to Diff 221936. hokein added a comment. Upload correct diff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67695/new/ https://reviews.llvm.org/D67695 Files: clang-tools-extra/clangd/SourceCode.cpp clang-tools-extra

[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

hokein added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.cpp:286 - Before = Lexer::GetBeginningOfToken(Before, SM, LangOpts); - Token Tok; - if (Before.isValid() && - !Lexer::getRawToken(Before, Tok, SM, LangOpts, false) && - Tok.is(tok::raw_id

[PATCH] D68080: [clangd][vscode] Add npm helper commands to package/release the extension.

hokein marked an inline comment as done. hokein added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/package.json:38 +"package": "vsce package --baseImagesUrl https://raw.githubusercontent.com/llvm/llvm-project/master/clang-tools-extra/clangd

[PATCH] D68080: [clangd][vscode] Add npm helper commands to package/release the extension.

hokein marked an inline comment as done. hokein added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/package.json:38 +"package": "vsce package --baseImagesUrl https://raw.githubusercontent.com/llvm/llvm-project/master/clang-tools-extra/clangd

[PATCH] D68080: [clangd][vscode] Add npm helper commands to package/release the extension.

This revision was automatically updated to reflect the committed changes. Closed by commit rL372980: [clangd][vscode] Add npm helper commands to package/release the extension. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior

[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

hokein added a comment. In D66647#1684046 , @ilya-biryukov wrote: > We also need to rename parameters sometimes, right? > > // Sometimes we need to rename parameters. > void usages(int decl_param, int); > > void usages(int def_param, int now_named

[PATCH] D67907: [clangd] Implement a smart version of HeaderSource switch.

hokein updated this revision to Diff 222111. hokein marked 9 inline comments as done. hokein added a comment. address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67907/new/ https://reviews.llvm.org/D67907 Files: clang-tools-ex

[PATCH] D67907: [clangd] Implement a smart version of HeaderSource switch.

hokein updated this revision to Diff 222112. hokein added a comment. format the testcases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67907/new/ https://reviews.llvm.org/D67907 Files: clang-tools-extra/clangd/HeaderSourceSwitch.cpp clang-to

[PATCH] D67907: [clangd] Implement a smart version of HeaderSource switch.

hokein added inline comments. Comment at: clang-tools-extra/clangd/HeaderSourceSwitch.cpp:86 + auto AwardTarget = [&](const char *TargetURI) { +if (auto TargetPath = URI::resolve(TargetURI, OriginalFile)) { + if (*TargetPath != OriginalFile) // exclude the original file

[PATCH] D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154)

hokein added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:181 + addToken(E->getMemberLoc(), E->getQualifier() + ? HighlightingKind::StaticField + : HighlightingKind::F

[PATCH] D68024: [clangd] Implement GetEligiblePoints

hokein added a comment. The implementation looks good, just have some questions on the API. Comment at: clang-tools-extra/clangd/SourceCode.cpp:997 + // definitions as well. One might use a closing parantheses(")" followed by an + // opening brace "{" to trigger the start. +

[PATCH] D67907: [clangd] Implement a smart version of HeaderSource switch.

hokein updated this revision to Diff 222360. hokein marked 3 inline comments as done. hokein added a comment. address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67907/new/ https://reviews.llvm.org/D67907 Files: clang-tools-ex

[PATCH] D67907: [clangd] Implement a smart version of HeaderSource switch.

hokein added inline comments. Comment at: clang-tools-extra/clangd/HeaderSourceSwitch.cpp:113 + auto Best = Candidates.begin(); + for (auto It = Candidates.begin(); It != Candidates.end(); ++It) { +if (It->second > Best->second) kadircet wrote: > nit: `for(

[PATCH] D67907: [clangd] Implement a smart version of HeaderSource switch.

This revision was automatically updated to reflect the committed changes. Closed by commit rL373197: [clangd] Implement a smart version of HeaderSource switch. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: ht

[PATCH] D68211: [clangd] Use the index-based API to do the header-source switch.

hokein created this revision. hokein added a reviewer: kadircet. Herald added subscribers: usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. If the file heuristic fails, we try to use the index&AST to do the header/source inference. Repository: rG LLVM Github

[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

hokein updated this revision to Diff 222423. hokein marked 8 inline comments as done. hokein added a comment. address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67695/new/ https://reviews.llvm.org/D67695 Files: clang-tools-extra/cla

[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

hokein added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.cpp:242 + +enum TokenKind { Identifier, Operator, Whitespace, Other }; + ilya-biryukov wrote: > `TokenKind` has the same name as `tok::TokenKind`. Could we use a different > name here

[PATCH] D68024: [clangd] Implement GetEligiblePoints

hokein added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.h:270 + /// It will be “a::b” for both carrot locations. + std::string CurrentNamespace; + /// Offsets into the code marking eligible points to insert a function kadircet wrote: > ho

[PATCH] D68211: [clangd] Use the index-based API to do the header-source switch.

hokein updated this revision to Diff 222432. hokein marked 4 inline comments as done. hokein added a comment. address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68211/new/ https://reviews.llvm.org/D68211 Files: clang-tools-extra/cla

[PATCH] D68211: [clangd] Use the index-based API to do the header-source switch.

hokein updated this revision to Diff 222574. hokein marked 3 inline comments as done. hokein added a comment. address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68211/new/ https://reviews.llvm.org/D68211 Files: clang-tools-extra/cla

[PATCH] D68211: [clangd] Use the index-based API to do the header-source switch.

This revision was automatically updated to reflect the committed changes. Closed by commit rL373320: [clangd] Use the index-based API to do the header-source switch. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit

[PATCH] D68024: [clangd] Implement GetEligiblePoints

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. looks good. Comment at: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp:610 + /*FullyQualifiedName=*/const char *, + /*Curr

[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

hokein updated this revision to Diff 222583. hokein marked 2 inline comments as done. hokein added a comment. add one more testcase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67695/new/ https://reviews.llvm.org/D67695 Files: clang-tools-extr

[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

hokein added inline comments. Comment at: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp:330 + "/^/ comments", // non-interesting token + "void f(int abc) { abc ^ ++; }",// whitespace + "void f(int abc) { ^abc^++; }", // range of iden

[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

This revision was automatically updated to reflect the committed changes. Closed by commit rL373323: [clangd] Implement getBeginning for overloaded operators. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: htt

[PATCH] D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154)

hokein added a comment. In D67901#1687308 , @nridge wrote: > I do feel strongly that types and non-types should be highlighted > differently, so the updated patch adds two new dependent highlightings, one > for types and one for variables/functions. I

[PATCH] D68322: [clang-rename] Better renaming the typedef decl.

hokein created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. when renaming a typedef decl, we used to rename the underlying decl of the typedef, we should rename the typedef itself. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D68322 Fi

[PATCH] D68325: [clangd] Bail out early if we are sure that the symbol is used outside of the file.

hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. This would reduce the false positive when the static index is in an unavailable state, e.g. background index i

[PATCH] D68322: [clang-rename] Better renaming the typedef decl.

This revision was automatically updated to reflect the committed changes. Closed by commit rL373440: [clang-rename] Better renaming the typedef decl. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://revi

[PATCH] D67536: [WIP] [clangd] Add support for an inactive regions notification

hokein added a comment. Could you add tests for this? Comment at: clang-tools-extra/clangd/Compiler.cpp:66 CI->getLangOpts()->CommentOpts.ParseAllComments = true; + CI->getPreprocessorOpts().DetailedRecord = true; return CI; I'm not sure how does this fl

[PATCH] D68325: [clangd] Bail out early if we are sure that the symbol is used outside of the file.

This revision was automatically updated to reflect the committed changes. Closed by commit rL373444: [clangd] Bail out early if we are sure that the symbol is used outside of the… (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed p

[PATCH] D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154)

hokein added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:505 + case HighlightingKind::DependentType: +return "entity.name.type.dependent.cpp"; + case HighlightingKind::DependentName: ilya-biryukov wrote: > Maybe have a sep

[PATCH] D45513: [clangd] Add line and column number to the index symbol.

hokein updated this revision to Diff 142170. hokein marked 5 inline comments as done. hokein added a comment. Address review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45513 Files: clangd/index/Index.cpp clangd/index/Index.h clangd/index/SymbolCollector.cpp

[PATCH] D45513: [clangd] Add line and column number to the index symbol.

hokein added inline comments. Comment at: clangd/index/Index.h:39 // using half-open range, [StartOffset, EndOffset). + // FIXME(hokein): remove these fields in favor of Position. unsigned StartOffset = 0; sammccall wrote: > I don't think we should remove

[PATCH] D45513: [clangd] Add line and column number to the index symbol.

hokein added inline comments. Comment at: clangd/index/Index.h:32 +// Character offset on a line in a document (zero-based). +int Character = 0; + }; sammccall wrote: > sammccall wrote: > > Column? > > > > LSP calls this "character" but this is nonstand

[PATCH] D45513: [clangd] Add line and column number to the index symbol.

hokein updated this revision to Diff 142195. hokein marked 3 inline comments as done. hokein added a comment. Update the patch, address remaining issues. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45513 Files: clangd/index/Index.cpp clangd/index/Index.h clangd/index/S

[PATCH] D44882: [clangd] Implementation of workspace/symbol request

hokein added a comment. @malaperle, what's your plan of this patch? Are you going to land it before https://reviews.llvm.org/D45513? With the Line&Column info in the index, this patch could be simplified. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44882

[PATCH] D45513: [clangd] Add line and column number to the index symbol.

hokein added inline comments. Comment at: clangd/index/Index.h:32 +// Character offset on a line in a document (zero-based). +int Character = 0; + }; MaskRay wrote: > hokein wrote: > > sammccall wrote: > > > sammccall wrote: > > > > Column? > > > > > >

[PATCH] D45513: [clangd] Add line and column number to the index symbol.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE329997: [clangd] Add line and column number to the index symbol. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D45513?vs=142195&id=142349#toc Repository: rC

[PATCH] D44882: [clangd] Implementation of workspace/symbol request

hokein added a comment. In https://reviews.llvm.org/D44882#1065932, @malaperle wrote: > In https://reviews.llvm.org/D44882#1065787, @hokein wrote: > > > @malaperle, what's your plan of this patch? Are you going to land it before > > https://reviews.llvm.org/D45513? With the Line&Column info in t

[PATCH] D45692: [clangd] Fix "fail to create file URI" warnings in FileIndexTest.

hokein created this revision. hokein added a reviewer: ioeric. Herald added subscribers: MaskRay, jkorous-apple, ilya-biryukov, klimek. When running the FileIndexTest, it shows "Failed to create an URI for file XXX: not a valid absolute file path" warnings, and the generated Symbols is missing Loc

[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.

hokein created this revision. hokein added a reviewer: alexfh. Herald added subscribers: xazax.hun, klimek. Fix https://bugs.llvm.org/show_bug.cgi?id=34900. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45697 Files: clang-tidy/ClangTidyOptions.cpp clang-tidy/ClangTidyOptio

[PATCH] D45717: [clangd] Using index for GoToDefinition.

hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: MaskRay, ioeric, jkorous-apple, ilya-biryukov, klimek. This patch adds index support for GoToDefinition -- when we don't get the definition from local AST, we query our index (Static&Dynamic) index to get i

[PATCH] D45692: [clangd] Fix "fail to create file URI" warnings in FileIndexTest.

This revision was automatically updated to reflect the committed changes. Closed by commit rL330182: [clangd] Fix "fail to create file URI" warnings in FileIndexTest. (authored by hokein, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D456

[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.

hokein added inline comments. Comment at: test/clang-tidy/read_file_config.cpp:1 +// RUN: mkdir -p %T/read-file-config/ +// RUN: cp %s %T/read-file-config/test.cpp JonasToth wrote: > Will this work on windows? I believe it works on windows (although I don't have

[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.

hokein updated this revision to Diff 142759. hokein marked an inline comment as done. hokein added a comment. Add one more test to ensure the test code triggering "clang-analyzer-*" checks. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45697 Files: clang-tidy/ClangTidyOption

[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.

hokein added inline comments. Comment at: test/clang-tidy/read_file_config.cpp:5 +// RUN: echo '[{"command": "cc -c -o test.o test.cpp", "directory": "%T/read-file-config", "file": "%T/read-file-config/test.cpp"}]' > %T/read-file-config/compile_commands.json +// RUN: clang-tidy

[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.

hokein updated this revision to Diff 142817. hokein marked an inline comment as done. hokein added a comment. Make grep pattern more obvious. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45697 Files: clang-tidy/ClangTidyOptions.cpp clang-tidy/ClangTidyOptions.h clang-ti

[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.

This revision was automatically updated to reflect the committed changes. Closed by commit rL330245: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file. (authored by hokein, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.or

[PATCH] D44882: [clangd] Implementation of workspace/symbol request

hokein added inline comments. Herald added a subscriber: jkorous. Comment at: clangd/FindSymbols.cpp:117 +} +auto Path = URI::resolve(*Uri); +if (!Path) { The current URI scheme (`file`, `test`) works fine without `HintPath` because they don't use it

[PATCH] D45717: [clangd] Using index for GoToDefinition.

hokein updated this revision to Diff 143508. hokein marked 7 inline comments as done. hokein added a comment. Refine the patch and address all review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45717 Files: clangd/ClangdServer.cpp clangd/XRefs.cpp clangd/XRef

[PATCH] D45717: [clangd] Using index for GoToDefinition.

hokein added a comment. I have updated the patch according to offline discussion -- for each symbol, we will return both decl and def locations (if available, def first) as they seems to be most sensible to users. We always prefer location from AST over Index when conflicts.

[PATCH] D45717: [clangd] Using index for GoToDefinition.

hokein updated this revision to Diff 143728. hokein marked 14 inline comments as done. hokein added a comment. Address review comments: - clarify the flow of go to definition. - simplify the test code. - some function move-out stuff. Repository: rCTE Clang Tools Extra https://reviews.llvm.or

[PATCH] D45717: [clangd] Using index for GoToDefinition.

hokein added a comment. Thanks for the useful comments! I refined the patch, and it becomes a bit larger (including some moving stuff). Comment at: clangd/XRefs.cpp:137 + +IdentifiedSymbol getSymbolAtPosition(ParsedAST &AST, SourceLocation Pos) { + auto DeclMacrosFinder = std

[PATCH] D46035: [clangd] Fix unicode handling, using UTF-16 where LSP requires it.

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. Cool, the code looks good to me (just a few nits), thanks for the descriptive comments! > This seems likely to cause problems with editors that have the same bug, and > treat the protocol as

[PATCH] D46065: [clangd] Add "str()" method to SymbolID.

hokein created this revision. hokein added a reviewer: ioeric. Herald added subscribers: jkorous, MaskRay, ilya-biryukov, klimek. This is a convenient function when we try to get std::string of SymbolID. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46065 Files: clangd/globa

[PATCH] D46065: [clangd] Add "str()" method to SymbolID.

hokein marked an inline comment as done. hokein added inline comments. Comment at: clangd/index/Index.h:72 + // Returns a 40-bytes hex encoded string. + std::string str() const; ioeric wrote: > I think Sam wants to reduce the size to 20 bytes. Maybe just drop

[PATCH] D46065: [clangd] Add "str()" method to SymbolID.

This revision was automatically updated to reflect the committed changes. hokein marked an inline comment as done. Closed by commit rL330835: [clangd] Add "str()" method to SymbolID. (authored by hokein, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://revie

[PATCH] D46258: [clangd] Also use UTF-16 in index position.

hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: jkorous, MaskRay, ioeric, ilya-biryukov, klimek. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46258 Files: clangd/index/SymbolCollector.cpp Index: clangd/index/SymbolCollector.cpp ==

[PATCH] D46258: [clangd] Also use UTF-16 in index position.

hokein updated this revision to Diff 144540. hokein added a comment. Add a test. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46258 Files: clangd/index/SymbolCollector.cpp unittests/clangd/SymbolCollectorTests.cpp Index: unittests/clangd/SymbolCollectorTests.cpp ===

[PATCH] D46258: [clangd] Also use UTF-16 in index position.

This revision was automatically updated to reflect the committed changes. Closed by commit rL331168: [clangd] Also use UTF-16 in index position. (authored by hokein, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D46258 Files: clang-too

[PATCH] D45717: [clangd] Using index for GoToDefinition.

hokein updated this revision to Diff 144561. hokein marked 7 inline comments as done. hokein added a comment. Address review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45717 Files: clangd/ClangdServer.cpp clangd/XRefs.cpp clangd/XRefs.h clangd/index/FileIn

[PATCH] D45717: [clangd] Using index for GoToDefinition.

hokein added a comment. Thanks for the review! Comment at: clangd/XRefs.cpp:277 +// it. +auto ToLSPLocation = [&HintPath]( +const SymbolLocation &Loc) -> llvm::Optional { sammccall wrote: > hokein

[PATCH] D45717: [clangd] Using index for GoToDefinition.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE331189: [clangd] Using index for GoToDefinition. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D45717?vs=144561&id=144566#toc Repository: rCTE Clang Tools E

[PATCH] D48368: [clangd] Sema ranking tweaks: downrank keywords and injected names.

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. The code looks good to me. It'd be nicer if you can put ranking screenshots (before vs after) in the patch summary. Comment at: clangd/Quality.cpp:160 +case Keyword: /

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

hokein added inline comments. Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:191 +void templated_thrower() { throw T{}(); } +// CHECK-MESSAGES: [[@LINE-1]]:34: warning: throwing an exception whose type 'int' is not derived from 'std::exception' + I th

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

hokein added a comment. > Could we try to figure out why the duplicates were there in the first place > and why the paths were different? +1, I think there are two issues here: 1. the result contains two candidates, which should be one, IIUC. 2. the absolute file path problem, we encountered si

[PATCH] D48854: Use ExprMutationAnalyzer in performance-for-range-copy

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

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

hokein added a comment. After taking a look closely, I figured why there are two candidates here -- one is from AST (the one with ".." path); the other one is from dynamic index, the deduplication failed because of the different paths :( I think the fixing way is to normalize the file path from

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

hokein added subscribers: bkramer, hokein. hokein added a comment. I'm not familiar with this part of code, but the change looks fine to me. I think @bkramer is the right person to review it. Please make sure the style align with LLVM code style. Comment at: lib/Basic/Virtual

[PATCH] D48951: [clang-move] ClangMoveTests: Remove dots in output paths

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. Looks good. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48951 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

hokein added a comment. Seems to me you have a few comments unaddressed (and make sure you marked them done when updating the patch). Repository: rC Clang https://reviews.llvm.org/D48903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D49008: [clangd] Upgrade logging facilities with levels and formatv.

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. Nice, look good. Comment at: clangd/tool/ClangdMain.cpp:104 +llvm::cl::values(clEnumVal(Logger::Error, "Error messages only"), + clEnumVal(Logger::Inf

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

hokein added a comment. Herald added a subscriber: omtcyfz. The code looks good. I'll let Ben take a final look. Comment at: lib/Basic/VirtualFileSystem.cpp:485 + /// \p Status::Name in the return value, to mimic the behavior of \p RealFile. + Status getStatus(std::string Re

[PATCH] D49008: [clangd] Upgrade logging facilities with levels and formatv.

hokein added a comment. still LGTM. The diff contains unrelated changes now. Comment at: clangd/tool/ClangdMain.cpp:104 +llvm::cl::values(clEnumVal(Logger::Error, "Error messages only"), + clEnumVal(Logger::Info, "High level execution tracing"), +

[PATCH] D49190: [clang-tidy/ObjC] Add SQL to list of acronyms

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

[PATCH] D49224: [clangd] log request/response messages with method/ID/error at INFO level

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clangd/JSONRPCDispatcher.cpp:138 }); + auto ID = 1; + log("--> {0}({1})", Method, ID); nit I'd suggest putting this statement immediat

<    8   9   10   11   12   13   14   15   16   17   >