[PATCH] D69263: [clangd] Implement cross-file rename.

ilya-biryukov added a comment. Thanks, this is in a good shape! The only concern I have is the racy way to get dirty buffers, please see the corresponding comment. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:764 + /*WantFormat=*/true, + [this](PathRef Fi

[PATCH] D69804: [clang-tidy] Update TransformerClangTidyCheck to use new Transformer bindings.

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

[PATCH] D69890: [clangd] Improve the output of rename tests where there are failures.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM! Many thanks, the new tests are much simpler to read! Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:31 + llvm::StringRef Code = Test.code

[PATCH] D69892: [clang-rename] Respect the traversal scope when traversing the entire AST.

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

[PATCH] D69896: [libTooling] Small changes in Transformer API.

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

[PATCH] D69928: [clangd] Set RetainCommentsFromSystemHeaders to true

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM, thanks! Do you need someone to land this? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69928/new/ https://reviews.llvm.org/D

[PATCH] D69928: [clangd] Set RetainCommentsFromSystemHeaders to true

This revision was automatically updated to reflect the committed changes. Closed by commit rG001968490049: [clangd] Set RetainCommentsFromSystemHeaders to true (authored by ilya-biryukov). Changed prior to commit: https://reviews.llvm.org/D69928?vs=228173&id=228186#toc Repository: rG LLVM Gi

[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

ilya-biryukov added a comment. It's a bit unclear how we manage to still rename overrides. Is this handled by the USR-generating functions? Could we factor out the part that collects `NamedDecl`s and use those instead of the USRs instead? Comment at: clang-tools-extra/clangd/

[PATCH] D69937: [clangd] Use name of Macro to compute its SymbolID.

ilya-biryukov added a comment. Does that mean we identify each macro merely by its name? It's not uncommon to have multiple `#define` for the same name, meaning completely different things. If find references shows all of those, it's not very useful... Repository: rG LLVM Github Monorepo CH

[PATCH] D69937: [clangd] Use name of Macro to compute its SymbolID.

ilya-biryukov added a comment. Sorry for not reading through and assuming the wrong thing from the title. Thanks for the explanation! LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69937/new/ https://reviews.llvm.org/D69937 __

[PATCH] D69263: [clangd] Implement cross-file rename.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:342 + // A snapshot of all file dirty buffers. + llvm::StringMap SnapShot = WorkScheduler.getAllFileContents(); auto Action = [File = File.str(), NewName = NewName.str(), Pos, WantForm

[PATCH] D69263: [clangd] Implement cross-file rename.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. It's probably worth collecting a list of things we need to fix before enabling cross-file rename and putting it somewhere (a GitHub issue, maybe?) Important things that imm

[PATCH] D69961: [clangd] Fix a regression of not showing documentation from forward declarations.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/index/Merge.cpp:195 +bool IsClass = S.SymInfo.Kind == index::SymbolKind::Class || + S.SymInfo.Kind == index::SymbolKind::Struct; +if (!IsClass || !S.Definition) NI

[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:326 +// class Foo, but the token under the cursor is not corresponding to the +// "Foo" range, though the final result is correct. SourceLocation Loc = getBeginningOfIdentifier

[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

ilya-biryukov added a comment. NIT: a typo in the description: s/Aslo/Also Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69934/new/ https://reviews.llvm.org/D69934 ___ cfe-commits mailing list cfe-comm

[PATCH] D69961: [clangd] Fix a regression of not showing documentation from forward declarations.

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/unittests/IndexTests.cpp:416 R.Documentation = "Forward declarations because x.h is too big to include"; + EXPECT_

[PATCH] D69996: [clangd] Fixed colon escaping on Windows

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:128 +this.highlighter.highlight( +vscode.Uri.parse(params.textDocument.uri).toString(), lines); } Could we accept a `URI`

[PATCH] D69996: [clangd] Fixes colon escaping on Windows

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM, thanks! Should I land this patch for you? If you don't have commit access yet, you could consider applying to get one

[PATCH] D69996: [clangd] Fixes colon escaping on Windows

ilya-biryukov closed this revision. ilya-biryukov added a comment. Sorry for the delay, forgot to submit on Friday. Also updated `package-lock.json` before submitting. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69996/new/ https://reviews.llvm.org/D69996 ___

[PATCH] D70139: [clangd] Add bool return type to Index::refs API.

ilya-biryukov added a comment. NIT: we could probably add LLVM_NODISCARD attribute to ensure the return value is always consumed? Another NIT: maybe do this for all methods in the index that have the limit in their request (pretty much all of them, I guess)? Repository: rG LLVM Github Monore

[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:225 tooling::Replacements FilteredChanges; - for (const tooling::SymbolOccurrence &Rename : - findOccurrencesWithinFile(AST, RenameDecl)) { -// Currently, we only support n

[PATCH] D70225: [clangd] Simplify the code in XRefs

ilya-biryukov created this revision. ilya-biryukov added a reviewer: hokein. Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. While here, also fix potential UB in MergeIndex. Thanks Kadir for finding this! Repository: rG LLVM Github Mon

[PATCH] D70225: [clangd] Simplify the code in XRefs

ilya-biryukov marked 2 inline comments as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/index/Merge.cpp:116 // files. - More |= Static->refs(Req, [&](const Ref &O) { if (DynamicIndexFileURIs.count(O.Location.FileURI)) ho

[PATCH] D70225: [clangd] Simplify the code in XRefs

This revision was automatically updated to reflect the committed changes. ilya-biryukov marked an inline comment as done. Closed by commit rG5a9547b00709: [clangd] Simplify the code in Index::refs (authored by ilya-biryukov). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

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

ilya-biryukov marked 2 inline comments as done. ilya-biryukov added inline comments. Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:447 +tryConsumeSpelledUntil(File, EndOffset + 1, SpelledIndex).hasValue(); +(void)HitMapping; +assert(!HitMapping && "recursive mac

[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

ilya-biryukov added a comment. Thanks, LG. The only important is about testing the `foo.inc` case separately from the rest of rename tests. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:180 +const NamedDecl &getPrimaryTemplateOrThis(const NamedDecl &ND) { + if (c

[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

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

[PATCH] D70440: [Driver] Use VFS to check if sanitizer blacklists exist

ilya-biryukov created this revision. ilya-biryukov added a reviewer: gribozavr. Herald added subscribers: llvm-commits, hiraditya, mgorny. Herald added projects: clang, LLVM. This is a follow-up to 590f279c456bbde632eca8ef89a85c478f15a249

[PATCH] D69648: Add VFS support for sanitizers' blacklist' 2

ilya-biryukov added a comment. It turned out there are more places that attempt to construct `SpecialCaseList` and access corresponding file via the usual filesystem APIs. D70440 should take care of those. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D70440: [Driver] Use VFS to check if sanitizer blacklists exist

ilya-biryukov added inline comments. Herald added a subscriber: ormris. Comment at: clang/unittests/Driver/SanitizerArgsTest.cpp:100 + Contains(StrEq(std::string("-fsanitize-system-blacklist=") + + ASanBlacklist))); + // User blacklists s

[PATCH] D70440: [Driver] Use VFS to check if sanitizer blacklists exist

ilya-biryukov added inline comments. Comment at: clang/unittests/Driver/SanitizerArgsTest.cpp:71 + prepareFS(llvm::ArrayRef ExtraFiles) { + +llvm::IntrusiveRefCntPtr FS = NIT to self: remove this newline and the one before return Repository: rG LLVM Gith

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

ilya-biryukov updated this revision to Diff 230104. ilya-biryukov marked 7 inline comments as done. ilya-biryukov added a comment. - Rename ActOnSemanticError to CreateRecoveryExpr - Do not print anything for recovery expr in TextNodeDumper - canThrow(RecoveryExpr) now returns CT_Dependent - Store

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

ilya-biryukov marked 13 inline comments as done. ilya-biryukov added a comment. In D69330#1746137 , @rsmith wrote: > I would prefer that we have dedicated tests for them rather than scattering > the tests throughout the existing test suite (for example, c

[PATCH] D70441: [clangd] Speed up when building rename edit.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.h:54 +/// Use of UTF-16 may be overridden by kCurrentOffsetEncoding. +llvm::Expected byteOffset(StringRef LineCode, int LSPCharacter); + How this is different from `positionToOff

[PATCH] D70480: [clangd] Use expansion location when the ref is inside macros.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:280 (Roles & static_cast(index::SymbolRole::Reference)) && - SM.getFileID(SpellingLoc) == SM.getMainFileID()) + SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainF

[PATCH] D70481: [clangd] Fix a crash in expected types

ilya-biryukov created this revision. ilya-biryukov added a reviewer: kadircet. Herald added subscribers: usaxena95, arphaman, jkorous, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D70481 Files: clang-tools-extra/clangd/ExpectedTypes.cpp

[PATCH] D70441: [clangd] Speed up when building rename edit.

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/refactor/Rename.cpp:453 + auto Offset = [&](const Position &P) -> llvm::Expected { +Position Shifted = { +

[PATCH] D70481: [clangd] Fix a crash in expected types

This revision was automatically updated to reflect the committed changes. Closed by commit rGb5135a86e047: [clangd] Fix a crash in expected types (authored by ilya-biryukov). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70481/new/ https://reviews.l

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

ilya-biryukov updated this revision to Diff 230270. ilya-biryukov added a comment. - Do not correct typos when creating recovery expr - Simplify StmtPrinter::VisitRecoveryExpr - Add -Wno-used and remove extra expect-warning directives Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

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

ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. @rsmith, could you also take a look at D65591 ? It's really important to have the `containsError()` check in this patch that marks decls with undeduced types as invalid. Otherwise, they

[PATCH] D70480: [clangd] Use expansion location when the ref is inside macros.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:280 (Roles & static_cast(index::SymbolRole::Reference)) && - SM.getFileID(SpellingLoc) == SM.getMainFileID()) + SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainF

[PATCH] D70480: [clangd] Use expansion location when the ref is inside macros.

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:659 + TYPE(TYPE([[Foo]])) foo3; + [[CAT]](Fo, o) foo4; +} Previously we would not report any

[PATCH] D70481: [clangd] Fix a crash in expected types

ilya-biryukov added a comment. Relanded in 339502cc8abb375e3dc8b5d3e9ef35c2541ccbbd . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70481/new/ https://reviews.llvm.org/D70481

[PATCH] D70440: [Driver] Use VFS to check if sanitizer blacklists exist

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

[PATCH] D69608: [clangd] Helper for getting nested namespace qualification

ilya-biryukov added a comment. Just NITs about various comments and requests to optimize the code a bit. The code itself LG, thanks! Comment at: clang-tools-extra/clangd/AST.cpp:77 +llvm::DenseSet +getUsingNamespceDirectives(const DeclContext *DestContext, +

[PATCH] D70440: [Driver] Use VFS to check if sanitizer blacklists exist

ilya-biryukov updated this revision to Diff 230397. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Remove redundant semicolon Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70440/new/ https://reviews.llvm.org/D70440

[PATCH] D70440: [Driver] Use VFS to check if sanitizer blacklists exist

This revision was automatically updated to reflect the committed changes. Closed by commit rGba6f90685426: [Driver] Use VFS to check if sanitizer blacklists exist (authored by ilya-biryukov). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70440/new/

[PATCH] D69033: [clangd] Improve symbol qualification in DefineInline code action

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:200 +std::string Qualifier; +// FIXME: Also take using directives and namespace aliases inside function +// body into account. kadircet wrote: >

[PATCH] D70528: [NFC] ASSERT_EQ before accessing items in containers

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:1877 -EXPECT_EQ(Results.Completions.size(), 1u); +ASSERT_EQ(Results.Completions.size(), 1u); EXPECT_THAT(Results.Completions.front().CompletionTokenRange,

[PATCH] D70528: [NFC] ASSERT_EQ before accessing items in containers

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM, but please update the other diff as well! Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:712 for (const auto &D : Parsed.getDiagno

[PATCH] D69033: [clangd] Improve symbol qualification in DefineInline code action

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1730 +TEST_F(DefineInlineTest, DropCommonNamespecifiers) { + struct { NIT: capitalize the last word: `DropCommonNameSpecifiers` Repository: rG LLVM Github Mo

[PATCH] D70440: [Driver] Use VFS to check if sanitizer blacklists exist

ilya-biryukov added a comment. @teemperor Sorry for the trouble. I did not run LLVM tests, so had to revert the change. Was reverted in 9f3fdb0d7fab73083e354768eb5808597474e1b8 and relanded as aa981c1802d7353c777e399f2568e5a

[PATCH] D69033: [clangd] Improve symbol qualification in DefineInline code action

ilya-biryukov accepted this revision. ilya-biryukov added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1865 +)cpp"; + // FIXME: The last reference to cux() in body of foo should not be qualifi

[PATCH] D70528: [NFC] ASSERT_EQ before accessing items in containers

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:713 +if (D.Fixes.size() != 1) { + ADD_FAILURE() << "D.Fixes.size() != 1"; +} add `continue`! CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D69608: [clangd] Helper for getting nested namespace qualification

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

[PATCH] D70440: [Driver] Use VFS to check if sanitizer blacklists exist

ilya-biryukov added a comment. In D70440#1756262 , @thakis wrote: > Do we need to add a dep on Frontend to DriverTests here? That's a heavy > dependency (it pulls in Sema etc). If it is needed, maybe the test is in the > wrong binary? We need it solely

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

ilya-biryukov added a comment. The test also break in our internal integrate. Relying on having programs in path and ignoring the `-B` and sysroot is definitely a bad idea. I would suggest reverting this patch and re-landing after doing another round of review and fixing those issues. @thakis,

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

ilya-biryukov added a comment. In D70500#1758963 , @sunfish wrote: > If people feel using -B paths, and COMPILER_PATH, are appropriate, we can use > `GetProgramPath` itself, though note that that still does have a PATH > fallback. I'm not an expert in

[PATCH] D70480: [clangd] Use expansion location when the ref is inside macros.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:280 (Roles & static_cast(index::SymbolRole::Reference)) && - SM.getFileID(SpellingLoc) == SM.getMainFileID()) + SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainF

[PATCH] D70358: Add Cursor.get_reference_name_range to clang python binding.

ilya-biryukov added a comment. Hi Arthur, Sorry for the late response. I'm not the maintainer of the Python bindings and I'm not sure who is. Maybe ask on Discord or cfe-dev mailing list? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70358/new/ h

[PATCH] D70746: [clangd] Highlighting dependent types in more contexts

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:259 H.addToken(L.getNameLoc(), HighlightingKind::DependentType); +visitDependentTypesInQualifier(L.getQualifierLoc()); +return true; Instead of calling

[PATCH] D70740: [clangd] Find reference to template parameter in 'sizeof...' expression

ilya-biryukov added a comment. Thanks! Could you also handle this in `targetDecl` and add the corresponding test? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70740/new/ https://reviews.llvm.org/D70740 _

[PATCH] D70787: [Syntax] Build SimpleDeclaration node that groups multiple declarators

ilya-biryukov created this revision. ilya-biryukov added a reviewer: gribozavr2. Herald added a project: clang. Also remove the temporary TopLevelDeclaration node and add UnknownDeclaration to represent other unknown nodes. See the follow-up change for building more top-level declarations. Adding

[PATCH] D70788: [Syntax] A tool to dump syntax tree and token buffer

ilya-biryukov added a comment. Would want to add some tests before landing this. But this is tool is generally useful to experiment with syntax trees, so posting it early (so that it doesn't get lost) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D70788: [Syntax] A tool to dump syntax tree and token buffer

ilya-biryukov created this revision. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. ilya-biryukov added a comment. ilya-biryukov planned changes to this revision. Would want to add some tests before landing this. But this is tool is generally useful to experiment wi

[PATCH] D70787: [Syntax] Build SimpleDeclaration node that groups multiple declarators

ilya-biryukov updated this revision to Diff 231269. ilya-biryukov added a comment. - Remove obsolete FIXME - Fix a typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70787/new/ https://reviews.llvm.org/D70787 Files: clang/include/clang/Tooling/S

[PATCH] D70809: [clangd] Tweak the no-index error message for rename, 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/D70809/new/ https://reviews.llvm.org/D70809 _

[PATCH] D70811: [clangd] Don't perform rename when the refs result from index is incomplete.

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

[PATCH] D70828: [clangd] Correct the file path in Edit::replacements when generating the rename edit.

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

[PATCH] D70787: [Syntax] Build SimpleDeclaration node that groups multiple declarators

ilya-biryukov updated this revision to Diff 231504. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. Add tests with declarations inside function body Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70787/new/ https://rev

[PATCH] D70787: [Syntax] Build SimpleDeclaration node that groups multiple declarators

This revision was automatically updated to reflect the committed changes. Closed by commit rGe702bdb8598f: [Syntax] Build SimpleDeclaration node that groups multiple declarators (authored by ilya-biryukov). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D70853: [clangd] Fix a regression issue in local rename.

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/refactor/Rename.cpp:136 + else if (!DeclaredInMainFile) +// the symbol declared outside of the main file, can't b

[PATCH] D70849: [AST] Traverse the class type loc inside the member pointer type loc.

ilya-biryukov added inline comments. Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:1165 // FIXME: location of base class? // We traverse this in the type case as well, but how is it not reached through Remove this FIXME Comment

[PATCH] D70856: [Syntax] Build nodes for simple cases of top level declarations

ilya-biryukov created this revision. ilya-biryukov added a reviewer: gribozavr2. Herald added a project: clang. More complicated nodes (e.g. template declarations) will be implemented in the follow-up patches. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D70856 Files: clang

[PATCH] D70746: [clangd] Highlighting dependent types in more contexts

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Thanks! LGTM Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:275 + if (NNS->getKind() == NestedNameSpecifier::Identifier) { +H.addTok

[PATCH] D70863: [clangd] Try harder to find a plausible `clang` as argv0, particularly on Mac.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. The code LG, but my mac is at home, so I could not verify it. @kbobyrev, would you mind running this on your machine? Comment at: clang-tools-extra/clangd/Globa

[PATCH] D70863: [clangd] Try harder to find a plausible `clang` as argv0, particularly on Mac.

ilya-biryukov added a comment. Another interesting consideration: we choose to ask users to whitelists compilers we might run from `compile_commands.json` that we can. We are in a better position here, since we're not running the binaries based on user input. Technically, we could consider usin

[PATCH] D70849: [AST] Traverse the class type loc inside the member pointer type loc.

ilya-biryukov added inline comments. Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:1168 DEF_TRAVERSE_TYPELOC(MemberPointerType, { - TRY_TO(TraverseType(QualType(TL.getTypePtr()->getClass(), 0))); + if (auto *TSI = TL.getClassTInfo()) +TRY_TO(TraverseTypeLoc(TSI

[PATCH] D70740: [clangd] Find reference to template parameter in 'sizeof...' expression

ilya-biryukov added a comment. Could you add another test for `findExplicitReferences` too? Those tests are right after `targetDecl`. These two functions kinda duplicate each other a lot, but that's intentional - those two functions do a somewhat similar thing and we want to keep them in sync (

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

ilya-biryukov added a comment. I would actually try to counter the type proposal and defend the `containsErrors` bit. Here's my thinking. Not knowing that `int()` had errors inside can lead to situations with bad diagnostics. We won't be able to suppress any non-type-related error, e.g. `int *a

[PATCH] D70740: [clangd] Find reference to template parameter in 'sizeof...' expression

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

[PATCH] D71050: [clangd] More unittests for cross-file rename.

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

[PATCH] D70849: [AST] Traverse the class type loc inside the member pointer type loc.

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

[PATCH] D71029: [clangd] (take 2) Try harder to find a plausible `clang` as argv0, particularly on Mac.

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

[PATCH] D71186: Reland "[AST] Traverse the class type loc inside the member type loc.""

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/MemberPointerTypeLoc.cpp:50 + EXPECT_FALSE(Visitor.runOver(R"cpp( + class a(b(a::*)) cla

[PATCH] D71187: [clangd] Delete default arguments while moving functions out-of-line

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1938 }, + // Default args + { NIT: missing full period at the end of the comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D70480: [clangd] Use expansion location when the ref is inside macros.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:280 (Roles & static_cast(index::SymbolRole::Reference)) && - SM.getFileID(SpellingLoc) == SM.getMainFileID()) + SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainF

[PATCH] D70480: [clangd] Use expansion location when the ref is inside macros.

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

[PATCH] D70856: [Syntax] Build nodes for simple cases of top level declarations

ilya-biryukov updated this revision to Diff 232840. ilya-biryukov marked 5 inline comments as done. ilya-biryukov added a comment. - Remove the duplicate test - Add 'message' to the node for static_assert - Handle nested namespace definitions, add them to the tests Repository: rG LLVM Github M

[PATCH] D70856: [Syntax] Build nodes for simple cases of top level declarations

ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:346 +/// static_assert(, ) +/// static_assert() +class StaticAssertDeclaration final : public Declaration { gribozavr2 wrote: > Why no semicolon, here and in other newly-a

[PATCH] D70856: [Syntax] Build nodes for simple cases of top level declarations

ilya-biryukov updated this revision to Diff 233004. ilya-biryukov added a comment. - Do not call Builder.getRange() twice for namespaces Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70856/new/ https://reviews.llvm.org/D70856 Files: clang/includ

[PATCH] D71300: [clangd] Deduplicate refs from index for cross-file rename.

ilya-biryukov added a comment. The fix itself LGTM. Just a few NITs and this is good to go. Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:34 +// Covnert a Range to a Ref. +std::pair toRef(const clangd::Range &Range, +

[PATCH] D71300: [clangd] Deduplicate refs from index for cross-file rename.

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

[PATCH] D71329: [CodeComplete] Fix a crash in preferred type and signature help

ilya-biryukov created this revision. ilya-biryukov added a reviewer: kadircet. Herald added a project: clang. sammccall accepted this revision. This revision is now accepted and ready to land. Null type pointers could be dereferenced in some cases. Repository: rG LLVM Github Monorepo https://

[PATCH] D71329: [CodeComplete] Fix a crash in preferred type and signature help

ilya-biryukov updated this revision to Diff 233268. ilya-biryukov added a comment. - Rename the test, tweak assert comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71329/new/ https://reviews.llvm.org/D71329 Files: clang/lib/Parse/ParseExprC

[PATCH] D71329: [CodeComplete] Fix a crash in preferred type and signature help

This revision was automatically updated to reflect the committed changes. Closed by commit rGf7c8ace4a52a: [CodeComplete] Fix a crash in preferred type and signature help (authored by ilya-biryukov). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7132

[PATCH] D70856: [Syntax] Build nodes for simple cases of top level declarations

This revision was automatically updated to reflect the committed changes. Closed by commit rGbe14a22b47e5: [Syntax] Build nodes for simple cases of top level declarations (authored by ilya-biryukov). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7085

[PATCH] D70856: [Syntax] Build nodes for simple cases of top level declarations

ilya-biryukov added a comment. Landing as is, but happy to address any comments in the follow-up patches. @gribozavr2, please feel free to leave any suggestions in the review comments, despite it being closed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

ilya-biryukov added inline comments. Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:270 + for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens)) +if (Tok.kind() == tok::identifier) + return &Tok; NIT: add braces around `if` statement Rep

[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

ilya-biryukov added inline comments. Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:260 + All, [&](const syntax::Token &Tok) { return Tok.location() < Loc; }); + bool AcceptRight = Right != All.end() && !(Loc < Right->location()); + bool AcceptLeft = Right != All.begin()

<    24   25   26   27   28   29   30   31   32   33   >