[PATCH] D124250: [Serialization] write expr dependence bits as a single integer

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Nice savings Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:2291 + unsigned ExprDependenceBits = llvm::BitWidth; // Abbreviation for EXPR_DEC

[PATCH] D80721: Updates to the 'CLion Integration' section in ClangFormat docs

This revision was automatically updated to reflect the committed changes. Closed by commit rG9982d48a92be: Updates to the 'CLion Integration' section in ClangFormat docs (authored by ilya-biryukov). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D69330: [AST] Add RecoveryExpr to retain expressions on semantic errors

ilya-biryukov updated this revision to Diff 241718. ilya-biryukov added a comment. Fix compilation after rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69330/new/ https://reviews.llvm.org/D69330 Files: clang/include/clang/AST/ComputeDepend

[PATCH] D69330: [AST] Add RecoveryExpr to retain expressions on semantic errors

ilya-biryukov updated this revision to Diff 241717. ilya-biryukov added a comment. Herald added a subscriber: bmahjour. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69330/new/ https://reviews.llvm.org/D69330 Files: clang/include/clang/

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

ilya-biryukov added a comment. As soon as I add a new enum value, this starts crashing some tests (even if we don't ever set the flag or serialize it). Not sure what causes it and won't have time to figure it out before I leave. Sorry about that :( Repository: rG LLVM Github Monorepo CHANGE

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

ilya-biryukov updated this revision to Diff 241706. ilya-biryukov added a comment. Herald added a subscriber: bmahjour. Rebase on top of refactored dependence propagation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65591/new/ https://reviews.llvm

[PATCH] D73723: [clangd][Hover] Handle uninstantiated default args

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/Hover.cpp:276 +// we only print the expression. +if (PVD->hasDefaultArg() && !PVD->hasUnparsedDefaultArg()) {

[PATCH] D73638: [AST] Move dependence computations into a separate file

ilya-biryukov added a comment. @rsmith this does not pass through testing, I've messed up somewhere while moving the code. I'll find what went wrong and fix it tomorrow. Please tell if the approach itself LG. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D73617: [clangd] Don't mmap source files on all platforms --> don't crash on git checkout

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

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

ilya-biryukov added a comment. I've opted for duplicating the common flags across all the introduced enums (contains-unexpanded-pack, instantiation-dependent) , this is somewhat ugly, but everything else is even more complicated to use. Less enums would also be a good idea probably, see the rele

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

ilya-biryukov updated this revision to Diff 240958. ilya-biryukov marked an inline comment as done and an inline comment as not done. ilya-biryukov added a comment. - Use different types for different AST categories - Rename to getDependence Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clang/include/clang/AST/DependencyFlags.h:17-28 +enum class DependencyFlags : uint8_t { + Type = 1, + Value = 2, + Instantiation = 4, + UnexpandedPack = 8, + + // Shorthands for

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

ilya-biryukov updated this revision to Diff 240604. ilya-biryukov added a comment. - Use DependencyFlags for TemplateName and NestedNameSpecifier Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71920/new/ https://reviews.llvm.org/D71920 Files: cla

[PATCH] D72508: [clangd] Support pseudo-obj expr, opaque values, and property references in findExplicitReferences()

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.cpp:794 +// Not clear why the source expression is skipped by default... +return RecursiveASTVisitor::TraverseStmt(OVE->getSourceExpr()); + } Should this be done by `Rec

[PATCH] D72334: [Syntax] Build nodes for template declarations.

ilya-biryukov updated this revision to Diff 238023. ilya-biryukov added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72334/new/ https://reviews.llvm.org/D72334 Files: clang/include/clang/Tooling/Syntax/Nodes.h clang/lib/To

[PATCH] D72089: [Syntax] Build declarator nodes

ilya-biryukov updated this revision to Diff 238022. ilya-biryukov added a comment. - Rebase, get rid of accidental changes with TemplateDeclaration Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72089/new/ https://reviews.llvm.org/D72089 Files: c

[PATCH] D72715: [clang][CodeComplete] Propogate printing policy to FunctionDecl

ilya-biryukov added a comment. The patch contains only tests, are we missing the actual functional change? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72715/new/ https://reviews.llvm.org/D72715 ___ c

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

ilya-biryukov added a comment. In D72498#1816424 , @lh123 wrote: > Currently, I think that in most cases, showing both expanded (canonical) and > spelled types is sufficient. > > > This has been used in ycmd for ~4 years without complaint. > > https://gi

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

ilya-biryukov updated this revision to Diff 237577. ilya-biryukov added a comment. - Add compound assignment operations Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71920/new/ https://reviews.llvm.org/D71920 Files: clang-tools-extra/clang-tidy/

[PATCH] D72581: [Syntax] Add mapping from spelled to expanded tokens for TokenBuffer

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added a project: clang. Same restrictions apply as in the other direction: macro arguments are not supported yet, only full macro expansions can be mapped. Repository: rG LLVM Github Monorepo https://review

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

ilya-biryukov added a comment. In D72498#1815500 , @lh123 wrote: > - hover over the `front` , you'll see "instance-method `front` → > `std::vector >::reference`". > - hover over the `push_back`, you'll see "`std::vector std::allocator >::value_type && __x

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

ilya-biryukov added a comment. In D72498#1814366 , @sammccall wrote: > @ilya-biryukov @kadircet what do you think about unwrapping decltype only > when it's a return value (optional: of a function whose leading return type > is auto) to narrowly catch th

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

ilya-biryukov added a comment. In D65591#1791876 , @rsmith wrote: > @ilya-biryukov Did I forget anything? SG, I don't think anything is missing. Thanks for the write-up! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

ilya-biryukov added a comment. In D72498#1813962 , @sammccall wrote: > Maybe we want both the normal type and the canonical type? > > Canonical types are often *really* ugly, especially with STL types (we don't > have the "as written" form). And presentin

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

ilya-biryukov added a comment. In D72498#1813963 , @kadircet wrote: > I think typedef and decltype have different nature, the latter is a lot more > obscure than the former, that was the reason why I handled decltypes > specifically. I tend to disagree

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

ilya-biryukov added a comment. In D72498#1813962 , @sammccall wrote: > It's particularly unclear to me why typeprinter descends into auto but prints > decltype, but Kadir says that seems to be intentional. Also don't see why they choose to have this inc

[PATCH] D72497: [CodeComplete] Suggest 'return nullptr' in functions returning pointers

This revision was automatically updated to reflect the committed changes. Closed by commit rG57a51b689e7b: [CodeComplete] Suggest 'return nullptr' in functions returning pointers (authored by ilya-biryukov). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

ilya-biryukov added a comment. Could it be the case that we want to show the canonical types (i.e. without all syntax sugar)? Maybe we want both the normal type and the canonical type? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72498/new/ https

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

ilya-biryukov added a comment. This does not work for more complicated types, though? E.g. `decltype(a+b)* a` or `vector a`? Why do we prefer to drop `decltype()`, yet show the typedefs? Both could lead to complicated types, arguably decltypes can be even worse than typedefs. Repository: rG

[PATCH] D72497: [CodeComplete] Suggest 'return nullptr' in functions returning pointers

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/D72497 Files: clang/lib/Sema/SemaCodeComplete.cpp clang/test/CodeCompletion/patterns.cpp Index: clang/test/CodeComple

[PATCH] D72494: [clangd] Fix targetDecl() on certain usage of ObjC properties.

ilya-biryukov requested changes to this revision. ilya-biryukov added a comment. This revision now requires changes to proceed. Could you also add a test (and possibly support this in the visitors) for `findExplicitReferences`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D72462: [clangd] Fix markdown rendering in VSCode

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

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

ilya-biryukov marked 2 inline comments as done. ilya-biryukov added inline comments. Comment at: include/clang/Tooling/Syntax/Tokens.h:206 + /// DECL(a); + /// spelledTokens() returns {"#", "define", "DECL", "(", "name", ")", "eof"}. + /// FIXME: we do not yet store tokens

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

ilya-biryukov marked 2 inline comments as done. ilya-biryukov added inline comments. Comment at: include/clang/Tooling/Syntax/Tokens.h:206 + /// DECL(a); + /// spelledTokens() returns {"#", "define", "DECL", "(", "name", ")", "eof"}. + /// FIXME: we do not yet store tokens

[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:898 class $10^Foo { - $11^Foo(int); +

[PATCH] D72458: [clangd] Adjust diagnostic range to be inside main file

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/Diagnostics.cpp:342 +llvm::Optional NoteInsideMainFile; +for (auto &N : D.Notes) { + if (!N.InsideMainFil

[PATCH] D72446: [Syntax] Build mapping from AST to syntax tree nodes

ilya-biryukov updated this revision to Diff 237042. ilya-biryukov added a comment. - Remove debug logs - Cosmetics Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72446/new/ https://reviews.llvm.org/D72446 Files: clang/include/clang/Tooling/Syntax

[PATCH] D72446: [Syntax] Build mapping from AST to syntax tree nodes

ilya-biryukov updated this revision to Diff 237039. ilya-biryukov added a comment. Remove the (now redundant) NodeAndRole class Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72446/new/ https://reviews.llvm.org/D72446 Files: clang/include/clang/T

[PATCH] D72446: [Syntax] Build mapping from AST to syntax tree nodes

ilya-biryukov created this revision. ilya-biryukov added a reviewer: gribozavr2. Herald added a project: clang. ilya-biryukov updated this revision to Diff 237039. ilya-biryukov added a comment. ilya-biryukov added a parent revision: D72334: [Syntax] Build nodes for template declarations.. Remove

[PATCH] D72119: [clangd] Handle DeducedTemplateSpecializationType in TargetFinder

ilya-biryukov added a comment. Here's my attempt at storing the deduced type in `TypeLoc`s: D72442 . It almost worked, but still needs an update to fix some test failures. The change proves it's possible to do this in principle, although it does require a few ha

[PATCH] D72442: [Sema] Store deduced type in TypeLoc

ilya-biryukov planned changes to this revision. ilya-biryukov added a comment. This produces the following test failures after running `check-clang`: Failing Tests (4):

[PATCH] D72442: [Sema] Store deduced type in TypeLoc

ilya-biryukov created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet. Herald added a project: clang. This greatly simplifies source-level tools that need to know what 'auto' deduced to, e.g. clangd. Also change default presentation of such types to print 'auto' instead

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

ilya-biryukov added a comment. @rsmith, gentle ping. WDYT? Is this a step in the right direction? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71920/new/ https://reviews.llvm.org/D71920 ___ cfe-commit

[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics

ilya-biryukov requested changes to this revision. ilya-biryukov added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:571 + ADD_FAILURE() << D << Code; +assert(AST.getDiagnostics().empty()

[PATCH] D72119: [clangd] Handle DeducedTemplateSpecializationType in TargetFinder

ilya-biryukov added a comment. I'm also looking into fixing this in clang, this shouldn't be too hard. But please land the workaround for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72119/new/ https://reviews.llvm.org/D72119 _

[PATCH] D72119: [clangd] Handle DeducedTemplateSpecializationType in TargetFinder

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/FindTarget.cpp:370 +if (auto *TD = DTST->getTemplateName().getAsTemplateDecl()) { + Outer.add(TD->get

[PATCH] D72334: [Syntax] Build nodes for template declarations.

ilya-biryukov updated this revision to Diff 236583. ilya-biryukov added a comment. - Cosmetics Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72334/new/ https://reviews.llvm.org/D72334 Files: clang/include/clang/Tooling/Syntax/Nodes.h clang/lib

[PATCH] D72334: [Syntax] Build nodes for template declarations.

ilya-biryukov created this revision. ilya-biryukov added a reviewer: gribozavr2. Herald added a project: clang. ilya-biryukov added a parent revision: D72089: [Syntax] Build declarator nodes. ilya-biryukov updated this revision to Diff 236583. ilya-biryukov added a comment. - Cosmetics Handles t

[PATCH] D71406: [clangd] Add xref for macros to FileIndex.

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 Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:362 + R.Location.End.setColumn(Range.en

[PATCH] D71406: [clangd] Add xref for macros to FileIndex.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:350 + const auto *MainFileEntry = SM.getFileEntryForID(SM.getMainFileID()); + if (!MainFileEntry) +return; Does this ever happen? Maybe `assert` it's not nul

[PATCH] D67224: [clangd] Enable completions with fixes in VSCode

ilya-biryukov added a comment. In D67224#1804513 , @nridge wrote: > Just throwing a wild idea out there: what if we used > `textDocument/onTypeFormatting` to replace the `.` with `->` as soon as it's > typed? There is no way we can do this with proper

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.h:194 +llvm::SmallVector +explicitReferenceTargets(ast_type_traits::DynTypedNode N, + DeclRelationSet Mask = {}); sammccall wrote: > ilya-biryukov wrote:

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.h:194 +llvm::SmallVector +explicitReferenceTargets(ast_type_traits::DynTypedNode N, + DeclRelationSet Mask = {}); ilya-biryukov wrote: > sammccall wrote:

[PATCH] D72163: [clangd] targetDecl() returns only NamedDecls.

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

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.h:194 +llvm::SmallVector +explicitReferenceTargets(ast_type_traits::DynTypedNode N, + DeclRelationSet Mask = {}); sammccall wrote: > ilya-biryukov wrote:

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.h:194 +llvm::SmallVector +explicitReferenceTargets(ast_type_traits::DynTypedNode N, + DeclRelationSet Mask = {}); sammccall wrote: > ilya-biryukov wrote:

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.h:194 +llvm::SmallVector +explicitReferenceTargets(ast_type_traits::DynTypedNode N, + DeclRelationSet Mask = {}); sammccall wrote: > ilya-biryukov wrote:

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.h:194 +llvm::SmallVector +explicitReferenceTargets(ast_type_traits::DynTypedNode N, + DeclRelationSet Mask = {}); sammccall wrote: > ilya-biryukov wrote:

[PATCH] D72119: [clangd] Handle DeducedTemplateSpecializationType in TargetFinder

ilya-biryukov requested changes to this revision. ilya-biryukov added a comment. This revision now requires changes to proceed. Please add a test for `findExplicitReferences` too Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72119/new/ https://revi

[PATCH] D72089: [Syntax] Build declarator nodes

ilya-biryukov created this revision. ilya-biryukov added a reviewer: gribozavr2. Herald added a project: clang. ilya-biryukov added a parent revision: D72072: [AST] Respect shouldTraversePostOrder when traversing type locs. They cover part of types and names for some declarations, including commo

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

ilya-biryukov marked 4 inline comments as done. ilya-biryukov added inline comments. Comment at: clang/include/clang/AST/Expr.h:126 +if (TD) + D = D | DependencyFlags::Type; +if (VD) riccibruno wrote: > ilya-biryukov wrote: > > Mordante wrote: > > >

[PATCH] D72085: [clangd] Fix hover for functions inside templated classes

ilya-biryukov added a comment. I landed a patch that does exactly this a few days ago: 14e11005d1a6ac1fecb230c470e9011d6956b8e4 Did you pull the latest changes? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D71652: [clangd] Replace shortenNamespace with getQualification

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/AST.cpp:307 + llvm::raw_string_ostream OS(Result); + auto Decls = explicitReferenceTargets( + ast_type_traits::

[PATCH] D72073: [Sema] Fix location of star ('*') inside MemberPointerTypeLoc

ilya-biryukov created this revision. ilya-biryukov added a reviewer: gribozavr2. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D72073 Files: clang/include/clang/Sema/DeclSpec.h clang/lib/Parse/ParseDecl.cpp clang/lib/Sema/SemaType.cpp Index

[PATCH] D72072: [AST] Respect shouldTraversePostOrder when traversing type locs

ilya-biryukov added a comment. Would be nice to write a test too, but didn't get to it yet... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72072/new/ https://reviews.llvm.org/D72072 ___ cfe-commits ma

[PATCH] D72072: [AST] Respect shouldTraversePostOrder when traversing type locs

ilya-biryukov created this revision. ilya-biryukov added a reviewer: gribozavr2. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D72072 Files: clang/include/clang/AST/RecursiveASTVisitor.h Index: clang/include/clang/AST/RecursiveASTVisitor.h

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

ilya-biryukov marked 4 inline comments as done. ilya-biryukov added inline comments. Comment at: clang/include/clang/AST/Expr.h:126 +if (TD) + D = D | DependencyFlags::Type; +if (VD) Mordante wrote: > Just curious why do you prefer `D = D | Dependenc

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

ilya-biryukov updated this revision to Diff 235837. ilya-biryukov added a comment. - Use DependencyFlagsBits for computing NumExprBits - Reformat Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71920/new/ https://reviews.llvm.org/D71920 Files: cla

[PATCH] D71965: include missing for std::abort

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Do you have commit access or should we land this for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71965/new/ https://r

[PATCH] D71982: [docs] Update path to clang-tools-extra

ilya-biryukov added a comment. +1, please upload a diff with full context Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71982/new/ https://reviews.llvm.org/D71982

[PATCH] D71962: Fix crash in getFullyQualifiedName for inline namespace

ilya-biryukov added a comment. Thanks for fixing this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71962/new/ https://reviews.llvm.org/D71962 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D71962: Fix crash in getFullyQualifiedName for inline namespace

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

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp:17 +using namespace clang::ast_matchers; + NOTE: We need this to fix compiler errors down below. Ther

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

ilya-biryukov added a comment. Also note that this change does not move any code around to make sure this change is easy to review and validate that the code is doing the same thing. I'm also planning to move all the code that computes dependencies into one place (it's currently scattered around

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

ilya-biryukov added a comment. Herald added a subscriber: rnkovacs. @rsmith, could you please take a look and let me know whether you think adding a new type for this makes sense? On one hand, I feel it's good to have a type to represent "dependencies" and it allow to write helper functions and

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

ilya-biryukov created this revision. ilya-biryukov added a reviewer: rsmith. Herald added a reviewer: martong. Herald added a reviewer: shafik. Herald added a project: clang. This changes introduces an enum to represent dependencies as a bitmask and extract common patterns from code that computes

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

ilya-biryukov added a comment. Thanks for the fixes! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64573/new/ https://reviews.llvm.org/D64573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

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 Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:136 HI.Kind = index::SymbolKind::Cl

[PATCH] D71652: [clangd] Replace shortenNamespace with getQualification

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:305 + llvm::raw_string_ostream OS(Result); + if (auto *TD = QT->getAsTagDecl()) +OS << getQualification(CurContext.getParentASTContext(), &CurContext, TD, Why not use `explici

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.h:194 +llvm::SmallVector +explicitReferenceTargets(ast_type_traits::DynTypedNode N, + DeclRelationSet Mask = {}); No need to fix this. The name could pro

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:188 +// Returns the decl that should be used for querying comments, either from index +// or ast. +const NamedDecl *getDeclForComment(const NamedDecl *D) { NIT: AST ===

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

ilya-biryukov marked 3 inline comments as done. ilya-biryukov added inline comments. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:528 +void test() { + HALF_IF HALF_IF_2 else {} +})cpp", ilya-biryukov wrote: > gribozavr2 wrote: > > Could you also do so

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG1ad15046dcf6: [Syntax] Allow to mutate syntax trees (authored by ilya-biryukov). Changed prior to commit: https://reviews.llvm.org/D64573?vs=234494&id=234496#toc Repository: rG LLVM Github Monorepo

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

ilya-biryukov marked an inline comment as not done. ilya-biryukov added inline comments. Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:57 + return nullptr; +} + gribozavr2 wrote: > Seems like these first/last helpers should be methods on `syntax::Node`

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

ilya-biryukov updated this revision to Diff 234494. ilya-biryukov marked 6 inline comments as done. ilya-biryukov added a comment. - Fix a header guard - Make firstLeaf and lastLeaf methods inside Tree - Mark non-modifiable nodes with I (for immutable) Repository: rG LLVM Github Monorepo CHAN

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:191 + // attached to that. + if (auto *CTSD = llvm::dyn_cast(D)) { +if (!CTSD->isExplicitInstantiationOrSpecialization()) You might need to do the same for specializations o

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:188 +// returns D. +const NamedDecl *getExplicitSpec(const NamedDecl *D) { + if (auto *CTSD = llvm::dyn_cast(D)) { kadircet wrote:

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

ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Syntax/BuildTree.h:25 +/// Allows to create syntax trees from subtrees not backed by the source code. +class Factory { +public: gribozavr2 wrote: > Why a class to contain static function

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

ilya-biryukov updated this revision to Diff 234359. ilya-biryukov marked 24 inline comments as done. ilya-biryukov added a comment. - Addressed most comments - Rebased onto HEAD Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64573/new/ https://revie

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:188 +// returns D. +const NamedDecl *getExplicitSpec(const NamedDecl *D) { + if (auto *CTSD = llvm::dyn_cast(D)) { kadircet wrote: > ilya-biryukov wrote: > > What's the purpose o

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:188 +// returns D. +const NamedDecl *getExplicitSpec(const NamedDecl *D) { + if (auto *CTSD = llvm::dyn_cast(D)) { What's the purpose of this function? I don't think its descript

[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

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

[PATCH] D71597: [clangd][NFC] Make use of TagDecl inside type for hover on auto

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

[PATCH] D71598: [clangd] Filter implicit references from index while renaming

ilya-biryukov added a comment. If we go with the solution proposed by @hokein, it looks like using the current patch is an improvement of what we have now. One big issue with the adding a new ref kind/ref modifier is that it requires modifications to Kythe-based index implementation, something t

[PATCH] D71406: [clangd] Add xref for macros to FileIndex.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:86 +// Add macro references. +for (const auto &IDToRefs : MacroRefsToIndex->MacroRefs) { + for (const auto &Range : IDToRefs.second) { hokein wrote: > ilya-

[PATCH] D71597: [clangd][NFC] Make use of TagDecl inside type for hover on auto

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:353 - if (D) { + if (const auto *D = T->getAsTagDecl()) { HI.Kind = index::getSymbolInfo(D).Kind; This might be a functional change in case of typedefs. Could you check

[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:372 +T.print(OS, Policy); +OS.flush(); + } NIT: is flush redundant? I believe it's called in destructor Comment at: clang-tools-extra/clangd/Hover.cpp:

[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:353 /// Generate a \p Hover object given the type \p T. HoverInfo getHoverContents(QualType T, const Decl *D, ASTContext &ASTCtx, + const SymbolIndex *Index) {

[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

ilya-biryukov added a comment. In D71543#1785842 , @kadircet wrote: > I've got D71545 to reduce that regression. D71545 seems to be pretty small, yet depends on this change. Maybe add i

[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:353 /// Generate a \p Hover object given the type \p T. HoverInfo getHoverContents(QualType T, const Decl *D, ASTContext &ASTCtx, + const SymbolIndex *Index) {

<    1   2   3   4   5   6   7   8   9   10   >