[PATCH] D99489: [clang] [PR49736] [C++2b] Correctly reject lambdas with requires clause and no parameter list

2021-03-30 Thread Marek Kurdej via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa99b8ae39091: [clang] [PR49736] [C++2b] Correctly reject lambdas with requires clause and no… (authored by curdeius). Changed prior to commit: htt

[PATCH] D99458: [clang-format] Fix east const pointer alignment of operators

2021-03-30 Thread Marek Kurdej via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGbc4b0fc53e47: [clang-format] Fix east const pointer alignment of operators (authored by nrieck, committed by curdeius). Changed prior to commit: https://reviews.llvm.org/D99458?vs=333678&id=334173#toc

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

2021-03-31 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM, but please clean up the comments before landing. Comment at: clang/docs/ClangFormatStyleOptions.rst:2132 +**EmptyLineAfterAccessModifier** (``EmptyLineAfterAccessMo

[PATCH] D99663: [clang-cl] [Sema] Do not prefer integral conversion over floating-to-integral for MS compatibility 19.28 and higher.

2021-03-31 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. curdeius added reviewers: mstorsjo, thakis, phosek. Herald added a subscriber: dexonsmith. curdeius requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. As of MSVC 19.28 (2019 Update 8), integral conversion is no

[PATCH] D99663: [clang-cl] [Sema] Do not prefer integral conversion over floating-to-integral for MS compatibility 19.28 and higher.

2021-04-01 Thread Marek Kurdej via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG2ec7f639c49f: [clang-cl] [Sema] Do not prefer integral conversion over floating-to-integral… (authored by curdeius). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

2021-04-15 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. I think you can land it. The issues seem indeed unrelated. You might want to rebase and retest before landing to be sure. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98237/new/ https://reviews.llvm.org/D98237 __

[PATCH] D99503: [clang-format] Inconsistent behavior regarding line break before access modifier

2021-04-15 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. No remarks from me. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99503/new/ https://reviews.llvm.org/D99503

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

2021-04-15 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. I'll land it for you. Could you please provide "Name Surname " for commit attribution unless the info associated with your phabricator user is okay? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98237/new/ https://review

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

2021-04-15 Thread Marek Kurdej via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdda978eef87c: [clang-format] Option for empty lines after an access modifier. (authored by Max_S, committed by curdeius). Repository: rG LLVM Gith

[PATCH] D99503: [clang-format] Inconsistent behavior regarding line break before access modifier

2021-04-16 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. @max_s I think you need to rebase on current main and fix two tests as //probably// D98237 (or another patch) changed the behaviour of this patch. [ FAILED ] 2 tests, listed below: [ FAILED ] FormatTest.FormatsAccessModifiers

[PATCH] D99503: [clang-format] Inconsistent behavior regarding line break before access modifier

2021-04-16 Thread Marek Kurdej via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGfd4e08aa8f7e: [clang-format] Inconsistent behavior regarding line break before access modifier (authored by Max_S, committed by curdeius). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D100696: Fixed typos

2021-04-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision as: curdeius. curdeius added a comment. LGTM. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100696/new/ https://reviews.llvm.org/D100696 ___ cfe-commits mailing li

[PATCH] D100727: [clang-format] Correctly apply AllowShortIfStatementsOnASingleLine: Always to else branch.

2021-04-18 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks. curdeius requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This fixes a bug reported on Discord #clang-format

[PATCH] D100727: [clang-format] Correctly apply AllowShortIfStatementsOnASingleLine: Always to else branch.

2021-04-18 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. In D100727#2697419 , @HazardyKnusperkeks wrote: > How is > > if (a) return; > else > return; > > formatted with the different options? Do you have something specific in mind? > And from the documentation I think it was

[PATCH] D100727: [clang-format] Correctly apply AllowShortIfStatementsOnASingleLine: Always to else branch.

2021-04-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Oh, you're right. Should we go for a separate option then? AllowShortElseStatementsOnASingleLine? What would control else if statements then? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100727/new/ https://reviews.llvm

[PATCH] D99031: [clang-format] Fix CompactNamespaces corner case when AllowShortLambdasOnASingleLine/BraceWrapping.BeforeLambdaBody are set

2021-04-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. In D99031#2697563 , @aybassiouny wrote: > After rechecking, turns out `AllowShortLambdasOnASingleLine` and > `BeforeLambdaBody` both need to be turned on in order for the regression to > be expressed, this affects the UT. Pleas

[PATCH] D99031: [clang-format] Fix CompactNamespaces corner case when AllowShortLambdasOnASingleLine/BraceWrapping.BeforeLambdaBody are set

2021-04-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3498 + !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral) && + !Tok.Previous->Previous->is(tok::kw_namespace)); } Please add at least asserts for `Tok.Previous` and `

[PATCH] D100727: [clang-format] Correctly apply AllowShortIfStatementsOnASingleLine: Always to else branch.

2021-04-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius planned changes to this revision. curdeius added a subscriber: klimek. curdeius added a comment. More I look at the current state of this option, more I think it's misleading. I would expect that the if after else is also controlled by this option. And, the last else as well (adding yet

[PATCH] D93846: [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier

2020-12-28 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. My 2 cents. Otherwise I agree with @MyDeveloperDay's comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1219-1228 + if (Style.InsertEmptyLineBeforeAccessModifier && PreviousLine && + PreviousLine->Last->isOneOf(tok::semi, tok::r_br

[PATCH] D93839: [clang-format] PR48594 BraceWrapping: SplitEmptyRecord ignored for templates

2020-12-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision. curdeius added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:364 + +if (TheLine->Last->is(tok::l_brace) && I != AnnotatedLines.begin() && +I[-1]->Las

[PATCH] D93846: [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier

2020-12-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Nit. Comment at: clang/docs/ClangFormatStyleOptions.rst:1545 +**EmptyLineBeforeAccessModifier** (``bool``) + If true, the empty line is inserted before access modifiers + Full stop at the end of the phrase. CHANGES SINCE LAST ACTION

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2020-12-30 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision. curdeius added a comment. This revision now requires changes to proceed. Please add and fix tests. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2485 - if (!Style.AllowShortEnumsOnASingleLine) -addUnwrappedLine(); -

[PATCH] D93839: [clang-format] PR48594 BraceWrapping: SplitEmptyRecord ignored for templates

2020-12-30 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. Thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93839/new/ https://reviews.llvm.org/D93839 ___ cfe-commits mailing list

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-01-02 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. In D93938#2476186 , @MyDeveloperDay wrote: > I think we are missing some clarity in this bug as to what the actual problem > is, I do agree the test looks wrong, I agree on this. If like to see a more exhaustive test suite for

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

2021-01-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/WhitespaceManager.cpp:421 // matching token, the sequence ends here. - if (Changes[i].NewlinesBefore > 1 || !FoundMatchOnLine) + if (((Changes[i].NewlinesBefore > 1) && (!AcrossEmpty)) || + (!F

[PATCH] D94206: [clang-format] turn on formatting after "clang-format on" while sorting includes

2021-01-07 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. LGTM from my perspective. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94206/new/ https://reviews.llvm.org/D94206 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://list

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

2021-01-07 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. LGTM if you add a test for hdrstop(filename) and possibly with LF newline (as the test you've already added tests CRLF). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94217/new/ https://reviews.llvm.org/D94217 ___ c

[PATCH] D94287: [clang-format] Fix include sorting bug

2021-01-07 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Seems to be a duplicate of D94206 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94287/new/ https://reviews.llvm.org/D94287 ___ cfe-commits mai

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

2021-01-08 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. I like what you did with tests. And thank you for the patch. Before accepting this formally, I'd like to see the premerge tests pass. How do you upload the diff? Do you use arcanist? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94217/new/ https://reviews.llvm

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

2021-01-08 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. I've set the repository to rG LLVM. Could you please rebase and update the patch so that the CI gets triggered? I hope it is enough to just reupload a patch. I have heard that sometimes you might need to reupload the diff

[PATCH] D94206: [clang-format] turn on formatting after "clang-format on" while sorting includes

2021-01-08 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Please rebase this patch too to trigger CI. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94206/new/ https://reviews.llvm.org/D94206 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

2021-01-09 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius 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/D94217/new/ https://reviews.llvm.org/D94217

[PATCH] D94201: [clang-format] Skip UTF8 Byte Order Mark while sorting includes

2021-01-11 Thread Marek Kurdej via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGee27c767bd20: [clang-format] Skip UTF8 Byte Order Mark while sorting includes (authored by rjelonek, committed by curdeius). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D94206: [clang-format] turn on formatting after "clang-format on" while sorting includes

2021-01-11 Thread Marek Kurdej via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG7473940bae0f: [clang-format] turn on formatting after "clang-format on" while sorting includes (authored by rjelonek, committed by curdeius). Changed prior to commit: https://reviews.llvm.org/D94206?vs

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

2021-01-11 Thread Marek Kurdej via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG89878e8c966a: [clang-format] Find main include after block ended with #pragma hdrstop (authored by rjelonek, committed by curdeius). Changed prior to commit: https://reviews.llvm.org/D94217?vs=315509&id

[PATCH] D93846: [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier

2021-01-14 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. One last nit, otherwise LGTM. Comment at: clang/unittests/Format/FormatTest.cpp:8891 +TEST_F(FormatTest, FormatsAccessModifiers) { + verifyFormat("struct foo {\n" + "private:\n" For the e

[PATCH] D94661: [clang-format] [PR19056] Add support for access modifiers indentation

2021-01-14 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. That looks like a strange style option to me. Is there any bigger codebase formatting the code this way? Comment at: clang/docs/ClangFormatStyleOptions.rst:2039 +**IndentAccessModifiers** (``bool``) + Makes an indentation level for record (``class`

[PATCH] D93776: [clang-format] Add StatementAttributeLikeMacros option

2021-01-16 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM apart from the minor comment. But I'd like @mydeveloperday to have a look too. Comment at: clang/lib/Format/FormatToken.h:54 TYPE(FunctionTypeLParen)

[PATCH] D94906: [clang-format] Apply Allman style to lambdas

2021-01-18 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. LGTM. As a note, I agree with @MyDeveloperDay. It's more user-friendly and IMO less surprising to set the base wrapping style in `BreakBeforeBraces` and then customize it changing in `BraceWrapping`. I'd argue that it won't be a breaking

[PATCH] D94955: [clang-format] Treat ForEachMacros as loops

2021-01-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision. curdeius added a comment. This revision now requires changes to proceed. LGTM per se. But, as it's a somehow a breaking change, I'd rather wait for release 12 branch before landing to main, so that folks have time to veto before release 13. Please upd

[PATCH] D95078: [clang-format] [NFC] Use some constexpr StringRef instead of const char*.

2021-01-20 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius 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/D95078/new/ https://reviews.llvm.org/D95078

[PATCH] D95081: [clang-format] [NFC] Restructure getLineCommentIndentPrefix

2021-01-20 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/BreakableToken.cpp:44 const FormatStyle &Style) { + // Keep these prefixes sorted after length. static constexpr StringRef KnownCStylePrefixes[] = {"///<", "//!<", "///",

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-13 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. In D69764#2996365 , @steveire wrote: > FYI - there's nothing "anti-inclusive" about East/West. Thank you for your comment. Personally I don't think that the question of not using East/West is whether these terms are inclusive o

<    4   5   6   7   8   9