[PATCH] D54693: [clangd] Store source file hash in IndexFile{In,Out}

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/index/Serialization.cpp:368 +Reader Hash(Chunks.lookup("hash")); +llvm::StringRef Digest = Hash.consume(20); +Result.Digest.emplace()

[PATCH] D54475: [clangd] Allow observation of changes to global CDBs.

sammccall updated this revision to Diff 174633. sammccall marked 2 inline comments as done. sammccall added a comment. Herald added a subscriber: mgorny. Address comments, add test for Event machinery. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54475 Files: clangd/Functio

[PATCH] D54475: [clangd] Allow observation of changes to global CDBs.

sammccall added inline comments. Comment at: clangd/Function.h:147 +private: + static_assert(std::is_same::type, T>::value, +"use a plain type: event values are always passed by const&"); ilya-biryukov wrote: > NIT: Maybe move this static_assert

[PATCH] D54746: [clangd] Respect task cancellation in TUScheduler.

sammccall created this revision. sammccall added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ioeric, javed.absar. - Reads are never executed if canceled before ready-to run. In practice, we finalize cancelled reads eagerly and out-of-or

[PATCH] D54475: [clangd] Allow observation of changes to global CDBs.

This revision was automatically updated to reflect the committed changes. Closed by commit rL347297: [clangd] Allow observation of changes to global CDBs. (authored by sammccall, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D54475 Files

[PATCH] D54694: [clangd] Replay preamble #includes to clang-tidy checks.

This revision was automatically updated to reflect the committed changes. Closed by commit rL347298: [clangd] Replay preamble #includes to clang-tidy checks. (authored by sammccall, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D54694 Fi

[PATCH] D54694: [clangd] Replay preamble #includes to clang-tidy checks.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE347298: [clangd] Replay preamble #includes to clang-tidy checks. (authored by sammccall, committed by ). Changed prior to commit: https://reviews.llvm.org/D54694?vs=174621&id=174742#toc Repository:

[PATCH] D52273: [clangd] Initial implementation of expected types

sammccall added inline comments. Comment at: clangd/ExpectedTypes.h:32 +/// this allows the representation to be stored in the index and compared with +/// types coming from a different AST later. +class OpaqueType { ilya-biryukov wrote: > sammccall wrote: > > Do

[PATCH] D52274: [clangd] Collect and store expected types in the index

sammccall added inline comments. Comment at: clangd/index/Index.cpp:118 +static void own(Symbol &S, UniqueStringSaver &Strings, +BumpPtrAllocator &Arena) { visitStrings(S, [&](StringRef &V) { V = Strings.save(V); }); why these changes? ==

[PATCH] D52276: [clangd] Add type boosting in code completion

sammccall added inline comments. Comment at: clangd/Quality.h:98 + /// Whether the item matches the type expected in the completion context. + bool TypeMatchesPreferred = false; /// FIXME: unify with index proximity score - signals should be you've inserted

[PATCH] D52276: [clangd] Add type boosting in code completion

sammccall added inline comments. Comment at: clangd/Quality.cpp:373 + if (TypeMatchesPreferred) +Score *= 2.0; + is 2 really enough? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52276 ___ cf

[PATCH] D54760: [clangd] Cleanup: make the diags callback global in TUScheduler

sammccall added a comment. Nice! The biggest suggestion I have is merging the callback into ASTCallbacks. That's awkward in initialization (`makeUpdateCallbacks` probably needs to be called unconditionally with a maybe-null pointer) but it seems like a natural grouping in all the places it gets

[PATCH] D52311: [clangd] Add support for hierarchical documentSymbol

sammccall added a comment. Very nice! Mostly just a few style/structure nits. Comment at: clangd/AST.cpp:69 +std::string printName(const NamedDecl &ND) { + const NamedDecl *NameSource = &ND; + std::string Name = llvm::to_string(NameSource->getDeclName()); jus

[PATCH] D53900: [CodeComplete] Penalize inherited ObjC properties for auto-completion

This revision was automatically updated to reflect the committed changes. Closed by commit rC347352: [CodeComplete] Penalize inherited ObjC properties for auto-completion (authored by sammccall, committed by ). Changed prior to commit: https://reviews.llvm.org/D53900?vs=171799&id=174838#toc Re

[PATCH] D54799: [clangd][WIP] textDocument/CursorInfo method

sammccall added a comment. Thanks for sending this! Broadly looks good, a few details to work out. I think the biggest one is multiple symbols which you've flagged. > I'd like to ask for early feedback - what's still missing is relevant client > capability. I'm actually not 100% sure that's ne

[PATCH] D54760: [clangd] Cleanup: make the diags callback global in TUScheduler

sammccall added a comment. Just the context thing. Rest is some optional simplifications. Comment at: clangd/ClangdServer.cpp:77 +FileIndex *FIndex, +llvm::unique_function)> OnDiags) { + using DiagsCallback = decltype(OnDiags); hmm, the double-indirect

[PATCH] D54746: [clangd] Respect task cancellation in TUScheduler.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE347450: [clangd] Respect task cancellation in TUScheduler. (authored by sammccall, committed by ). Changed prior to commit: https://reviews.llvm.org/D54746?vs=174738&id=175025#toc Repository: rL LL

[PATCH] D54746: [clangd] Respect task cancellation in TUScheduler.

This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rL347450: [clangd] Respect task cancellation in TUScheduler. (authored by sammccall, committed by ). Herald added a subscriber: llvm-commits. Changed prior to com

[PATCH] D54800: [clangd] Cleanup: stop passing around list of supported URI schemes.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Awesome! Comment at: clangd/URI.cpp:202 +("Not a valid absolute path: " + AbsolutePath).str().c_str()); + for (auto I = URISchemeRegistry::begin(), E = URISche

[PATCH] D54829: [clangd] Cleanup: make diagnostics callbacks from TUScheduler non-racy

sammccall added inline comments. Comment at: clangd/TUScheduler.cpp:424 if (*AST) { OnUpdated((*AST)->getDiagnostics()); trace::Span Span("Running main AST callback"); as discussed offline, this doesn't guarantee we're not going to send OnUpdat

[PATCH] D54817: [clangd] Put direct headers into srcs section.

sammccall added a comment. Going to leave awkward comments suggesting we expand the scope a bit. Full disclosure: file dependency information is something that's come up as useful in lots of contexts. Comment at: clangd/index/Serialization.cpp:469 + // There's no point in p

[PATCH] D54829: [clangd] Cleanup: make diagnostics callbacks from TUScheduler non-racy

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/ClangdServer.h:132 /// Remove \p File from list of tracked files, schedule a request to free - /// resources associated with it. + /// resou

[PATCH] D52311: [clangd] Add support for hierarchical documentSymbol

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: unittests/clangd/FindSymbolsTests.cpp:442 +SymNameRange(Main.range("decl"), + AllOf(Wit

[PATCH] D52273: [clangd] Initial implementation of expected types

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/ExpectedTypes.cpp:8 + +namespace clang { +namespace clangd { nit: using namespace llvm (until/unless we switch other files) ==

[PATCH] D54760: [clangd] Cleanup: make the diags callback global in TUScheduler

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Everything looks so much better... Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54760 ___ cfe-commits mailing list cfe-c

[PATCH] D54796: [clangd] **Prototype**: C++ API for emitting file status

sammccall added inline comments. Comment at: clangd/ClangdLSPServer.cpp:787 + notify("window/showMessage", + json::Object { + {"uri", URI}, we should have a protocol model object (in Protocol.h) for the file status updates. I think it should

[PATCH] D54799: [clangd][WIP] textDocument/CursorInfo method

sammccall added a subscriber: hokein. sammccall added a comment. In https://reviews.llvm.org/D54799#1306488, @jkorous wrote: > >> - conditional return in `getCursorInfo` - Should we return for example > >> data with empty `USR`? > > > > Please return a symbol unless it has no SymbolID (we don't

[PATCH] D52276: [clangd] Add type boosting in code completion

sammccall added inline comments. Comment at: clangd/CodeComplete.cpp:1307 getCompletionKindString(Recorder->CCContext.getKind())); log("Code complete: sema context {0}, query scopes [{1}] (AnyScope={2})", getCompletionKindString(Recorder->CCCo

[PATCH] D54781: [clangd] Add 'Switch header/source' command in clangd-vscode

sammccall added a comment. This is great! I'm slightly nervous - the way we've extended the protocol with `textDocument/switchSourceHeader` is pretty hard to extend, itself (since the response is a string directly). This is only tangentially related to this patch, the same issue exists with Th

[PATCH] D54865: [clangd] Auto-index watches global CDB for changes.

sammccall created this revision. sammccall added reviewers: ioeric, kadircet. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay. Instead of receiving compilation commands, auto-index is triggered by just filenames to reindex, and gets commands from the global comp DB internally. Th

[PATCH] D54799: [clangd][WIP] textDocument/CursorInfo method

sammccall added a comment. Thanks, this looks good! Just nits, and please do port most of the test cases to unit tests. Comment at: clangd/ClangdServer.cpp:529 + + WorkScheduler.runWithAST("CursorInfo", File, Bind(Action, std::move(CB))); +} nit: SymbolInfo

[PATCH] D54865: [clangd] Auto-index watches global CDB for changes.

sammccall marked 3 inline comments as done. sammccall added inline comments. Comment at: clangd/index/Background.cpp:130 + if (auto Cmd = CDB.getCompileCommand(File, &Project)) { +auto *Storage = IndexStorageFactory(Project.SourceRoot); +enqueueTask(Bind( ---

[PATCH] D54865: [clangd] Auto-index watches global CDB for changes.

sammccall updated this revision to Diff 175208. sammccall marked an inline comment as done. sammccall added a comment. If the CDB dir is unknown, don't try to write shards to disk. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54865/new/ https://rev

[PATCH] D54865: [clangd] Auto-index watches global CDB for changes.

This revision was automatically updated to reflect the committed changes. Closed by commit rL347538: [clangd] Auto-index watches global CDB for changes. (authored by sammccall, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION https://revi

[PATCH] D54799: [clangd][WIP] textDocument/SymbolInfo method

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. LG from my side. If you have unit tests in the next couple of days, happy to take a look, otherwise go ahead once Alex/Ben are happy. (I'm at work today/tomorrow and then I'm going to be

[PATCH] D54894: [clangd] Enable auto-index behind a flag.

sammccall created this revision. sammccall added a reviewer: kadircet. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov. Ownership and configuration: The auto-index (background index) is maintained by ClangdServer, like Dynamic. (This means ClangdServer wil

[PATCH] D52274: [clangd] Collect and store expected types in the index

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/index/Index.h:283 + /// Type of the symbol, used for scoring purposes. + llvm::StringRef Type; either call this OpaqueType o

[PATCH] D52276: [clangd] Add type boosting in code completion

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. You may want to add a FIXME in SymbolIndex to include opaque type in fuzzyfind request. Comment at: clangd/CodeComplete.cpp:1504 +if (PreferredType) + Relevan

[PATCH] D54894: [clangd] Enable auto-index behind a flag.

sammccall updated this revision to Diff 175249. sammccall marked an inline comment as done. sammccall added a comment. Make blockUntilIdleForTest() accept a timeout, update comment. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54894/new/ https://re

[PATCH] D54894: [clangd] Enable auto-index behind a flag.

sammccall marked an inline comment as done. sammccall added inline comments. Comment at: test/clangd/background-index.test:16 +# Test that the index is writing files in the expected location. +# RUN: ls %t/.clangd-index/foo.cpp.*.idx + kadircet wrote: > kadircet

[PATCH] D54894: [clangd] Enable auto-index behind a flag.

This revision was automatically updated to reflect the committed changes. Closed by commit rL347567: [clangd] Enable auto-index behind a flag. (authored by sammccall, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D54845: [clangd] Canonicalize file path in URIForFile.

sammccall added inline comments. Comment at: clangd/Protocol.h:73 + + /// \p AbsPath is resolved to a canonical path corresponding to its URI. + URIForFile(llvm::StringRef AbsPath, llvm::StringRef HintPath = ""); We need to document the APIs and motivation, the

[PATCH] D54796: [clangd] C++ API for emitting file status

sammccall added inline comments. Comment at: clangd/ClangdLSPServer.cpp:787 +void ClangdLSPServer::onFileUpdated(const FileStatus &FStatus) { + notify("window/showMessage", FStatus); +} notifying via `showMessage` looks sensible at first glance as a fallback. La

[PATCH] D54938: [clangd] Prevent thread starvation in tests on loaded systems.

sammccall created this revision. sammccall added a reviewer: kadircet. Herald added subscribers: cfe-commits, jfb, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov. Background index deliberately runs low-priority, but for tests this may stop them making progress. Repository: rCTE Clang Tools

[PATCH] D54938: [clangd] Prevent thread starvation in tests on loaded systems.

This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rCTE347655: [clangd] Prevent thread starvation in tests on loaded systems. (authored by sammccall, committed by ). Changed prior to commit: https://reviews.llvm

[PATCH] D54817: [clangd] Put direct headers into srcs section.

sammccall added a comment. structure and serialization stuff looks great! still some questions around the auto-index logic and organization of the extraction code, let's chat offline. May make sense to split. Comment at: clangd/index/Background.cpp:329 +llvm::StringMap +Includ

[PATCH] D54799: [clangd][WIP] textDocument/SymbolInfo method

sammccall accepted this revision. sammccall marked an inline comment as done. sammccall added a comment. Tests look great, thanks! Comment at: clangd/ClangdServer.cpp:529 + + WorkScheduler.runWithAST("CursorInfo", File, Bind(Action, std::move(CB))); +} jkorous

[PATCH] D54817: [clangd] Put direct headers into srcs section.

sammccall added a comment. To summarize offline discussion: I think we need to split this patch up, as the scope is growing. (In a really useful direction I think!) The first patch should add the include graph data structure. (I think the right place for it is probably Headers.h, as we want to

[PATCH] D54845: [clangd] Canonicalize file path in URIForFile.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks! Mostly just places that should be updated HintPath -> TUPath. Changing the whole codebase doesn't seem important, but in places that are touched... Comment at:

[PATCH] D54817: [clangd] Put direct headers into srcs section.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Only really significant thing is I think the name "build graph" is confusing, and we should specifically mention includes. Rest is just nits to take or leave... Commen

[PATCH] D54737: [clang-tidy] Add the abseil-duration-comparison check

sammccall added inline comments. Comment at: clang-tidy/abseil/DurationComparisonCheck.cpp:25 +static llvm::Optional getScaleForInverse(llvm::StringRef Name) { + static const std::unordered_map ScaleMap( + {{"ToDoubleHours", DurationScale::Hours}, hwright w

[PATCH] D54737: [clang-tidy] Add the abseil-duration-comparison check

sammccall added inline comments. Comment at: clang-tidy/abseil/DurationComparisonCheck.cpp:25 +static llvm::Optional getScaleForInverse(llvm::StringRef Name) { + static const std::unordered_map ScaleMap( + {{"ToDoubleHours", DurationScale::Hours}, aaron.bal

[PATCH] D54737: [clang-tidy] Add the abseil-duration-comparison check

sammccall added inline comments. Comment at: clang-tidy/abseil/DurationComparisonCheck.cpp:25 +static llvm::Optional getScaleForInverse(llvm::StringRef Name) { + static const std::unordered_map ScaleMap( + {{"ToDoubleHours", DurationScale::Hours}, aaron.bal

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

sammccall added a comment. In D54945#1309467 , @JonasToth wrote: > I like the overview, maybe a link to `clangd` here might help, as there is > currently a lot of effort of integrating `clang-tidy` into it. (@sammccall > WDYT?) This would be great! The

[PATCH] D50147: clang-format: support external styles

sammccall added a comment. In D50147#1309880 , @Typz wrote: > ping? Sorry for losing track of this. I think `-style=` is a logical extension of the current options. I'm less sure of supporting it in BasedOnStyle, but happy to go either way. Referring

[PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)

sammccall added a comment. Hi Nathan, Really sorry about the delay here. I'm actually out on leave at the moment, and didn't get things wrapped up before I went. (I'm 1 week into 4 week parental leave, and got sick the week I was supposed to be handing things off). @kadircet, can you finish the

[PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)

sammccall added a comment. Thanks for picking this up... just wanted to leave one last comment as I'd been thinking about the recursive template case too. Comment at: clang-tools-extra/clangd/XRefs.cpp:885 +static Optional +getTypeHierarchy(const CXXRecordDecl &CXXRD, ASTConte

[PATCH] D59814: [Testing] Move clangd::Annotations to llvm testing support

sammccall added a comment. This probably needs tests if we're lifting it into `llvm`. Sorry there aren't any today :-( BTW the other similar lib in clang I'm aware of is https://reviews.llvm.org/diffusion/L/browse/cfe/trunk/tools/clang-refactor/TestSupport.h It makes different tradeoffs. We ta

[PATCH] D58275: [clangd] Support utf-8 offsets (rather than utf-16) as a protocol extension

This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rCTE357102: [clangd] Support utf-8 offsets (rather than utf-16) as a protocol extension (authored by sammccall, committed by ). Changed prior to commit: https:/

[PATCH] D59927: [clangd] Support UTF-32 (i.e. codepoint) offsets.

sammccall created this revision. sammccall added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ioeric. Herald added a project: clang. (Changes to UTF-8/UTF-16 here are NFC, moving things around to make the cases more symmetrical) Reposit

[PATCH] D59927: [clangd] Support UTF-32 (i.e. codepoint) offsets.

sammccall updated this revision to Diff 192628. sammccall added a comment. Fix capitalization. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59927/new/ https://reviews.llvm.org/D59927 Files: clangd/Protocol.cpp clangd/Protocol.h clangd/Source

[PATCH] D59927: [clangd] Support UTF-32 (i.e. codepoint) offsets.

sammccall marked an inline comment as done. sammccall added inline comments. Comment at: unittests/clangd/SourceCodeTests.cpp:164 + llvm::Failed()); // out of range + // middle line + EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, -1)), --

[PATCH] D59927: [clangd] Support UTF-32 (i.e. codepoint) offsets.

This revision was automatically updated to reflect the committed changes. Closed by commit rL357173: [clangd] Support UTF-32 (i.e. codepoint) offsets. (authored by sammccall, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINC

[PATCH] D59935: Disable warnings when indexing as a standalone action.

sammccall created this revision. sammccall added a reviewer: hokein. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, ioeric, ilya-biryukov. Herald added a project: clang. - we don't record the warnings at all - even when -Werror is set, we don't want to stop indexing for a war

[PATCH] D59935: Disable warnings when indexing as a standalone action.

sammccall updated this revision to Diff 192650. sammccall added a comment. Herald added a subscriber: jdoerfert. Add tests, fix setting the flag too late. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59935/new/ https://reviews.llvm.org/D59935 File

[PATCH] D59935: Disable warnings when indexing as a standalone action.

sammccall updated this revision to Diff 192651. sammccall added a comment. fix baseline Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59935/new/ https://reviews.llvm.org/D59935 Files: clangd/index/IndexAction.cpp unittests/clangd/IndexActionTes

[PATCH] D59935: Disable warnings when indexing as a standalone action.

sammccall marked an inline comment as done. sammccall added inline comments. Comment at: clangd/index/IndexAction.cpp:140 +// Avoids some analyses too. +CI.getDiagnosticOpts().IgnoreWarnings = true; return WrapperFrontendAction::BeginInvocation(CI);

[PATCH] D59935: Disable warnings when indexing as a standalone action.

This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rL357186: Disable warnings when indexing as a standalone action. (authored by sammccall, committed by ). Herald added a project: LLVM. Herald added a subscriber: l

[PATCH] D59407: [clangd] Add RelationSlab

sammccall added a comment. (Sorry to arrive late at this discussion, I'm just back from leave. I have some suggestions and would appreciate your thoughts, but if simply this feels too much like going around in circles I'm happy to work out how we can address this after this patch lands instead.

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

sammccall added a comment. incomplete, haven't reviewed token collector Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:72 + /// macro or a name, arguments and parentheses of a function-like macro. + llvm::ArrayRef macroTokens(const TokenBuffer &B) const; + ///

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

sammccall added inline comments. Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:1 +//===- TokenBuffer.h - store tokens of preprocessed files -*- C++ -*-=// +// ilya-biryukov wrote: > sammccall wrote: > > are you sure TokenBuffer is the cent

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

sammccall added a comment. Not sure what the implications of design changes are, so will defer reviewing details of tokencollector (which generally looks good, but is of course highly coupled to lexer/pp) and range mapping (which I suspect could be simplified, but depends heavily on the model).

[PATCH] D60194: [Tooling] add a Heuristic field indicating that a CompileCommand was guessed.

sammccall created this revision. sammccall added reviewers: ilya-biryukov, klimek. Herald added subscribers: cfe-commits, kadircet, ioeric. Herald added a project: clang. Use cases: - a tool that dumps the heuristic used for each header in a project can be used to evaluate changes to the heurist

[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

sammccall added a comment. Got here trying to understand how D60126 works. It seems there's two fairly independent changes here: - the one described, allow actions to run without a preamble - defer the blocking getCompileCommand() call until we're building the

[PATCH] D60258: [CodeComplete] Fix crash when completing ObjC block parameter with a broken type

sammccall created this revision. sammccall added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, arphaman, kristof.beyls, javed.absar. Herald added a project: clang. The fix isn't great, but it's hard to fix properly because the completion code sensibly uses ParmVarDecl to repres

[PATCH] D60258: [CodeComplete] Fix crash when completing ObjC block parameter with a broken type

This revision was automatically updated to reflect the committed changes. Closed by commit rL357686: [CodeComplete] Fix crash when completing ObjC block parameter with a broken type (authored by sammccall, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repo

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

sammccall added inline comments. Comment at: clangd/CodeComplete.cpp:1136 +std::unique_ptr +indexIdentifiers(llvm::StringRef FileName, llvm::StringRef Content, + const format::FormatStyle &Style) { as discussed offline, I love the lexer approach b

[PATCH] D60267: [clangd] Support relatedInformation in diagnostics.

sammccall created this revision. sammccall added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ioeric. Herald added a project: clang. We already have the structure internally, we just need to expose it. Repository: rCTE Clang Tools Ext

[PATCH] D60308: Leave alone the semicolon after the MacroBlockBegin name

sammccall added a comment. The documentation examples of MacroBlockBegin are: /// A regular expression matching macros that start a block. /// \code ///# With: ///MacroBlockBegin: "^NS_MAP_BEGIN|\ ///NS_TABLE_HEAD$" ///MacroBlockEnd: "^\ ///NS_MAP_END|\ ///

[PATCH] D60316: [clangd] Include insertion: require header guards, drop other heuristics, treat .def like .inc.

sammccall created this revision. sammccall added a reviewer: ioeric. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. We do have some reports of include insertion behaving badly in some codebases. Requiring header guards bo

[PATCH] D60316: [clangd] Include insertion: require header guards, drop other heuristics, treat .def like .inc.

sammccall marked an inline comment as done. sammccall added inline comments. Comment at: unittests/clangd/FileIndexTests.cpp:214 TEST(FileIndexTest, HasSystemHeaderMappingsInPreamble) { + TestTU TU; I suspect we can replace most of these tests with TestTU - h

[PATCH] D60194: [Tooling] add a Heuristic field indicating that a CompileCommand was guessed.

sammccall added a comment. In D60194#1454633 , @ilya-biryukov wrote: > Yet, having the new field in the `CompileCommand` feels wrong, especially in > combination with a fact that `CompilationDatabase` returns a vector of > compile commands rather just a

[PATCH] D60194: [Tooling] add a Heuristic field indicating that a CompileCommand was guessed.

This revision was automatically updated to reflect the committed changes. Closed by commit rL357770: [Tooling] add a Heuristic field indicating that a CompileCommand was guessed. (authored by sammccall, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Reposit

[PATCH] D60323: [clangd] Include compile command inference in fileStatus API

sammccall created this revision. sammccall added a reviewer: hokein. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ioeric, javed.absar, ilya-biryukov. Herald added a project: clang. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D60323 Files: clan

[PATCH] D59407: [clangd] Add RelationSlab

sammccall added a comment. In D59407#1447070 , @nridge wrote: > @sammccall, thank you for having a look at this. > > I have no objection to revising the data model if there's agreement on a > better one. > > In D59407#1446464

[PATCH] D59756: [clangd] Support dependent bases in type hierarchy

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. LG, though I think your patch might be based on top of some uncommitted changes. I noticed template names rather than type names are shown (not new in this patch) - we should fix that I

[PATCH] D59756: [clangd] Support dependent bases in type hierarchy

sammccall accepted this revision. sammccall added inline comments. Comment at: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp:405 + *Result, + AllOf(WithName("S"), WithKind(SymbolKind::Struct), +Parents(AllOf(WithName("S"), WithKind(SymbolKind::St

[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

sammccall added a comment. API looks good. Implementation looks like it can be simplified a bit, unless I'm missing something. Comment at: clangd/CodeComplete.h:117 + /// (e.g. preamble is still being built). + bool AllowFallbackMode = false; }; nit: "mode"

[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

sammccall added inline comments. Comment at: unittests/clangd/ClangdTests.cpp:1082 + EXPECT_EQ(Res.Context, CodeCompletionContext::CCC_Recovery); + ASSERT_TRUE(Server.blockUntilIdleForTest()); + EXPECT_THAT(cantFail(runCodeComplete(Server, FooCpp, Code.point(), ---

[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59811/new/ https://reviews.llvm.org/D59811 ___

[PATCH] D60409: [clangd] Add -header-insertion=never flag to disable include insertion in code completion

sammccall created this revision. sammccall added a reviewer: ioeric. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. One clear use case: use with an editor that reacts poorly to edits above the cursor. Repository: rCT

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

sammccall created this revision. sammccall added a reviewer: ioeric. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Intent is to use the heuristically-parsed scope in cases where we get bogus results from sema, such as in

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

sammccall updated this revision to Diff 194462. sammccall added a comment. Handle edge-case correctly. Add test for leading :: case. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60500/new/ https://reviews.llvm.org/D60500 Files: clangd/CodeC

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

sammccall updated this revision to Diff 194463. sammccall added a comment. Fix terrible variable name. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60500/new/ https://reviews.llvm.org/D60500 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h

[PATCH] D60503: [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

sammccall created this revision. sammccall added a reviewer: ioeric. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. There are cases where Sema can't tell that "foo" in foo::Bar is a namespace qualifier, like in incomplete

[PATCH] D60503: [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

sammccall updated this revision to Diff 194486. sammccall added a comment. Update comment Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60503/new/ https://reviews.llvm.org/D60503 Files: clangd/CodeComplete.cpp unittests/clangd/CodeCompleteTests

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

sammccall marked 2 inline comments as done. sammccall added inline comments. Comment at: clangd/CodeComplete.cpp:1542 + assert(Offset <= Content.size()); + StringRef Suffix = Content.take_front(Offset); + CompletionPrefix Result; ioeric wrote: > `Suffix` is ac

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rL358074: [clangd] Refactor speculateCompletionFilter and also extract scope. (authored by sammccall, committed by ). Herald added a project: LLVM. Herald added a

[PATCH] D60409: [clangd] Add -header-insertion=never flag to disable include insertion in code completion

sammccall accepted this revision. sammccall marked 2 inline comments as done. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/CodeComplete.cpp:1176 // This is available after Sema has run. - llvm::Optional Inserter; // Ava

[PATCH] D60503: [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

sammccall marked an inline comment as done. sammccall added inline comments. Comment at: clangd/CodeComplete.cpp:545 const CodeCompleteOptions &Opts) { - auto GetAllAccessibleScopes = [](CodeCompletionContext &CCContext) { -SpecifiedScope Info; -for (auto

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