[PATCH] D66578: Remove \brief commands from doxygen comments.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Kill it with fire Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66578/new/ https://reviews.llvm.org/D66578

[PATCH] D66530: [clangd] Don't collect locations that implicitly refer to the target decl.

ilya-biryukov added a comment. This actually seems useful. If one starts the search at `operator bool`, it's nice to see its implicit calls too. What are the cons of having this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66530/new/

[PATCH] D66516: [clangd] Added highlighting to types dependant on templates.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:177 return; +if (TP->isPointerType() || TP->isLValueReferenceType()) + // When highlighting dependant template types the type can be a pointer or

[PATCH] D66478: [clangd] Ignore implicit conversion-operator nodes in find refs.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66478/new/ https://reviews.llvm.org/D66478

[PATCH] D66516: [clangd] Added highlighting to types dependant on templates.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:177 return; +if (TP->isPointerType() || TP->isLValueReferenceType()) + // When highlighting dependant template types the type can be a pointer or

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66349/new/ https://reviews.llvm.org/D66349

[PATCH] D66470: [Syntax] Added function to get macro expansion tokens to TokenBuffer.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:239 + /// Get all tokens that expand a macro in FID. For the following input + /// #define

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:389 llvm::sort(References, [](const Reference , const Reference ) { - return std::tie(L.Loc, L.CanonicalTarget, L.Role) < - std::tie(R.Loc, R.CanonicalTarget, R.Role); +

[PATCH] D66226: [clangd] Skip function bodies inside processed files while indexing

ilya-biryukov updated this revision to Diff 216073. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Change /// to // Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66226/new/ https://reviews.llvm.org/D66226 Files:

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:389 llvm::sort(References, [](const Reference , const Reference ) { - return std::tie(L.Loc, L.CanonicalTarget, L.Role) < - std::tie(R.Loc, R.CanonicalTarget, R.Role); +

[PATCH] D66226: [clangd] Skip function bodies inside processed files while indexing

ilya-biryukov marked 2 inline comments as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:745 +bool SymbolCollector::shouldProcessFile(FileID FID) { + assert(ASTCtx); kadircet wrote: > we already have

[PATCH] D66226: [clangd] Skip function bodies inside processed files while indexing

ilya-biryukov updated this revision to Diff 216066. ilya-biryukov added a comment. - Expose the existing helper instead of introducing a new one Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66226/new/ https://reviews.llvm.org/D66226 Files:

[PATCH] D64741: [clangd] Added highlighting for tokens that are macro arguments.

ilya-biryukov accepted this revision. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM from my side (and a few NIT, but up to you whether to apply them) Comment at:

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:389 llvm::sort(References, [](const Reference , const Reference ) { - return std::tie(L.Loc, L.CanonicalTarget, L.Role) < - std::tie(R.Loc, R.CanonicalTarget, R.Role); +

[PATCH] D64741: [clangd] Added highlighting for tokens that are macro arguments.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:332 + #define DEF_CLASS(T) class T {}; + DEF_MULTIPLE(XYZ); + DEF_MULTIPLE(XYZW); Unrelated to the change: do we plan to highlight

[PATCH] D64741: [clangd] Added highlighting for tokens that are macro arguments.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:54 + if (Conflicting.size() > 1) { +Tokens.erase(Tokens.begin() + I, + Tokens.begin() + I + Conflicting.size()); This is

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:389 llvm::sort(References, [](const Reference , const Reference ) { - return std::tie(L.Loc, L.CanonicalTarget, L.Role) < - std::tie(R.Loc, R.CanonicalTarget, R.Role); +

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:389 llvm::sort(References, [](const Reference , const Reference ) { - return std::tie(L.Loc, L.CanonicalTarget, L.Role) < - std::tie(R.Loc, R.CanonicalTarget, R.Role); +

[PATCH] D66343: [clangd] Simplify code of ClangdLSPServer::onCommand

ilya-biryukov created this revision. ilya-biryukov added a reviewer: hokein. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. By inlining a complicated lambda into its single call-site. Also ensure we call Reply() exactly once even if tweaks return

[PATCH] D66335: [clangd] Added special HighlightingKind for function parameters.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66335/new/ https://reviews.llvm.org/D66335

[PATCH] D66221: [clangd] Added highlighting for non type templates.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66221/new/ https://reviews.llvm.org/D66221

[PATCH] D66221: [clangd] Added highlighting for non type templates.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:231 + if (TP->isFunctionPointerType()) { +addToken(Loc, HighlightingKind::Function); +return; jvikstrom wrote: > ilya-biryukov

[PATCH] D66221: [clangd] Added highlighting for non type templates.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:231 + if (TP->isFunctionPointerType()) { +addToken(Loc, HighlightingKind::Function); +return; Why do we special-case template

[PATCH] D66226: [clangd] Skip function bodies inside processed files while indexing

ilya-biryukov created this revision. ilya-biryukov added reviewers: kadircet, sammccall. Herald added subscribers: arphaman, jkorous, MaskRay. Herald added a project: clang. This significantly improves performance of background indexing. We do not collect references and declarations inside the

[PATCH] D66215: [clangd] Print qualifiers of out-of-line definitions in document outline

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:106 + +/// Returns a nested name specifier if \p ND refers to a an out-of-line +/// definition. hokein wrote: > `if .. out-of-line definition` seems a bit confusing, I think here

[PATCH] D66211: [clangd][vscode] Surface the error when applying tweaks fails

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Thanks for providing examples of error messages, also agree they look fine. If we find bad error messages later, we could revisit the presentation strategy. LGTM

[PATCH] D66215: [clangd] Print qualifiers of out-of-line definitions in document outline

ilya-biryukov updated this revision to Diff 215090. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Rename the helper function - Remove mention of 'out-of-line definition' from a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D66211: [clangd][vscode] Surface the error when applying tweaks fails

ilya-biryukov added a comment. Surfacing errors to the users in those cases is definitely something we need to do, thanks! How do they look in practice? In particular, should we add more context information (either in clangd or in the VSCode extension) to those errors, now that we know they

[PATCH] D66215: [clangd] Print qualifiers of out-of-line definitions in document outline

ilya-biryukov created this revision. ilya-biryukov added a reviewer: hokein. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. To improve the UX around navigating and searching through the results. Repository: rG LLVM Github Monorepo

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

ilya-biryukov added a comment. I did some experiments with adding something similar to the `ErrorExpr` Aaron suggest. It has this flag set and does not get removed from the AST. I believe the best way to address Aaron's comments is to add tests mentioning this expression instead of trying to

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

ilya-biryukov added a comment. In D65591#1625744 , @aaron.ballman wrote: > The problem is: those bits are not infinite and we only have a handful left > until bumping the allocation size; is this use case critical enough to use > one of those bits? I

[PATCH] D66087: [clangd] Preserve line break when rendering text chunks of markdown

ilya-biryukov added a comment. The heuristics for properly rendering the comments are probably a good way to go. I'd say this change is still a step in the right direction - text blocks in formatted strings should be properly escaped to avoid being interpreted like markdown constructs. Any

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

ilya-biryukov added a comment. In D65591#1625183 , @aaron.ballman wrote: > In D65591#1625154 , @riccibruno > wrote: > > > It seems that these two options are not exactly the same right ? The > > `ContainsError`

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

ilya-biryukov added a comment. In D65591#1625154 , @riccibruno wrote: > It seems that these two options are not exactly the same right ? The > `ContainsError` bit is useful to quickly answer "Does this expression > contains an invalid sub-expression"

[PATCH] D66087: [clangd] Preserve line break when rendering text chunks of markdown

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. Fixes https://github.com/clangd/clangd/issues/95 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D66087

[PATCH] D66086: [clangd] Separate chunks with a space when rendering markdown

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. This results in better rendering of resulting markdown. Especially noticeable in coc.nvim that does not have a visible

[PATCH] D66083: [clangd] Remove highlightings coming from non topLevelDecls from included files.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Great example for the test case! It will definitely stay valid even if we fix all problems caused by RecursiveASTVisitor! Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

ilya-biryukov added a comment. Gentle ping. @rsmith, please take a look when you have time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65591/new/ https://reviews.llvm.org/D65591 ___ cfe-commits

[PATCH] D65752: [Sema] Refactor LookupVisibleDecls. NFC

ilya-biryukov added a subscriber: doug.gregor. ilya-biryukov added a comment. In D65752#1623914 , @sammccall wrote: > This looks reasonable to me, there are a couple of variations you might think > about: > > - also treat QualifiedNameLookup as an

[PATCH] D66074: [clangd] Drop diags from non-written #include.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM, thanks! We should also merge this into the release branch if it's not too late yet. Comment at:

[PATCH] D65373: [clangd] Update features table in the docs with links to LSP extension proposals

ilya-biryukov added inline comments. Comment at: clang-tools-extra/docs/clangd/Features.rst:260 +-++--+ -| Syntax and Semantic Coloring| No | No | +| Syntax and Semantic Coloring|`Proposed`__|

[PATCH] D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM, but please print the function template arguments uniformly before landing this (to avoid different forms of outputs inside the same test). Repository: rG LLVM Github

[PATCH] D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:85 +if (const auto *Args = FD->getTemplateSpecializationArgs()) { + std::string SpecializationArgs; + // Without the PrintingPolicy "bool" will be printed as

[PATCH] D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:144 + EXPECT_THAT(AST.getLocalTopLevelDecls(), + ElementsAre(DeclNamed("f"), DeclNamed("f"), DeclNamed("f"), + DeclNamed("V"),

[PATCH] D65849: [unittests] Mark private gmock headers with IWYU pragmas. NFC

ilya-biryukov created this revision. ilya-biryukov added a reviewer: gribozavr. Herald added subscribers: llvm-commits, kadircet. Herald added a project: LLVM. To prevent clangd from adding #include of those headers. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D65849 Files:

[PATCH] D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls.

ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. One important comment about somehow distinguishing multiple decls with the same name. Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:110 +template +void f(T) {} +

[PATCH] D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/AST.h:86 +bool isImplicitTemplateInstantiation(const NamedDecl *D); +bool isExplicitTemplateSpecialization(const NamedDecl *D); Could you add a small comment with an example? ``` ///

[PATCH] D64907: [AST] Traverse attributes inside DEF_TRAVERSE_DECL macro

ilya-biryukov updated this revision to Diff 213558. ilya-biryukov added a comment. - Add the missing newline Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64907/new/ https://reviews.llvm.org/D64907 Files:

[PATCH] D65796: [clangd] Compute scopes eagerly in IncludeFixer

ilya-biryukov updated this revision to Diff 213557. ilya-biryukov added a comment. - Make sure the test actually runs IncludeFixer.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65796/new/ https://reviews.llvm.org/D65796 Files:

[PATCH] D65796: [clangd] Compute scopes eagerly in IncludeFixer

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. Computing lazily leads to crashes. In particular, computing scopes may produce diagnostics (from inside template

[PATCH] D65752: [Sema] Refactor LookupVisibleDecls. NFC

ilya-biryukov added a comment. This is just a proposal, there are probably other ways to reach better readability, e.g. group some of those parameters into a struct. But let me know what you think, happy to refactor in a slightly different manner or simply drop this revision. Repository: rG

[PATCH] D65752: [Sema] Refactor LookupVisibleDecls. NFC

ilya-biryukov updated this revision to Diff 213371. ilya-biryukov added a comment. - Remove accidental change from revision Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65752/new/ https://reviews.llvm.org/D65752 Files:

[PATCH] D65752: [Sema] Refactor LookupVisibleDecls. NFC

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous. Herald added a project: clang. We accumulated some configuration parameters for LookupVisibleDecls that are being passed unchanged to recursive calls, e.g.

[PATCH] D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:68 + if (const auto *TD = dyn_cast(D)) +return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation; + return false; ilya-biryukov wrote: > jvikstrom

[PATCH] D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:68 + if (const auto *TD = dyn_cast(D)) +return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation; + return false; jvikstrom wrote: > ilya-biryukov

[PATCH] D65735: [AST] Fix RecursiveASTVisitor visiting implicit constructor initializers.

ilya-biryukov added a comment. LGTM from my side, a few optional NITs. Feel free to land as soon as @hokein stamps. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/ImplicitCtorInitializer.cpp:16 + +// Check to ensure that CXXCtorInitializer is not visited when

[PATCH] D65735: [AST] Fix RecursiveASTVisitor visiting implicit constructor initializers.

ilya-biryukov added a comment. The fix looks good. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp:17 // syntactic and semantic form. class InitListExprPreOrderVisitor : public ExpectedLocationVisitor { Could you

[PATCH] D65517: [Preprocessor] Always discard body of #define if we failed to parse it

ilya-biryukov added a comment. Thanks! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65517/new/ https://reviews.llvm.org/D65517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D65655: [clangd] Fix a crash when presenting values for Hover

ilya-biryukov added a subscriber: hans. ilya-biryukov added a comment. @hans, could we merge this commit into the release branch? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65655/new/ https://reviews.llvm.org/D65655

[PATCH] D65655: [clangd] Fix a crash when presenting values for Hover

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. We should pass the expression type, not a variable type when printing the resulting value. Variable type may be different

[PATCH] D65525: [clangd] Add new helpers to make tweak tests scale better. Convert most tests. NFC

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65525/new/ https://reviews.llvm.org/D65525

[PATCH] D65625: [clangd] Allow the client to specify multiple compilation databases in the 'initialize' request

ilya-biryukov added a subscriber: sammccall. ilya-biryukov added a comment. Could this be handled outside clangd? If not, what are the complications? We currently have a capability to specify a path to a single `compile_commands.json`. I would expect clients using this extension (Theia?) also

[PATCH] D65525: [clangd] Add new helpers to make tweak tests scale better. Convert most tests. NFC

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/TweakTesting.h:44 +// Snippet is an expression. +Expression, + }; ilya-biryukov wrote: > I wonder whether we could use `Function` and get rid of 'expression' mode >

[PATCH] D65592: [Parser] Avoid spurious 'missing template' error in presence of typos

ilya-biryukov created this revision. ilya-biryukov added a reviewer: rsmith. Herald added a project: clang. ilya-biryukov added a parent revision: D65591: [AST] Add a flag indicating if any subexpression had errors. Suppress those diagnostics if lhs of a member expression contains errors. Typo

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

ilya-biryukov created this revision. ilya-biryukov added a reviewer: rsmith. Herald added a subscriber: jfb. Herald added a project: clang. ilya-biryukov added a child revision: D65592: [Parser] Avoid spurious 'missing template' error in presence of typos. The only subexpression that is

[PATCH] D65525: [clangd] Add new helpers to make tweak tests scale better. Convert most tests. NFC

ilya-biryukov added a comment. Neat, thanks for the cleanup! A few suggestions from me, no major comments. Comment at: clang-tools-extra/clangd/unittests/TweakTesting.cpp:74 +return true; + llvm::consumeError(PrepareResult.takeError()); + return false;

[PATCH] D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:68 + if (const auto *TD = dyn_cast(D)) +return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation; + return false; hokein wrote: > ilya-biryukov wrote: >

[PATCH] D65517: [Preprocessor] Always discard body of #define if we failed to parse it

ilya-biryukov added a comment. SG, just wanted to make sure it's on your list ;-) Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65517/new/ https://reviews.llvm.org/D65517 ___ cfe-commits mailing list

[PATCH] D65517: [Preprocessor] Always discard body of #define if we failed to parse it

ilya-biryukov added a subscriber: hans. ilya-biryukov added a comment. @hans, could you please merge rL367530 into the release? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65517/new/ https://reviews.llvm.org/D65517

[PATCH] D65517: [Preprocessor] Always discard body of #define if we failed to parse it

ilya-biryukov updated this revision to Diff 212754. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Update a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65517/new/ https://reviews.llvm.org/D65517 Files:

[PATCH] D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:68 + if (const auto *TD = dyn_cast(D)) +return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation; + return false; We also want to skip

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov accepted this revision. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. LGTM from my side Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1125 +Old = std::move(FileToHighlightings[File]); +FileToHighlightings[File]

[PATCH] D64907: [AST] Traverse attributes inside DEF_TRAVERSE_DECL macro

ilya-biryukov updated this revision to Diff 212592. ilya-biryukov added a comment. - Reformat Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64907/new/ https://reviews.llvm.org/D64907 Files: clang/include/clang/AST/RecursiveASTVisitor.h

[PATCH] D64907: [AST] Traverse attributes inside DEF_TRAVERSE_DECL macro

ilya-biryukov updated this revision to Diff 212591. ilya-biryukov added a comment. Herald added a subscriber: mgorny. - Add a test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64907/new/ https://reviews.llvm.org/D64907 Files:

[PATCH] D65517: [Preprocessor] Always discard body of #define if we failed to parse it

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added a project: clang. Preivously we would only discard it if we failed to parse parameter lists. If we do not consume the body, parser sees tokens inside directive. In turns, this leads to spurious

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

ilya-biryukov added a comment. An experiment with popping on expression evaluation context failed, I couldn't find a good way to fix the problems described above. Typo correction can and will run after the evaluation context where expression created is popped and the diagnostic we produce

[PATCH] D65263: [clangd] a prototype for triggering rename after extracting.

ilya-biryukov added a comment. In D65263#1606371 , @hokein wrote: > I agree that doing rename after extraction is not an ideal approach (both > correctness and latency), there should be other better ways to achieve this. > However as a LSP server, we

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:408 +} + } // namespace ilya-biryukov wrote: > Could you also add a separate test that checks diffs when the number of lines > is getting smaller

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1125 +Old = std::move(FileToHighlightings[File]); +FileToHighlightings[File] = Highlightings; + } NIT: avoid copying (from `Highlightings` into the map) under

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

ilya-biryukov added a comment. Tried the suggested approach and ran into the issue described above. Either we make the diagnostics less useful ('did you mean foo::bar?' turns into 'unresolved identifier bar'; without mentioning the correction) or we even stop providing some corrections in

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

ilya-biryukov added a comment. Thanks for the suggestion. Sounds reasonably simple, I'll try this and report back with the result. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64799/new/ https://reviews.llvm.org/D64799

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

ilya-biryukov added a comment. @rsmith, emitting the typos on pop expression evaluation context resulted in a bunch of failures when trying to transform the resulting errors, see a typical stacktrace below. It seems we can actually pop expression evaluation contexts between producing and

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov added a comment. Mostly final NITs from me, but there is an important one about removing the `erase` call on `didOpen`. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:467 +std::lock_guard Lock(HighlightingsMutex); +

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov added a comment. In D64475#1602195 , @jvikstrom wrote: > In D64475#1593481 , @ilya-biryukov > wrote: > > > The fix for a race condition on remove has landed in rL366577 > >

[PATCH] D64863: [clangd] Ignore diags from builtin files

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Many thanks for fixing this. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:111 -void adjustDiagFromHeader(Diag , const clang::Diagnostic , +bool

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

ilya-biryukov added a comment. Could you add a bit more context to the description of the change: why do we need the callback and what is the problem with what we have now? I'm sure this was discussed offline and the change is justified, just wanted to make sure we write it down in the change

[PATCH] D65263: [clangd] a prototype for triggering rename after extracting.

ilya-biryukov added a comment. If we are going down that path, could we consider to pre-selecting a name of the extract variable and its usage (using multi-select)? I.e. the optimal workflow seems to be (`[[code]] marks selected regions`): // Input: foo([[10+10]]); // 'extract'

[PATCH] D64863: [clangd] Ignore diags from builtin files

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:563 FillDiagBase(*LastDiag); -adjustDiagFromHeader(*LastDiag, Info, *LangOpts); +if (!InsideMainFile) + LastDiagWasAdjusted = adjustDiagFromHeader(*LastDiag, Info,

[PATCH] D64762: [AST] Treat semantic form of InitListExpr as implicit code in traversals

ilya-biryukov updated this revision to Diff 211035. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Rewrite code as suggested in the review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64762/new/

[PATCH] D64762: [AST] Treat semantic form of InitListExpr as implicit code in traversals

ilya-biryukov added inline comments. Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:2332 S->isSemanticForm() ? S->getSyntacticForm() : S, Queue)); TRY_TO(TraverseSynOrSemInitListExpr( S->isSemanticForm() ? S : S->getSemanticForm(), Queue));

[PATCH] D64863: [clangd] Ignore diags from builtin files

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:128 +return false; + Position StartPos = sourceLocToPosition(SM, IncludeInMainFile); NIT: inline `StartPos`, it has online a single usage now.

[PATCH] D64863: [clangd] Ignore diags from builtin files

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:601 - if (mentionsMainFile(*LastDiag) || - (LastDiag->Severity >= DiagnosticsEngine::Level::Error && - IncludeLinesWithErrors.insert(LastDiag->Range.start.line).second)) {

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov added a comment. The fix for a race condition on remove has landed in rL366577 , this revision would need a small update after it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64475/new/

[PATCH] D64985: [clangd] Provide a way to publish highlightings in non-racy manner

ilya-biryukov updated this revision to Diff 210823. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Group PublishFn with onMainAST Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64985/new/

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:307 +llvm::StringRef NewCode; +std::vector DiffedLines; + } TestCases[]{ @hokein rightfully pointed out that mentioning all changed lines

[PATCH] D64638: [CrossTU] Fix plist macro expansion if macro in other file.

ilya-biryukov added a comment. `StaticAnalyzer/Core` does not depend on `clangFrontend` now, you can see this by looking at `lib/StaticAnalyzer/Core/CMakeLists.txt`: add_clang_library(clangStaticAnalyzerCore ... LINK_LIBS clangAST clangASTMatchers clangAnalysis

[PATCH] D64985: [clangd] Provide a way to publish highlightings in non-racy manner

ilya-biryukov added a comment. Thanks for all the suggestions. This is ready for the next round now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64985/new/ https://reviews.llvm.org/D64985 ___

[PATCH] D64985: [clangd] Provide a way to publish highlightings in non-racy manner

ilya-biryukov updated this revision to Diff 210808. ilya-biryukov added a comment. - Remove a leftover comment from the previous version Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64985/new/ https://reviews.llvm.org/D64985 Files:

[PATCH] D64985: [clangd] Provide a way to publish highlightings in non-racy manner

ilya-biryukov updated this revision to Diff 210807. ilya-biryukov added a comment. - Update usage of DiagsMu in a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64985/new/ https://reviews.llvm.org/D64985 Files:

[PATCH] D64985: [clangd] Provide a way to publish highlightings in non-racy manner

ilya-biryukov updated this revision to Diff 210805. ilya-biryukov added a comment. - Use the same mechanism for diagnostics - Change typedef to function)> - Update a comment - s/PublishResults/PublishFn - Reformat Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

<    4   5   6   7   8   9   10   11   12   13   >