[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: clang-tools-ex

[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: clang/include/clang/AST/RecursiveASTVisi

[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? ``` /// Indi

[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) {} +void

[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 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"), DeclNamed(

[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 "_Bo

[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 Mon

[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] 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: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:95

[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 option,

[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 mail

[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 SI

[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 hor

[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] 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" wit

[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` b

[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 stru

[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 do

[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 cat

[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 https://review

[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 are

[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 https://r

[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 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 w

[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 pr

[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 param

[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 wrot

[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] 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] 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 bo

[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 &L, const Reference &R) { - 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 &L, const Reference &R) { - 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/SemanticHighlighting.cpp:54 + if (Conflicting.size() > 1) { +Tokens.erase(Tokens.begin() + I, + Tokens.begin() + I + Conflicting.size()); This is potentiall

[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 mac

[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 &L, const Reference &R) { - 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 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: clang-tools-extra/clangd/Sema

[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: clan

[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 &L, const Reference &R) { - 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 &L, const Reference &R) { - return std::tie(L.Loc, L.CanonicalTarget, L.Role) < - std::tie(R.Loc, R.CanonicalTarget, R.Role); +

[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 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] D63621: [git-clang-format] recognize hxx as a C++ file

ilya-biryukov accepted this revision. ilya-biryukov added a reviewer: ilya-biryukov. ilya-biryukov added a comment. LGTM. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63621/new/ https://reviews.llvm.org/D63621 ___ c

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov updated this revision to Diff 205974. ilya-biryukov added a comment. - A few more renames and docs - Cleanups and comments - Reformat the code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61637/new/ https://reviews.llvm.org/D61637 F

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov marked 13 inline comments as done. ilya-biryukov added a comment. This is ready for another round now. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:37 + /// and \p Last are added as children of the new node. + void learnNode(SourceLocation Fist, SourceLoca

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov marked 4 inline comments as done. ilya-biryukov added a comment. Although there are still rough edges, I believe the storage model is agreed upon and we can hopefully address the rest in the follow-ups. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D63264: [clang][Driver] Deduce target triplet from clang executable name

ilya-biryukov added a comment. After looking at this, adjusting arguments on the clangd side seems to be a better approach. Let's abandon this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63264/new/ https://reviews.llvm.org/D63264 ___

[PATCH] D63194: [clangd] Link in target infos and pass target and mode while invoking driver

ilya-biryukov added a comment. Sorry about the confusion, I can now see why the original version of the patch was actually simpler. I was put off by the fact that we override by adding command-line arguments instead of passing things around in the code, but it appears that's actually simpler th

[PATCH] D63194: [clangd] Link in target infos and pass target and mode while invoking driver

ilya-biryukov added a comment. Thanks, this looks very good! Leaving a few nitpicky comments, but nothing important really. Two more general NITs: - could we update the title and description of this revision to mirror that it focuses on code in tooling rather than in clangd? - maybe land the cl

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

ilya-biryukov updated this revision to Diff 206246. ilya-biryukov marked 10 inline comments as done. ilya-biryukov added a comment. - Address comments, document code. - s/Expansion/CollectedExpansions. - Added FIXMEs for macro arguments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:319 + /// 2. macro name and arguments for macro expansions. + using PPExpansions = llvm::DenseMap; class Builder; sammccall wrote: > do I understand right that thi

[PATCH] D63755: [clang][Tooling] Infer target and mode from argv[0] when using JSONCompilationDatabase

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/D63755/new/ https://reviews.llvm.org/D63755 _

[PATCH] D63194: [clangd] Link and initialize target infos

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:14 #include "clang/Tooling/CompilationDatabase.h" +#include "clang/Tooling/Tooling.h"

[PATCH] D61681: [clangd] A code tweak to expand a macro

ilya-biryukov updated this revision to Diff 206388. ilya-biryukov added a comment. - Rebase - Update some comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61681/new/ https://reviews.llvm.org/D61681 Files: clang-tools-extra/clangd/refactor/

[PATCH] D63817: [clangd] No longer getting template instantiations from header files in Main AST.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:47 #include +#include NIT: This include is redundant, maybe remove? Probably added by accident. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:71

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov updated this revision to Diff 206714. ilya-biryukov added a comment. - Introduce roles to allow distinguishing the child nodes. - Remove recovery node, use an unknown role instead. - TreeBuidler now can consume children at any point, not just suffix nodes. Repository: rG LLVM Git

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov updated this revision to Diff 206715. ilya-biryukov added a comment. - Remove (outdated) changes to gn files Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61637/new/ https://reviews.llvm.org/D61637 Files: clang/include/clang/Toolin

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov added a comment. This is now in a pretty good shape, I've incorporated changes after our offline discussions about child roles. The builder interface is also much richer now, removing a requirement that the tree has to be traversed left-to-right (bottom-up is still required!). Re

[PATCH] D63835: [Syntax] Add nodes for most common statements

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added a project: clang. ilya-biryukov added a parent revision: D61637: [Syntax] Introduce syntax trees. Most of the statements mirror the ones provided by clang AST. Major differences are: - expressions are wra

[PATCH] D63835: [Syntax] Add nodes for most common statements

ilya-biryukov added a comment. This change mostly aims to illustrate that `TreeBuilder` seems to be powerful enough to go beyond basic nodes. But it also introduces enough nodes to make the syntax trees minimally useful for traversing statement nodes. Hopefully that could become a good basis to

[PATCH] D61681: [clangd] A code tweak to expand a macro

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp:57 + if (It == Spelled.begin()) +return Spelled.end(); + // Check the token we found actually touches the cursor position. sammccall wrote: > it's pret

[PATCH] D61681: [clangd] A code tweak to expand a macro

ilya-biryukov updated this revision to Diff 208425. ilya-biryukov marked 5 inline comments as done. ilya-biryukov added a comment. - Replace bsearch with partition_point. - Include macro name in the title. - Added a FIXME for empty selection case. - Return null when no token is found. Repository

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov updated this revision to Diff 208454. ilya-biryukov marked 4 inline comments as done. ilya-biryukov added a comment. - s/TranslationUnitDeclaration/TranslationUnit - Remove accessor from 'eof', add a FIXME to remove it from the tree altogether Repository: rG LLVM Github Monorepo

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:35 +/// A root node for a translation unit. Parent is always null. +class TranslationUnitDeclaration final : public Tree { +public: sammccall wrote: > I don't think TU is

[PATCH] D61637: [Syntax] Introduce syntax trees

This revision was automatically updated to reflect the committed changes. Closed by commit rGb736969eddce: [Syntax] Introduce syntax trees (authored by ilya-biryukov). Changed prior to commit: https://reviews.llvm.org/D61637?vs=208454&id=208455#toc Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov added a comment. In D61637#1575542 , @RKSimon wrote: > @ilya-biryukov We're seeing buildbot failures in SyntaxTests.exe : > > http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/50927 > > http://lab.llvm.o

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov added a comment. Relanded in rL365466 with a fix to the crash. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61637/new/ https://reviews.llvm.org/D61637 __

[PATCH] D63835: [Syntax] Add nodes for most common statements

ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. Submitting a few comments to start up the discussions. The actual changes will follow. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:99 /// An abstract node for C++ statements, e.g. 'while

[PATCH] D63085: Provide a fix-it hint for -Wswitch, which adds missing cases. If there are >3 cases, the inserted text will contain newlines so it will not be shown in console output (but will be appl

ilya-biryukov added a comment. One major drawback that I see is the lack of indentation (and other format options) in the added code. Should we have this fix at a higher level that can have formatting (either now or in the future)? E.g. in `clangd` directly? Comment at: lib/S

[PATCH] D64481: [clangd] Add a flag to clangdServer rename function to control whether we want format the replacements.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM with a small NIT. Was also thinking about adding a test for this, but the amount of work required to do so seems to outweigh the usefulness. Therefore seems ok to land witho

[PATCH] D63835: [Syntax] Add nodes for most common statements

ilya-biryukov updated this revision to Diff 208937. ilya-biryukov marked 5 inline comments as done. ilya-biryukov added a comment. - Rebase - Address comments - Restructure the roles - Remove the role from tree dumps for now With too many roles it is annoying to update the test outputs on increme

[PATCH] D63835: [Syntax] Add nodes for most common statements

ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:185 +/// if (cond) else +class IfStatement final : public Statement { +public: sammccall wrote: > I guess the missing cond here (and similar below) are due to complexiti

[PATCH] D63835: [Syntax] Add nodes for most common statements

ilya-biryukov updated this revision to Diff 208995. ilya-biryukov added a comment. - Mark groups of kinds for statements and expressions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63835/new/ https://reviews.llvm.org/D63835 Files: clang/includ

[PATCH] D63835: [Syntax] Add nodes for most common statements

ilya-biryukov added a comment. This is ready for another round Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:25 /// A kind of a syntax node, used for implementing casts. enum class NodeKind : uint16_t { Leaf, sammccall wrote: > there are going to

[PATCH] D64565: [clangd] Don't run the prepare for tweaks that are disabled.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.h:136 -/// Returns true if the StringRef is a tweak that should be enabled -std::function TweakFilter = [](llvm::StringRef TweakToSearch) {return true;}; +/// Returns true if the

[PATCH] D64573: [Syntax] Allow to mutate syntax trees

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added a subscriber: mgorny. Herald added a project: clang. ilya-biryukov added a parent revision: D63835: [Syntax] Add nodes for most common statements. This patch adds facilities to mutate the syntax trees and

[PATCH] D61841: [clangd] Respect WarningsAsErrors configuration for clang-tidy

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:346 + std::string CheckName = CTContext->getCheckName(Info.getID()); + if (!CheckName.empty() && WarningAsErrorFilter->contains(CheckName)) { +Level = DiagnosticsEngine::Err

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. I'll try to explore bringing the overhead down. The fact that `CachingLex` is happening at `LexLevel==1` might help, thanks for pointing that out! Comment at: clang/lib/Lex/PPCaching.cpp:64 ExitC

[PATCH] D53866: [Preamble] Stop circular inclusion of main file when building preamble

ilya-biryukov added a comment. I guess using `Edit Related Object -> Edit Commits` should do the trick. I'm not sure what the "Lean Into Action" is either. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53866/new/ https://reviews.llvm.org/D53866 ___

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:196 +/// Joins multiple rules into a single rule that applies the first rule in +/// `Rules` whose pattern matches a given node. All of the rules must use the +/// same kind of

[PATCH] D61386: [clang-tidy] Add support writing a check as a Transformer rewrite rule.

ilya-biryukov added a comment. @aaron.ballman, would be a good reviewer for this? I'm happy to look at the transformer bits, but I'm definitely the wrong one to asses it from the `clang-tidy` perspective. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ilya-biryukov added a comment. The implementation LG, thanks! Just a few NITs and comments about the public interface and the comments Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:196 +/// Joins multiple rules into a single rule that applies the first rule

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 199485. ilya-biryukov added a comment. - Use a flag inside clang::Token instead Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/ https://reviews.llvm.org/D59885 Files: clang/include/clang/Lex/P

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov added a comment. The suggested approach looks promising. The difference seems to be within the noise levels on my machine: Before the change (baseline upstream revision): Time (mean ± σ): 159.1 ms ± 8.7 ms[User: 138.3 ms, System: 20.6 ms] Range (min … max): 1

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 199486. ilya-biryukov added a comment. - Remove the now redundant 'public:' spec. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/ https://reviews.llvm.org/D59885 Files: clang/include/clang/Lex

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov added a comment. Will do. Also think reporting annotation tokens is ok, one can easily tell them apart and filter them by looking at the corresponding flags in `clang::Token`, will put it into the docs, though Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov added a comment. I've added a new parameter to `EnterToken` and `EnterTokenStream`. There are a bunch of interesting cases. `Preprocessor::AnnotateCachedTokens` is one. It consumes some `CachedTokens` (which are always considered re-injected) and replaces them with a single annot

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 199637. ilya-biryukov added a comment. Herald added a subscriber: eraman. - Properly mark tokens as reinjected where necessary. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/ https://reviews.llv

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 199639. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Improve comment, remove redundant braces Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/ https://reviews.l

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov added inline comments. Comment at: clang/lib/Lex/Preprocessor.cpp:955 --LexLevel; + if (OnToken && LexLevel == 0 && !Result.getFlag(Token::IsReinjected)) +OnToken(Result); Could probably remove the `IsReinjected` check from here. It's fine t

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 199775. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/ https://reviews.llvm.org/D59885 Files:

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:1521 Tok[0].setAnnotationValue(AnnotationVal); - EnterTokenStream(std::move(Tok), 1, true); + EnterTokenStream(std::move(Tok), 1, true, /*IsReinject*/ true); } rsmith wrote: > I

[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.

ilya-biryukov added a comment. Thanks! Mostly NITs from me, the design looks nice! Would you mind adding some tests before we land this? The only major thing that I'd argue against is adding helper functions like `findPreviousToken` to `SourceCode.h`. It's fine to have them in the `.cpp` files

[PATCH] D62010: [CodeComplete] Complete enumerators when preferred type is an enum

ilya-biryukov created this revision. ilya-biryukov added a reviewer: kadircet. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D62010 Files: clang/lib/Sema/SemaCodeComplete.cpp clang/test/CodeCompletion/enum-preferred-type.cpp Index: clang/test/

[PATCH] D62010: [CodeComplete] Complete enumerators when preferred type is an enum

ilya-biryukov updated this revision to Diff 199835. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - add a test case with a fixme Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62010/new/ https://reviews.llvm.org/D620

[PATCH] D62010: [CodeComplete] Complete enumerators when preferred type is an enum

ilya-biryukov added a comment. Thanks for a quick review! Comment at: clang/test/CodeCompletion/enum-preferred-type.cpp:19 + // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:16 %s -o - | FileCheck %s + // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:21 %

[PATCH] D62010: [CodeComplete] Complete enumerators when preferred type is an enum

ilya-biryukov updated this revision to Diff 199839. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Fix a line number Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62010/new/ https://reviews.llvm.org/D62010 Files:

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ilya-biryukov added a comment. LG, just a few NITs wrt to exposing implementation details in the header. Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:161 +// components are gathered as a `Case` and rules are defined as an ordered list +// of cases. //

<    13   14   15   16   17   18   19   20   21   22   >