[PATCH] D128621: [clangd] Do not try to use $0 as a placeholder in completion snippets

sammccall accepted this revision. sammccall added a comment. Thank you! Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:195 ++SnippetArg; - *Snippet += - "${" + - std::to_string(SnippetArg == CursorSnippetArg ? 0 : SnippetArg) +

[PATCH] D130004: [pseudo] Add `clang-pseudo -html-forest=`, an HTML forest browser

sammccall created this revision. sammccall added a reviewer: hokein. Herald added a subscriber: mgorny. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, alextsao1999. Herald added a project: clang-tools-extra. It generates a

[PATCH] D129359: [pseudo] Generate an enum type for identifying grammar rules.

sammccall added inline comments. Comment at: clang-tools-extra/pseudo/lib/grammar/Grammar.cpp:48 +std::string Grammar::mangleSymbol(SymbolID SID) const { + static const char *const TokNames[] = { hokein wrote: > sammccall wrote: > > I'm not sure exposing

[PATCH] D129648: Use pseudo parser for folding ranges

sammccall added inline comments. Comment at: clang-tools-extra/clangd/CMakeLists.txt:149 +target_include_directories(clangDaemon PUBLIC + ${CMAKE_CURRENT_SOURCE_DIR}/../pseudo/include +) It should rather be up to the clangPseudo target to specify its include

[PATCH] D129797: [clang-tidy] Reduce the dependencies for the "make-confusable-table" tool

sammccall added a comment. This matches what I did for clang-pseudo-gen. It looks reasonable to me, but I'd love to get a third opinion... (If none in a couple of days, happy to stamp it) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129797/new/

[PATCH] D129642: [Sema] Tweak diagnostic logic so suppress-in-hedaer logic works in tools too.

sammccall added a comment. In D129642#3648197 , @sammccall wrote: > It's possible we should *only* be checking the IsHeaderFile flag here and > drop all the sourcemanager stuff, but I'm not sure of the implications - > thoughts appreciated. As

[PATCH] D129683: [Sema] Move Diags.isIgnored() checks off hot paths, it's not free. NFC

sammccall created this revision. sammccall added a reviewer: kadircet. Herald added a subscriber: usaxena95. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang. This speeds up clangd's

[PATCH] D129595: [clangd] Enable designator inlay hints by default.

This revision was automatically updated to reflect the committed changes. Closed by commit rG6b50f2bbdbf4: [clangd] Enable designator inlay hints by default. (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129595/new/

[PATCH] D129642: [Sema] Tweak diagnostic logic so suppress-in-hedaer logic works in tools too.

sammccall added a comment. It's possible we should *only* be checking the IsHeaderFile flag here and drop all the sourcemanager stuff, but I'm not sure of the implications - thoughts appreciated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D129642: [Sema] Tweak diagnostic logic so suppress-in-hedaer logic works in tools too.

sammccall updated this revision to Diff 444253. sammccall edited the summary of this revision. sammccall added a comment. Herald added a subscriber: ilya-biryukov. add bug link to description Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D129642: [Sema] Tweak diagnostic logic so suppress-in-hedaer logic works in tools too.

sammccall created this revision. sammccall added a reviewer: kadircet. Herald added subscribers: usaxena95, arphaman. Herald added a project: All. sammccall requested review of this revision. Herald added projects: clang, clang-tools-extra. Herald added a subscriber: cfe-commits. Certain idioms

[PATCH] D129579: [clangd] Remove `allCommitCharacters`

sammccall added a comment. In D129579#3647834 , @kadircet wrote: > sorry for being late to the party, but reading the spec >

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

sammccall added a comment. Sorry, didn't realize this was waiting on me! I like the feature and the threshold seems like a good idea. People might find it inconsistent, but we'll have to see. I like placing it after the macro definition, I think the cases where expansion is/isn't shown feel

[PATCH] D129595: [clangd] Enable designator inlay hints by default.

sammccall created this revision. sammccall added a reviewer: kadircet. Herald added subscribers: usaxena95, arphaman. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra.

[PATCH] D129579: [clangd] Remove `allCommitCharacters`

This revision was automatically updated to reflect the committed changes. Closed by commit rG93cd159ca9d3: [clangd] Remove `allCommitCharacters` (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D129579?vs=444015=444051#toc Repository: rG LLVM Github Monorepo

[PATCH] D129579: [clangd] Remove `allCommitCharacters`

sammccall created this revision. sammccall added reviewers: kadircet, hokein. Herald added subscribers: usaxena95, arphaman. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project:

[PATCH] D128621: [clangd] Do not try to use $0 as a placeholder in completion snippets

sammccall added a comment. In D128621#3645123 , @ilya-biryukov wrote: > Another alternative that I think should give the best UX is to replace > `${0:named}` with `$0`. > The items will look different, but will behave identically to the old > behavior

[PATCH] D124447: [clang-tidy] Add infrastructure support for running on project-level information

sammccall added a comment. In D124447#3608747 , @aaron.ballman wrote: > In general, I think this is looking pretty close to good. Whether clang-tidy > should get this functionality in this form or not is a bit less clear to me. > *I* think it's

[PATCH] D128411: [syntax] Introduce a BaseToken class.

sammccall added inline comments. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:370 + TreeBuilder(syntax::Arena ) + : Arena(Arena), STM(cast(Arena.getTokenManager())), +Pending(Arena, STM.tokenBuffer()) { hokein wrote: > sammccall wrote: > >

[PATCH] D88299: [clang-format] Add MacroUnexpander.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Herald added a project: All. OK, I think we've reached the point where: - the impact is very clear, this solves a whole class of clang-format's biggest problems - the idea clearly works

[PATCH] D128411: [syntax] Introduce a BaseToken class.

sammccall added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:24 #include "clang/Basic/TokenKinds.h" #include "clang/Lex/Token.h" I think Token and TokenKinds are also no longer needed? Comment at:

[PATCH] D128826: Go-to-type on smart_ptr now also shows Foo

sammccall added a comment. In D128826#3641093 , @tom-anders wrote: > @sammccall I think you can merge this for me now (and also > https://reviews.llvm.org/D128202) Done, sorry for the delay. FWIW I think you should be able to get commit access

[PATCH] D128202: [clangd] Include "final" when printing class declaration

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9f57b65a2728: [clangd] Include final when printing class declaration (authored by tom-anders, committed by sammccall). Repository: rG LLVM Github

[PATCH] D128826: Go-to-type on smart_ptr now also shows Foo

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcd85d9aeef9b: Go-to-type on smart_ptrFoo now also shows Foo (authored by tom-anders, committed by sammccall). Repository: rG LLVM Github Monorepo

[PATCH] D129359: [pseudo] Generate an enum type for identifying grammar rules.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h:172 + // terminal `,` becomes `comma`; + // terminal `INT` becomes `int`; +

[PATCH] D129100: [clangd] Support external throttler for preamble builds

sammccall added inline comments. Comment at: clang-tools-extra/clangd/TUScheduler.cpp:499 // Build the preamble and let the waiters know about it. build(std::move(*CurrentReq)); } kadircet wrote: > regarding the flakiness, what about

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

sammccall added inline comments. Comment at: clang-tools-extra/pseudo/gen/Main.cpp:114 + const clang::pseudo::Rule = G.table().Rules[RID]; + // lhs$$rhs$rhs$rhs + std::string EnumName = symbolName(R.Target, G) + "$"; hokein wrote: > sammccall

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

sammccall added inline comments. Comment at: clang-tools-extra/pseudo/gen/Main.cpp:114 + const clang::pseudo::Rule = G.table().Rules[RID]; + // lhs$$rhs$rhs$rhs + std::string EnumName = symbolName(R.Target, G) + "$"; so the dollar signs are a

[PATCH] D129100: [clangd] Support external throttler for preamble builds

sammccall added inline comments. Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:1499 + +S.remove(Filenames.back()); +// Now shut down the TU Scheduler. kadircet wrote: > sequencing is hard here but it'd be nice to ensure release is

[PATCH] D129100: [clangd] Support external throttler for preamble builds

sammccall added a comment. In D129100#3634548 , @vitalybuka wrote: > This looks flaky https://lab.llvm.org/buildbot/#/builders/5/builds/25929 Sorry about that, I believe this is fixed in eb64dbd6e0e617298579d32372fb92e595816d45

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

sammccall added inline comments. Comment at: clang-tools-extra/pseudo/gen/Main.cpp:60 + +std::string symbolName(clang::pseudo::SymbolID SID, + const clang::pseudo::Grammar ) { This code is copied right? Comment at:

[PATCH] D129100: [clangd] Support external throttler for preamble builds

sammccall added a comment. In D129100#3633073 , @anarazel wrote: > This caused llvm builds to fail for me (using clang-14, debug, debian > unstable, lld as linker): Thanks, I have a very similar setup but the build passed locally, I'll never

[PATCH] D129158: [pseudo] Define recovery strategy as grammar extension.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7d8e2742d958: [pseudo] Define recovery strategy as grammar extension. (authored by sammccall). Changed prior to commit:

[PATCH] D129158: [pseudo] Define recovery strategy as grammar extension.

sammccall marked an inline comment as done. sammccall added inline comments. Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:54 + return { + {(ExtensionID)Extension::Brackets, recoverBrackets}, + }; hokein wrote: > btw, it is annoying to write an

[PATCH] D129100: [clangd] Support external throttler for preamble builds

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rGed0e20d5e8c5: [clangd] Support external throttler for preamble builds (authored by sammccall). Changed

[PATCH] D129100: [clangd] Support external throttler for preamble builds

sammccall marked 5 inline comments as done. sammccall added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.h:108 +/// This throttler controls which preambles may be built at a given time. +clangd::PreambleThrottler *PreambleThrottler = nullptr; +

[PATCH] D129158: [pseudo] Define recovery strategy as grammar extension.

sammccall created this revision. sammccall added a reviewer: hokein. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, alextsao1999. Herald added a project: clang-tools-extra. Repository: rG LLVM Github Monorepo

[PATCH] D128486: [pseudo] Add error-recovery framework & brace-based recovery

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rG312116748890: [pseudo] Add error-recovery framework brace-based recovery (authored by sammccall).

[PATCH] D128486: [pseudo] Add error-recovery framework & brace-based recovery

sammccall marked 5 inline comments as done. sammccall added a comment. Thanks in particular for flagging the issue with duplicate forest nodes, you've found a hole in the model. That said, I've left a big FIXME and I think we should patch it later. Comment at:

[PATCH] D129100: [clangd] Support external throttler for preamble builds

sammccall updated this revision to Diff 442319. sammccall marked an inline comment as done. sammccall added a comment. address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129100/new/ https://reviews.llvm.org/D129100 Files:

[PATCH] D129100: [clangd] Support external throttler for preamble builds

sammccall marked 2 inline comments as done. sammccall added inline comments. Comment at: clang-tools-extra/clangd/TUScheduler.cpp:457 +// Give up on this request, releasing resources if any. +void abandon() { + std::lock_guard Lock(Mu); kadircet

[PATCH] D129100: [clangd] Support external throttler for preamble builds

sammccall updated this revision to Diff 442309. sammccall added a comment. remove dead code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129100/new/ https://reviews.llvm.org/D129100 Files: clang-tools-extra/clangd/ClangdServer.cpp

[PATCH] D129100: [clangd] Support external throttler for preamble builds

sammccall updated this revision to Diff 442308. sammccall added a comment. Simplify, leaning more on the throttler's lifetime and expecting it to manage state. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129100/new/

[PATCH] D129100: [clangd] Support external throttler for preamble builds

sammccall planned changes to this revision. sammccall added a comment. Summarizing an offline discussion: - ThrottlerState isolates complexity inside TUScheduler by absolving Throttler of the need to worry about callback lifetimes - but it does so at the cost of extra aggregate complexity

[PATCH] D129093: [pseudo] Eliminate LRTable::Action. NFC

This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rG9fbf1107cc76: [pseudo] Eliminate LRTable::Action. NFC (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D129093?vs=442127=442267#toc

[PATCH] D127448: [pseudo] Implement guard extension.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Great! Comment at: clang-tools-extra/pseudo/include/clang-pseudo/GLR.h:117 struct ParseParams { - // The grammar of the language we're going to parse. - const

[PATCH] D129100: [clangd] Support external throttler for preamble builds

sammccall added a comment. I wanted to send a first version out for feedback even if incomplete. (I want to do more extensive tests, and likely we should think about having an in-tree throttler on by default) To motivate the design: - when attempting to acquire a slot, we want to wait on

[PATCH] D129100: [clangd] Support external throttler for preamble builds

sammccall created this revision. sammccall added a reviewer: kadircet. Herald added subscribers: usaxena95, arphaman, javed.absar. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project:

[PATCH] D129093: [pseudo] Eliminate LRTable::Action. NFC

sammccall created this revision. sammccall added a reviewer: hokein. Herald added a subscriber: mgrang. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, alextsao1999. Herald added a project: clang-tools-extra. The last remaining uses

[PATCH] D128485: [pseudo] Store shift and goto actions in a compact structure with faster lookup.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. sammccall marked 3 inline comments as done. Closed by commit rGb37dafd5dc83: [pseudo] Store shift and goto actions in a compact structure with faster lookup. (authored

[PATCH] D129074: [pseudo] Use the prebuilt cxx grammar for the lit tests, NFC.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Up to you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129074/new/ https://reviews.llvm.org/D129074

[PATCH] D129074: [pseudo] Use the prebuilt cxx grammar for the lit tests, NFC.

sammccall added a comment. Can we make this the default value of the flag instead? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129074/new/ https://reviews.llvm.org/D129074 ___ cfe-commits mailing list

[PATCH] D128930: [pseudo] Add ForestNode descendants iterator, print ambiguous/opaque node stats.

This revision was automatically updated to reflect the committed changes. Closed by commit rG9b6bb12b8584: [pseudo] Add ForestNode descendants iterator, print ambiguous/opaque node stats. (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D128930?vs=441458=441482#toc

[PATCH] D128930: [pseudo] Add ForestNode descendants iterator, print ambiguous/opaque node stats.

sammccall marked an inline comment as done. sammccall added inline comments. Comment at: clang-tools-extra/pseudo/tool/ClangPseudo.cpp:68 + +struct NodeStats { + unsigned Total = 0; hokein wrote: > I can foresee we will use it in other places (e.g. our internal

[PATCH] D128930: [pseudo] Add ForestNode descendants iterator, print ambiguous/opaque node stats.

sammccall added a comment. Results on the first 1k lines of SemaCodeComplete.cpp: Forest bytes: 809824 nodes: 47378 GSS bytes: 24736 nodes: 41925 1490 Ambiguous nodes: 518 type-name 377 simple-type-specifier 137 namespace-name 135 nested-name-specifier 65

[PATCH] D128930: [pseudo] Add ForestNode descendants iterator, print ambiguous/opaque node stats.

sammccall created this revision. sammccall added a reviewer: hokein. Herald added a subscriber: mgrang. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, alextsao1999. Herald added a project: clang-tools-extra. Repository: rG LLVM

[PATCH] D128379: [clangd] Change the url for clang-tidy check documentation

sammccall added a comment. In D128379#3622128 , @sammccall wrote: > Hmm, this version looks complicated to me. > And also fragile: downstream we have CLANG_VERSION_STRINGs that don't match > upstream, Apple has their own versioning scheme, linux distros

[PATCH] D128379: [clangd] Change the url for clang-tidy check documentation

sammccall added a comment. Hmm, this version looks complicated to me. And also fragile: downstream we have CLANG_VERSION_STRINGs that don't match upstream, Apple has their own versioning scheme, linux distros tend to do things like `6.0.1~ubuntu3`... Let me sync with @kadircet Repository:

[PATCH] D128805: [pseudo] Fix bugs/inconsistencies in forest dump.

This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rGbc5e7ced1c7e: [pseudo] Fix bugs/inconsistencies in forest dump. (authored by sammccall). Changed prior to commit:

[PATCH] D128805: [pseudo] Fix bugs/inconsistencies in forest dump.

sammccall marked an inline comment as done. sammccall added inline comments. Comment at: clang-tools-extra/pseudo/unittests/ForestTest.cpp:138 + + const auto *B = (symbol("B"), ruleFor("B"), {Star}); + const auto *A1 = (symbol("A"), ruleFor("A"), {B}); hokein

[PATCH] D128486: [pseudo] Add error-recovery framework & brace-based recovery

sammccall added inline comments. Comment at: clang-tools-extra/pseudo/include/clang-pseudo/GLR.h:150 +// OldHeads is the parse state at TokenIndex. +// This function consumes consumes zero or more tokens (advancing TokenIndex), +// and places any recovery states created in

[PATCH] D128486: [pseudo] Add error-recovery framework & brace-based recovery

sammccall updated this revision to Diff 441154. sammccall added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128486/new/ https://reviews.llvm.org/D128486 Files: clang-tools-extra/pseudo/include/clang-pseudo/GLR.h

[PATCH] D128486: [pseudo] Add error-recovery framework & brace-based recovery

sammccall updated this revision to Diff 441125. sammccall marked 14 inline comments as done. sammccall added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128486/new/ https://reviews.llvm.org/D128486 Files:

[PATCH] D128826: Go-to-type on smart_ptr now also shows Foo

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. LG, just nits you know of outstanding and I think no extra tests needed. In D128826#3619956 , @tom-anders wrote: >>> So here's what I thought:

[PATCH] D128826: Go-to-type on smart_ptr now also shows Foo

sammccall added a comment. TL;DR: I'm convinced that this patch (return multiple locations) is a good choice. Also talked to Kadir offline. In D128826#3619339 , @tom-anders wrote: > I'm using this with neovim's builtin LSP, where when there are

[PATCH] D128826: Go-to-type on smart_ptr now also shows Foo

sammccall requested changes to this revision. sammccall added a comment. This revision now requires changes to proceed. Sorry, I didn't mean to accept yet - wanted some discussion on behavior & probably a unittest for the newly-public function. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D128826: Go-to-type on smart_ptr now also shows Foo

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/HeuristicResolver.h:75 + // could look up the name appearing on the RHS. + const Type *getPointeeType(const Type *T) const;

[PATCH] D128826: Go-to-type on smart_ptr now also shows Foo

sammccall added a comment. Thanks! I just have a question if this behavior should be even "stronger": In most editors, if you return one result you go straight there, if you return two results you get a menu. A menu is significantly worse than going straight to the right result - the UI is

[PATCH] D128821: [clangd][ObjC] Fix ObjC method definition completion

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Ah, this is subtle. Thanks! Comment at: clang-tools-extra/clangd/CodeComplete.cpp:866 case CodeCompletionResult::RK_Pattern: - return

[PATCH] D124690: [clangd] add inlay hints for std::forward-ed parameter packs

sammccall added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:483 + !Type.getNonReferenceType().isConstQualified() && + !isExpandedParameterPack(Param); } upsj wrote: > sammccall wrote: > > upsj wrote: > > >

[PATCH] D128679: [pseudo] Define a clangPseudoCLI library.

sammccall accepted this revision. sammccall added a comment. Looks good! Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Language.h:21 + Grammar G; + LRTable Table; + LR might be more distinguishing (and shorter!), up to you Repository: rG LLVM

[PATCH] D128805: [pseudo] Fix bugs/inconsistencies in forest dump.

sammccall created this revision. sammccall added a reviewer: hokein. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, alextsao1999. Herald added a project: clang-tools-extra. - when printing a shared node for the second time, don't

[PATCH] D124690: [clangd] add inlay hints for std::forward-ed parameter packs

sammccall accepted this revision. sammccall added a comment. Thanks for the readability improvements! I've forgotten if you have commit access? This stuff is complicated and I'm definitely going to forget the reasoning, the intuitive explanations are going to help a lot if changes are needed

[PATCH] D128795: [pseudo] Reimplement hardcoded error handling as a recovery strategy. NFC

sammccall created this revision. sammccall added a reviewer: hokein. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, alextsao1999. Herald added a project: clang-tools-extra. Repository: rG LLVM Github Monorepo

[PATCH] D124690: [clangd] add inlay hints for std::forward-ed parameter packs

sammccall added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:478 bool shouldHintReference(const ParmVarDecl *Param) { -// If the parameter is a non-const reference type, print an inlay hint +// If the parameter is of non-const l-value reference

[PATCH] D128679: [pseudo] Define a clangPseudoCLI library.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/pseudo/benchmarks/Benchmark.cpp:51 const std::string *SourceText = nullptr; -const Grammar *G = nullptr; +const Language *PLang =

[PATCH] D128486: [pseudo] Add error-recovery framework & brace-based recovery

sammccall reopened this revision. sammccall added a comment. Sorry, I committed this by mistake when working on another change. Reverted and this is ready for review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128486/new/

[PATCH] D128486: [pseudo] Add error-recovery framework & brace-based recovery

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa0f4c10ae227: [pseudo] Add error-recovery framework brace-based

[PATCH] D128486: [pseudo] Add error-recovery framework & brace-based recovery

sammccall updated this revision to Diff 440646. sammccall added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128486/new/ https://reviews.llvm.org/D128486 Files: clang-tools-extra/pseudo/include/clang-pseudo/GLR.h

[PATCH] D128708: [pseudo] Simplify/loosen the grammar around lambda captures.

This revision was automatically updated to reflect the committed changes. Closed by commit rGaacefc817d93: [pseudo] Simplify/loosen the grammar around lambda captures. (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D128687: [pseudo] Allow mixed designated/undesignated init lists.

This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rG8cf28585a403: [pseudo] Allow mixed designated/undesignated init lists. (authored by sammccall). Changed prior to commit:

[PATCH] D128687: [pseudo] Allow mixed designated/undesignated init lists.

sammccall marked an inline comment as done. sammccall added inline comments. Comment at: clang-tools-extra/pseudo/lib/cxx.bnf:468 designator := . IDENTIFIER +designator := [ expression ] expr-or-braced-init-list := expression hokein wrote: > I think this is

[PATCH] D124690: [clangd] add inlay hints for std::forward-ed parameter packs

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Sorry about the delay. This is complicated stuff and every time I put it down for a day I forget how it works! Thanks for your hard work here. I have some more suggestions but hopefully

[PATCH] D128708: [pseudo] Simplify/loosen the grammar around lambda captures.

sammccall created this revision. sammccall added a reviewer: hokein. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, alextsao1999. Herald added a project: clang-tools-extra. Treat captures as a uniform list, rather than

[PATCH] D128318: [pseudo] prototype: faster data structures for LRTable

sammccall abandoned this revision. sammccall added a comment. This is obsoleted by D128485 and D128472 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128318/new/

[PATCH] D128687: [pseudo] Allow mixed designated/undesignated init lists.

sammccall created this revision. sammccall added a reviewer: hokein. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, alextsao1999. Herald added a project: clang-tools-extra. This isn't allowed by the standard grammar but is allowed

[PATCH] D128472: [pseudo] Check follow-sets instead of tying reduce actions to lookahead tokens.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG85eaecbe8e54: [pseudo] Check follow-sets instead of tying reduce actions to lookahead tokens. (authored by sammccall). Changed prior to commit:

[PATCH] D128472: [pseudo] Check follow-sets instead of tying reduce actions to lookahead tokens.

sammccall marked 13 inline comments as done. sammccall added inline comments. Comment at: clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h:136 + // check whether a reduction should apply in the current context. + llvm::ArrayRef getReduceRules(StateID State)

[PATCH] D128472: [pseudo] Check follow-sets instead of tying reduce actions to lookahead tokens.

sammccall updated this revision to Diff 440415. sammccall marked 3 inline comments as done. sammccall added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128472/new/ https://reviews.llvm.org/D128472 Files:

[PATCH] D128679: [pseudo] Define a clangPseudoCLI library.

sammccall added a comment. Thanks, the split here looks good! Comment at: clang-tools-extra/pseudo/include/clang-pseudo/GLR.h:115 // Parameters for the GLR parsing. +// FIXME: refine it with the ParseLang struct. struct ParseParams { You're already touching

[PATCH] D128486: [pseudo] Add error-recovery framework & brace-based recovery

sammccall updated this revision to Diff 440393. sammccall edited the summary of this revision. sammccall added a comment. update testcase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128486/new/ https://reviews.llvm.org/D128486 Files:

[PATCH] D128486: [pseudo] Add error-recovery framework & brace-based recovery

sammccall updated this revision to Diff 440383. sammccall added a comment. revert format changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128486/new/ https://reviews.llvm.org/D128486 Files:

[PATCH] D128486: [pseudo] Add error-recovery framework & brace-based recovery

sammccall created this revision. Herald added a subscriber: mgrang. Herald added a project: All. sammccall updated this revision to Diff 440376. sammccall retitled this revision from "xxx recover" to "[pseudo] Add error-recovery framework & brace-based recovery". sammccall edited the summary of

[PATCH] D126731: [pseudo] Eliminate dependencies from clang-pseudo-gen. NFC

sammccall added a comment. In D126731#3609511 , @fmayer wrote: > FWIW this is not really NFC, I found this as the culprit in bisecting the > Android LLVM toolchain build, causing the following error: > > FAILED:

[PATCH] D128411: [syntax] Introduce a BaseToken class.

sammccall added inline comments. Comment at: clang/include/clang/Tooling/Syntax/TokenManager.h:9 +// +// Defines Token interfaces for the syntax-tree, decoupling the syntax-tree from +// the TokenBuffer. It enables producers (e.g. clang pseudoparser) to produce a

[PATCH] D128485: [pseudo] Store shift and goto actions in a compact structure with faster lookup.

sammccall updated this revision to Diff 439589. sammccall added a comment. remove debugging code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128485/new/ https://reviews.llvm.org/D128485 Files:

[PATCH] D128485: [pseudo] Store shift and goto actions in a compact structure with faster lookup.

sammccall created this revision. sammccall added a reviewer: hokein. Herald added a subscriber: mgrang. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, alextsao1999. Herald added a project: clang-tools-extra. The actions table is

[PATCH] D128472: [pseudo] Check follow-sets instead of tying reduce actions to lookahead tokens.

sammccall created this revision. sammccall added a reviewer: hokein. Herald added a subscriber: mgrang. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, alextsao1999. Herald added a project: clang-tools-extra. Previously, the action

[PATCH] D128307: [pseudo] Store reduction sequences by pointer in heaps, instead of by value.

sammccall added inline comments. Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:270 + +Sequences.emplace(F, PushSpec{N, Seq}); return; sammccall wrote: > hokein wrote: > > Just an idea (no action required) > > > > If we want to do further

[PATCH] D128307: [pseudo] Store reduction sequences by pointer in heaps, instead of by value.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rG7aff663b2a04: [pseudo] Store reduction sequences by pointer in heaps, instead of by value. (authored by

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