[PATCH] D33415: [clangd] Replaced WorkerRequest with std::function...

krasimir added inline comments. Comment at: clangd/ClangdUnitStore.h:56 + template + void runOnExistingUnit(PathRef File, Func Action) { +std::lock_guard Lock(Mutex); krasimir wrote: > Maybe make it less generic and put the implementation in the source fil

[PATCH] D32351: [Tooling][libclang] Remove unused CompilationDatabase::MappedSources

krasimir added a comment. @klimek: ping https://reviews.llvm.org/D32351 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32480: clang-format: Add CompactNamespaces option

krasimir added a comment. This change should also adapt NamespaceEndCommentFixer to respect the new option and not introduce/remove/change the comments unexpectedly. https://reviews.llvm.org/D32480 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D32480: clang-format: Add CompactNamespaces option

krasimir added a comment. In any case, adding a namespace end comment to a line closing multiple namespaces is super confusing for me: what does the comment refer to: the inner one, the outer one, or both? `}} // namespace A::B` https://reviews.llvm.org/D32480 __

[PATCH] D33415: [clangd] Replaced WorkerRequest with std::function...

krasimir accepted this revision. krasimir added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D33415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D32351: [Tooling][libclang] Remove unused CompilationDatabase::MappedSources

This revision was automatically updated to reflect the committed changes. Closed by commit rL303635: [Tooling][libclang] Remove unused CompilationDatabase::MappedSources (authored by krasimir). Changed prior to commit: https://reviews.llvm.org/D32351?vs=96138&id=99905#toc Repository: rL LLVM

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

krasimir added a comment. It appears that this is causing an assertion segfault in a `rustc` test over at our experimental rust + llvm@head bot: https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/8430#167e6de5-2dd5-41c3-87d7-b6e3f3908371/262-706 The test is https://github.co

[PATCH] D120315: [clang-format] fix preprocessor nesting after https://github.com/llvm/llvm-project/commit/529aa4b011c4ae808d658022ef643c44dd9b2c9c

krasimir created this revision. krasimir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. In https://github.com/llvm/llvm-project/commit/529aa4b011c4ae808d658022ef643c44dd9b2c9c by setting the identifier info to nullptr, we started to subtl

[PATCH] D113319: [clang-format] Improve require and concept handling

krasimir added a comment. It appears that after this patch clang-format started breaking up `requires` in javascript, e.g.: // before function f() { var requires = {}; } // after function f() { var requires = {}; } Could we restrict the requires logic only to (obj)

[PATCH] D113319: [clang-format] Improve require and concept handling

krasimir added a comment. It appears that this causes a regression by adding a space in-between pointer dereferences `*p` -> `* p` in some cases, e.g. formatting this with llvm style: // before void f() { while (p < a && *p == 'a') p++; } // after void f() { while (p < a

[PATCH] D120315: [clang-format] Fix preprocessor nesting after commit 529aa4b011c4ae808d658022ef643c44dd9b2c9c

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

[PATCH] D120315: [clang-format] Fix preprocessor nesting after commit 529aa4b011c4ae808d658022ef643c44dd9b2c9c

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc9592ae49b8c: [clang-format] Fix preprocessor nesting after commit… (authored by krasimir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D120324: [clang-format] Avoid parsing "requires" as a keyword in non-C++-like languages.

krasimir accepted this revision. krasimir added a comment. This revision is now accepted and ready to land. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120324/new/ https://reviews.llvm.org/D120324 _

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

krasimir added a comment. @achieveartificialintelligence any progress with the problematic IR? Have you been able to reproduce using nikic's reduced example? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93298/new/ https://reviews.llvm.org/D93298

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

krasimir added a comment. In D93298#3342480 , @achieveartificialintelligence wrote: > In D93298#3342452 , @krasimir wrote: > >> @achieveartificialintelligence any progress with the problematic IR? Have >> you been

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

krasimir added a comment. @achieveartificialintelligence thank you for looking into this, sorry for the late reply! It looks like the latest version addresses @nikic's IR reproducer with `-mtriple=riscv32`. There seems to be an error trying this out with `-mtriple=riscv64`: % cat test.ll

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

krasimir added a comment. > I just pushed 6cb42cd6669785f3b611106e1b6b38bbe65733a9 > to > hopefully fix this. Thank you Craig! That fixed it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D118869: [clang-format] Non-latin comment prefix whitespace

krasimir added a comment. Herald added a project: All. It appears that this caused a regression by adding an additional space of indentation to line comments in some cases: % cat test.cc # (before) // Comment int i; % clang-format -style=google test.cc //  Comment int i; @ksyx coul

[PATCH] D118869: [clang-format] Non-latin comment prefix whitespace

krasimir added a comment. Thank you! Turns out the original source wasn't using an ascii space for the comment indentation but a unicode [c2 a0] no-break space: 2f 2f c2 a0 43 6f 6d 6d 65 6e 74 0a 69 6e 74 20 |//..Comment.int | IMO we shouldn't aim to handle non-ascii spaces for indentation

[PATCH] D101033: [clang-format] fix indent in alignChainedConditionals

krasimir added a comment. I also have a limited understanding based on the original patch https://github.com/llvm/llvm-project/commit/4db94094b469b4715d08ef37f1799bf3ea7ca8ea together with examining affected test cases. I believe the intention of this section is to handle, e.g., the alignment of

[PATCH] D101033: [clang-format] fix indent in alignChainedConditionals

This revision was automatically updated to reflect the committed changes. Closed by commit rG5987d7c59da5: [clang-format] fix indent in alignChainedConditionals (authored by krasimir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101033/new/ https:

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

krasimir added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:125 + CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset( + -1))) return false; penagos wrote: > krasimir wrote: > > penago

[PATCH] D101515: clang-format: [JS] handle "off" in imports

krasimir added inline comments. Comment at: clang/lib/Format/SortJavaScriptImports.cpp:158 std::string ReferencesText; -bool SymbolsInOrder = true; -for (unsigned i = 0, e = Indices.size(); i != e; ++i) { - JsModuleReference Reference = References[Indices[i]]; -

[PATCH] D101515: clang-format: [JS] handle "off" in imports

krasimir added a comment. @h-joo , seems we had a comment race :D cheers! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101515/new/ https://reviews.llvm.org/D101515 ___ cfe-commits mailing list cfe-commi

[PATCH] D101515: clang-format: [JS] handle "off" in imports

krasimir accepted this revision. krasimir added inline comments. Comment at: clang/lib/Format/SortJavaScriptImports.cpp:257 +// references per group. +auto *Start = References.begin(); +SmallVector ReferencesSorted; please fix lint warning on line 257

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

krasimir added a comment. @penagos, I'll submit this for you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100778/new/ https://reviews.llvm.org/D100778 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

This revision was automatically updated to reflect the committed changes. Closed by commit rG8fa56f7ededc: [clang-format] Prevent extraneous space insertion in bitshift operators (authored by penagos, committed by krasimir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D101702: [clang-format] Add more support for C# 8 nullables

krasimir accepted this revision. krasimir added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1563 + if (Current.Previous) { +bool isIdentifier = +Style.Language == FormatStyle::LK_JavaScript nit: `s/isIdentifier/IsIden

[PATCH] D101702: [clang-format] Add more support for C# 8 nullables

krasimir added inline comments. Comment at: clang/lib/Format/FormatTokenLexer.cpp:135 return; +} } nit: use spaces for indentation, no tabs (Phabricator makes it look this line starts with a tab character) Repository: rG LLVM Github Monorepo C

[PATCH] D91949: [clang-format] Add BeforeStructInitialization option in BraceWrapping configuration

krasimir added a comment. Do we have some widely used code style that requires the new option (https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options)? I agree with @MyDeveloperDay that this option as-is is likely not addressing all the cases where we would want

[PATCH] D114583: [clang-format] Adjust braced list detection

krasimir added a comment. It appears that this regressed the formatting of initializer lists in some cases: # Before % cat ~/test.cc class A { A() : a{} {} A(int b) : b(b) {} A(int a, int b) : a(a), bs{{bs...}} { f(); } int a, b; }; # After % bui

[PATCH] D116000: Revert "[clang-format] Adjust braced list detection"

krasimir created this revision. krasimir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. It appears that this regressed the formatting of initializer lists in some cases, see comments on https://reviews.llvm.org/D114583. I'll follow-up by a

[PATCH] D116001: [clang-format] add regression tests for braced lists

krasimir created this revision. krasimir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Depends on https://reviews.llvm.org/D116000. Added test cases from the comments on https://reviews.llvm.org/D114583. Repository: rG LLVM Github Mo

[PATCH] D116000: Revert "[clang-format] Adjust braced list detection"

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd96bf6ea4618: Revert "[clang-format] Adjust braced list detection" (authored by krasimir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D116001: [clang-format] add regression tests for braced lists

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3a3fcd6a23ad: [clang-format] add regression tests for braced lists (authored by krasimir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D99063: [clang-format] Fix ObjC method indent after f7f9f94b

krasimir created this revision. krasimir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Commit https://github.com/llvm/llvm-project/commit/f7f9f94b2e2b4c714bac9036f6b73a3df42daaff changed the indent of ObjC method arguments from +4 to +2,

[PATCH] D99063: [clang-format] Fix ObjC method indent after f7f9f94b

This revision was automatically updated to reflect the committed changes. Closed by commit rGd9abcdd9f471: [clang-format] Fix ObjC method indent after f7f9f94b (authored by krasimir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99063/new/ https://

[PATCH] D101033: [clang-format] fix indent in alignChainedConditionals

krasimir created this revision. krasimir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Clang-format was indenting the lines following the `?` in the added test case by +5 instead of +4. This only happens in a very specific situation, wher

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

krasimir added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:125 + CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset( + -1))) return false; penagos wrote: > MyDeveloperDay wrote: > >

[PATCH] D108538: [clang-format] break after the closing paren of a TypeScript decoration

krasimir created this revision. krasimir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This fixes up a case that regressed from https://reviews.llvm.org/D105964: in specific contexts, clang-format stopped breaking after the `)` in TypeScr

[PATCH] D108538: [clang-format] break after the closing paren of a TypeScript decoration

This revision was automatically updated to reflect the committed changes. Closed by commit rGf3671a688db2: [clang-format] break after the closing paren of a TypeScript decoration (authored by krasimir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D108620: [clang-format] keep TypeScript argument decorators in line

krasimir created this revision. krasimir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. As a follow-up from https://reviews.llvm.org/D108538, ensure TypeScript argument decorators are kept in line with the argument. Repository: rG LLVM

[PATCH] D108620: [clang-format] keep TypeScript argument decorators in line

krasimir updated this revision to Diff 368311. krasimir added a comment. - consider also functions and setters Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108620/new/ https://reviews.llvm.org/D108620 Files: clang/lib/Format/ContinuationIndente

[PATCH] D108620: [clang-format] keep TypeScript argument decorators in line

krasimir marked an inline comment as done. krasimir added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:497 if (Style.Language == FormatStyle::LK_JavaScript && - Previous.is(tok::r_paren) && Previous.is(TT_JavaAnnotation)) { + Current.TokenTe

[PATCH] D108620: [clang-format] keep TypeScript argument decorators in line

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbefb9dc3694e: [clang-format] keep TypeScript argument decorators in line (authored by krasimir). Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D108692: [clang-format] Support TypeScript override keyword

krasimir accepted this revision. krasimir added a comment. This revision is now accepted and ready to land. Thank you! Let me know if you would like me to commit this for you in case you don't have commit rights for LLVM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D108692: [clang-format] Support TypeScript override keyword

This revision was automatically updated to reflect the committed changes. Closed by commit rGe708808f875f: [clang-format] Support TypeScript override keyword (authored by jankuehle, committed by krasimir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D108810: [clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C# behaviour to change

krasimir accepted this revision. krasimir added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Format/TokenAnnotator.cpp:3803 + if (Line.startsWith(Keywords.kw_interface) && + Style.BraceWrapping.AfterClass) +return t

<    5   6   7   8   9   10