[PATCH] D114151: [clang-format] [C++20] [Module] clang-format couldn't recognize partitions

2021-11-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 388819. MyDeveloperDay marked 10 inline comments as done. MyDeveloperDay added a comment. Address review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114151/new/ https://reviews.llvm.org/D114151 Files: clang/docs/ReleaseNotes.rst

[PATCH] D114320: [clang-format] Extend AllowShortBlocksOnASingleLine for else blocks

2021-11-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Could you add the case where the else is not empty. Comment at: clang/unittests/Format/FormatTest.cpp:5400 + Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always; + EXPECT_EQ("if (a)\n" +"{\n" Can

[PATCH] D114320: [clang-format] Extend AllowShortBlocksOnASingleLine for else blocks

2021-11-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Nothing majorly wrong here, its looks pretty good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114320/new/ https://reviews.llvm.org/D114320 ___ cfe-commits mailing list

[PATCH] D114320: [clang-format] Extend AllowShortBlocksOnASingleLine for else blocks

2021-11-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:5406 + "{ }", + format("if(a){}else if(b){}else{}", Style), Style); + Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Never; you ca

[PATCH] D80528: [clang-format][WIP] Run more stability FormatTests

2021-11-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added subscribers: HazardyKnusperkeks, owenpan, curdeius. MyDeveloperDay added a comment. @HazardyKnusperkeks, @curdeius, @owenpan I feel we should try and get this committed, people tend to follow the adjacent style of the unit tests, and I sort of feel we keep having to ask peop

[PATCH] D22505: [clang-format] Access Modifier Use Normal Indent

2021-11-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I think so, it came to the top because someone subscribed to it, I was wondering if this was something that is still needed or if our existing options cover this. I recently came across a style like this. I don't know if there is still interest. Repository:

[PATCH] D80528: [clang-format][WIP] Run more stability FormatTests

2021-11-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @JakeMerdichAMD are you still interested in landing this, if not I'm happy to NFC it in over the next couple of weeks, if others agree Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80528/new/ https://reviews.llvm.o

[PATCH] D114320: [clang-format] Extend AllowShortBlocksOnASingleLine for else blocks

2021-11-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. I agree I think this LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114320/new/ https://reviews.llvm.org/D114320 ___ cfe-commits

[PATCH] D114151: [clang-format] [C++20] [Module] clang-format couldn't recognize partitions

2021-11-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1055-1077 + while (FormatTok) { +if (FormatTok->is(tok::colon)) { + FormatTok->setType(TT_ModulePartitionColon); +} +// Ha

[PATCH] D114151: [clang-format] [C++20] [Module] clang-format couldn't recognize partitions

2021-11-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1055-1077 + while (FormatTok) { +if (FormatTok->is(tok::colon)) { + FormatTok->setType(TT_ModulePartitionColon); +} +// Handle import as we would an include statement +

[PATCH] D114320: [clang-format] Extend AllowShortBlocksOnASingleLine for else blocks

2021-11-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Ok now I'm a little puzzled, shouldn't this be covered by `AllowShortIfStatementsOnASingleLine`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114320/new/ https://reviews.llvm.org/D114320 __

[PATCH] D114142: [clang-format] [PR52527] can join * with /* to form an outside of comment error C4138

2021-11-23 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGe7cb3283c803: [clang-format] [PR52527] can join * with /* to form an outside of comment error… (authored by MyDeveloperDay). Changed prior to commit: https://reviews.llvm.org/D114142?vs=388696&id=389133

[PATCH] D113844: [clang-format] [NFC] build clang-format with -Wall

2021-11-23 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1cb3cfd932a0: [clang-format] [NFC] build clang-format with -Wall (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113844/new/ htt

[PATCH] D114151: [clang-format] [C++20] [Module] clang-format couldn't recognize partitions

2021-11-23 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 389139. MyDeveloperDay added a comment. Fix infinite loop CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114151/new/ https://reviews.llvm.org/D114151 Files: clang/docs/ReleaseNotes.rst clang/lib/Format/FormatToken.h clang/lib/Format/Tok

[PATCH] D114430: [clang-format] NFC - recent changes caused clang-format to no longer be clang-formatted.

2021-11-23 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: klimek, HazardyKnusperkeks, curdeius, owenpan. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. The following 2 commits caused files in clang-format to no longer be clang-for

[PATCH] D114430: [clang-format] NFC - recent changes caused clang-format to no longer be clang-formatted.

2021-11-24 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG93fc91610f42: [clang-format] NFC - recent changes caused clang-format to no longer be clang… (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D114519: [clang-format] [PR52595]

2021-11-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius, owenpan. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://bugs.llvm.org/show_bug.cgi?id=52595 missing space between `T (&&)` but not bet

[PATCH] D114519: [clang-format] [PR52595] clang-format does not recognize rvalue references to array

2021-11-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:21879 verifyFormat("Foo::operator&&();", Style); - verifyFormat("operator&&(int(&&)(), class Foo);", Style); + verifyFormat("operator&&(int (&&)(), class Foo);", Style); verifyFormat("

[PATCH] D114519: [clang-format] [PR52595] clang-format does not recognize rvalue references to array

2021-11-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:9662 +TEST_F(FormatTest, AmpAmpOperator) { + verifyFormat("int operator()(T (&&)[N]) { return 1; }"); curdeius wrote: > Nit: these are not &/&& operators but references so

[PATCH] D114521: [clang-format] [PR47936] AfterControlStatement: MultiLine breaks AllowShortFunctionsOnASingleLine

2021-11-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: HazardyKnusperkeks, owenpan, curdeius, jaafar, KyrBoh. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://bugs.llvm.org/show_bug.cgi?id=47936 Using the MultiLine setti

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

2021-11-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @tnorth Are you planning on working on this or do you mind if one of us fixes the issues enough to get it over the line. (this was previously accepted and landed but the unit tests fail, for what looks like a fairly minor issue) Repository: rC Clang CHANGES S

[PATCH] D114430: [clang-format] NFC - recent changes caused clang-format to no longer be clang-formatted.

2021-11-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D114430#3151082 , @klimek wrote: > Thanks for cleaning up after me, and sorry for the mess - do y'all have > clang-format set up as a presubmit or do you just remember to format manually? Its never a problem to clean u

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

2021-11-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. After rebasing these tests seem to work (on Windows) -- Testing: 24 tests, 16 workers -- PASS: Clang :: Format/dry-run-alias.cpp (1 of 24) PASS: Clang :: Format/remove-duplicate-includes.cpp (2 of 24) PASS: Clang :: Format/disable-format.cpp (3 of 24) PAS

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

2021-11-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 389488. MyDeveloperDay added a comment. Rebasing previously landed but reverted commit CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72326/new/ https://reviews.llvm.org/D72326 Files: clang/docs/ClangFormat.rst clang/docs/ClangFormatStyle

[PATCH] D114521: [clang-format] [PR47936] AfterControlStatement: MultiLine breaks AllowShortFunctionsOnASingleLine

2021-11-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D114521#3151431 , @curdeius wrote: > LGTM, but I have one question. You mentioned in the bug ticket comment that > "this exposes a greater issue in AllowShortXXX". Have you found other cases > that misbehave? If so, th

[PATCH] D114521: [clang-format] [PR47936] AfterControlStatement: MultiLine breaks AllowShortFunctionsOnASingleLine

2021-11-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. However if we add AllowShortBlocksOnASingleLine : Always then it will be class Foo { foo() { if (x) bar(); if (x) { bar(); } } }; but if we set BasedOnStyle: LLVM AllowShortIfStatementsOnASingleLine: Never ColumnLimit: 80 Allow

[PATCH] D114521: [clang-format] [PR47936] AfterControlStatement: MultiLine breaks AllowShortFunctionsOnASingleLine

2021-11-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > @curdeius > Alright, I'll create a report https://bugs.llvm.org/show_bug.cgi?id=52598 That is related. But I'd rather handle it separately because the fix for the function and the fix for a while or if will need to be different. It really comes down to this

[PATCH] D114521: [clang-format] [PR47936] AfterControlStatement: MultiLine breaks AllowShortFunctionsOnASingleLine

2021-11-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I think the problem here is encapsulated by adding the following tests Style.BraceWrapping.AfterFunction = true; Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All; Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_AllIfsAndElse; S

[PATCH] D114320: [clang-format] Extend AllowShortBlocksOnASingleLine for else blocks

2021-11-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. In D114320#3151741 , @jessesna wrote: > In D114320#3148046 , > @MyDeveloperDay wrote: > >> O

[PATCH] D114521: [clang-format] [PR47936] AfterControlStatement: MultiLine breaks AllowShortFunctionsOnASingleLine

2021-11-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. The if/for/while case is way more involved and requires a change in a separate piece of code, I'm going to land this and we can handle 52598 separately Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114521/new/ https

[PATCH] D114521: [clang-format] [PR47936] AfterControlStatement: MultiLine breaks AllowShortFunctionsOnASingleLine

2021-11-25 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG72e4f4a2a117: [clang-format] [PR47936] AfterControlStatement: MultiLine breaks… (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D114519: [clang-format] [PR52595] clang-format does not recognize rvalue references to array

2021-11-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 389679. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Add more test combinations CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114519/new/ https://reviews.llvm.org/D114519 Files: clang/lib/Format/TokenAnn

[PATCH] D114151: [clang-format] [C++20] [Module] clang-format couldn't recognize partitions

2021-11-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 389682. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Address review comments, guard against eof CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114151/new/ https://reviews.llvm.org/D114151 Files: clang/doc

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

2021-11-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:3274 + llvm::SmallVector FilesToLookFor; + // User provided clang-format file using -style=file:/path/to/format/file + // Check for explicit config filename part of me wonders if thi

[PATCH] D114519: [clang-format] [PR52595] clang-format does not recognize rvalue references to array

2021-11-25 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGc94667a810e4: [clang-format] [PR52595] clang-format does not recognize rvalue references to… (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D114151: [clang-format] [C++20] [Module] clang-format couldn't recognize partitions

2021-11-25 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGc2fe2b5a63bb: [clang-format] [C++20] [Module] clang-format couldn't recognize partitions (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D114320: [clang-format] Extend AllowShortBlocksOnASingleLine for else blocks

2021-11-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Thank you for the patch, Do you need help committing this? if so we need your name and email address to do so https://llvm.org/docs/DeveloperPolicy.html#commit-messages Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

2021-11-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. The reason I have picked this us was because of: https://twitter.com/bruxisma/status/1462987809879257101 This slightly annoys me because : a) What they were talking about was in my view is disrespectful and inaccurate. b) I thought we'd already landed this (which

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

2021-11-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. FormatsBracedListsInColumnLayout is actually failing in its own test... it now produces void foo() { { // asdf { int a; } } { { int b; } } } rather than previously void foo() { {// asdf {int a; } } { { int b;

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

2021-11-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. The other failure is -class C extends {} {} +class C extends {} +{} I'm slightly struggling to work out in both cases if its the test that is wrong @cpplearner what do you think? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

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

2021-11-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. For your test I think its your test that is wrong, looking at the simpler versions (using clang-format-12) void foo() { { { int a; } } } void foo() { { int a; } } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

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

2021-11-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. I ran the examples from https://bugs.llvm.org/show_bug.cgi?id=38314 and its so much better than the previous state! Thanks for this patch, LGTM namespace n { void foo() {

[PATCH] D114320: [clang-format] Extend AllowShortBlocksOnASingleLine for else blocks

2021-11-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. one of us can commit it for you but we need to do the following to attribute it to you commit --amend --author="John Doe " So I need your name and email address. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114320

[PATCH] D114320: [clang-format] Extend AllowShortBlocksOnASingleLine for else blocks

2021-11-25 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG813d486cbc99: [clang-format] Extend AllowShortBlocksOnASingleLine for else blocks (authored by jessesna, committed by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D114320: [clang-format] Extend AllowShortBlocksOnASingleLine for else blocks

2021-11-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D114320#3154544 , @jessesna wrote: > Thanks a lot for your time and help @MyDeveloperDay No problem, come play some more! We need people to help and review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add options

2021-11-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I'm looking at a bug (sorry can't log it as we don't seem to have a working bug tracker!) that causes a regression on `operator new(` and `operator delete(` which I believe I've tracked to this change v12 ~foo() { ::operator delete(bar); } ::operator dele

[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add options

2021-11-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3161-3169 +if (Line.MightBeFunctionDecl && (Left.is(TT_FunctionDeclarationName) || + Right.is(TT_OverloadedOperatorLParen))) { + if (Line.mightBeFunc

[PATCH] D114696: [clang-format] regressed default behavior for operator parentheses

2021-11-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: HazardyKnusperkeks, crayroud, curdeius, owenpan. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. D110833: [clang-format] Refactor SpaceBeforeParens to add options

[PATCH] D114696: [clang-format] regressed default behavior for operator parentheses

2021-11-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @crayroud I'm going to commit this change to get past the regression, but we can continue to discuss here how you'd like to proceed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114696/new/ https://reviews.llvm.org

[PATCH] D114696: [clang-format] regressed default behavior for operator parentheses

2021-11-29 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG814aabae3775: [clang-format] regressed default behavior for operator parentheses (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D114725: [clang-format] add back tests which didn't cause a regression

2021-11-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added a reviewer: crayroud. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. bring back the valid tests removed as part of D114696: [clang-format] regressed default behavior for operator pare

[PATCH] D114725: [clang-format] add back tests which didn't cause a regression

2021-11-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 390407. MyDeveloperDay added a comment. bring back another 2 verifyFormats CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114725/new/ https://reviews.llvm.org/D114725 Files: clang/unittests/Format/FormatTest.cpp Index: clang/unittests/For

[PATCH] D114725: [clang-format] add back tests which didn't cause a regression

2021-11-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 390443. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Herald added subscribers: sdasgup3, wenzhicui, wrengr, Chia-hungDuan, dcaballe, cota, mravishankar, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh,

[PATCH] D114725: [clang-format] add back tests which didn't cause a regression

2021-11-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 390445. MyDeveloperDay added a comment. Remove unrelated file CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114725/new/ https://reviews.llvm.org/D114725 Files: clang/unittests/Format/FormatTest.cpp Index: clang/unittests/Format/FormatTes

[PATCH] D114725: [clang-format] add back tests which didn't cause a regression

2021-11-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:14277 + // FIXME these tests regressed behaviour. // verifyFormat("T A::operator() () {}", SpaceFuncDef); verifyFormat("auto lambda = []

[PATCH] D114725: [clang-format] add back tests which didn't cause a regression

2021-11-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. Its really this case that caused me issues, the behaviour for the foo {} cases was different from the ::operator cases. My feeling is that one is being detected as a function the other not. verifyFormat("::operat

[PATCH] D114859: [clang-format] Add better support for co-routinues

2021-12-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, Quuxplusone, ChuanqiXu, EricWF. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. Responding to a Discord call to help D113977: [Coroutine] Warn

[PATCH] D114859: [clang-format] Add better support for co-routinues

2021-12-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 390980. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Add more tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114859/new/ https://reviews.llvm.org/D114859 Files: clang/docs/ReleaseNotes.rst clang/l

[PATCH] D114859: [clang-format] Add better support for co-routinues

2021-12-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:22748-22749 +TEST_F(FormatTest, CoRoutinereturn) { + verifyFormat("int x = co_return foo();"); + verifyFormat("int x = (co_return foo());"); + verifyFormat("co_return (42);"); -

[PATCH] D114859: [clang-format] Add better support for co-routinues

2021-12-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 391054. MyDeveloperDay marked 8 inline comments as done. MyDeveloperDay added a comment. Address review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114859/new/ https://reviews.llvm.org/D114859 Files: clang/docs/ReleaseNotes.rst

[PATCH] D114859: [clang-format] Add better support for co-routinues

2021-12-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:22727 + +TEST_F(FormatTest, CoRoutineawait) { + verifyFormat("int x = co_await foo();"); Quuxplusone wrote: > naming of the tests is to allow easy running of all CoRoutine t

[PATCH] D114859: [clang-format] Add better support for co-routinues

2021-12-02 Thread MyDeveloperDay 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 rG57b95aed2a04: [clang-format] Add better support for co-routinues (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D115050: [clang-format] PR48916 PointerAlignment not working when using C++20 init-statement in for loop

2021-12-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://bugs.llvm.org/show_bug.cgi?id=48916 Left and Right Alignment inside a loop is misaligne

[PATCH] D115050: [clang-format] PR48916 PointerAlignment not working when using C++20 init-statement in for loop

2021-12-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3052 +// & 1 +if (Right.Tok.isLiteral()) + return true; HazardyKnusperkeks wrote: > Is this valid code? Or did we just wrongly assign PointerOrReference? I'd say >

[PATCH] D115069: [clang-format][NFC] Merge another two calls to isOneOf

2021-12-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Format/ContinuationIndenter.cpp:1294-1296 + Previous->isOneOf(tok::l_paren, tok::comma, tok::colon, TT_BinaryOperator, +

[PATCH] D115064: [clang-format][NFC] Replace deque with vector

2021-12-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. This seems reasonable its as if we only ever push_back and iterate backwards? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D11506

[PATCH] D115050: [clang-format] PR48916 PointerAlignment not working when using C++20 init-statement in for loop

2021-12-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 391915. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. Address review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115050/new/ https://reviews.llvm.org/D115050 Files: clang/lib/Format/FormatToken

[PATCH] D115050: [clang-format] PR48916 PointerAlignment not working when using C++20 init-statement in for loop

2021-12-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 391990. MyDeveloperDay added a comment. This fix has to go super specific to range for loops or we start manipulating pointer alignment all over the place in places perhaps we don't want to CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115050

[PATCH] D115050: [clang-format] PR48916 PointerAlignment not working when using C++20 init-statement in for loop

2021-12-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 391993. MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. Fix the merge and add the tests back (which missed the patch) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115050/new/ https://reviews.llvm.org/D115050

[PATCH] D115050: [clang-format] PR48916 PointerAlignment not working when using C++20 init-statement in for loop

2021-12-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D115050#3172842 , @HazardyKnusperkeks wrote: > You have reverted some of your changes. Including all the tests. Yes oops! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115050/new/ https://reviews.llvm.org/D1

[PATCH] D115064: [clang-format][NFC] Replace deque with vector

2021-12-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Even if you choose a number, beyond that number it behaves like a vector (just without the data being on the stack), its really good for small vectors but its not bad for large ones. I'm not sure what the default N is if you don't specify a number? (0?) CHANGES

[PATCH] D115103: Leak Sanitizer port to Windows

2021-12-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: compiler-rt/lib/lsan/lsan.h:20 +#elif SANITIZER_WINDOWS +# include "lsan_win.h" #endif clang-format? Comment at: compiler-rt/lib/lsan/lsan_common.h:48 +#elif SANITIZER_NETBSD || SANITIZER_FUCH

[PATCH] D115103: Leak Sanitizer port to Windows

2021-12-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. You need to add some reviewers to this revision. check out the CODE_OWNERS.txt or what I sometimes do is to `git log` one of the files you are changing and see who is a major contributor. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115103/new/ https:/

[PATCH] D115147: clang-format: [JS] test case for numeric separators.

2021-12-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay 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/D115147/new/ https://reviews.llvm.org/D115147 ___

[PATCH] D136154: [clang-format] Fix the continuation indenter

2022-10-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Can we log a GitHub issue I can’t see what you are trying to fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136154/new/ https://reviews.llvm.org/D136154 ___ cfe-commits

[PATCH] D136154: [clang-format] Fix the continuation indenter

2022-10-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Is it seeing the ‘ :’ as a binary operator? And not an inheritance colon? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136154/new/ https://reviews.llvm.org/D136154 ___ cf

[PATCH] D136154: [clang-format] Fix the continuation indenter

2022-10-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. Needs a better title to the review please to explain what it’s doing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1361

[PATCH] D136154: [clang-format] Fix the continuation indenter

2022-10-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. You've dropped the test on your most recent updated Comment at: clang/unittests/Format/FormatTest.cpp:6603-6622 + EXPECT_EQ( + "struct Derived {\n" + " Derived(\n" + "int firstArgWithLongName,\n" + "int secondArgWith

[PATCH] D136154: [clang-format] Fix the continuation indenter

2022-10-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I get why this fix works for you, but I don't agree that the fix is the correct one.. for example you don't have any BinaryOperators in that code, lets just say in my style I had BreakBeforeBinaryOperators: All Then your fix wouldn't cover me and I'd get exac

[PATCH] D136154: [clang-format] Fix the continuation indenter

2022-10-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Its probably worth looking at how to fix this more in the context of the original bug, Its something I don't like is where we pile in all these expressions, without any comments about what cases we are handling here.. I'd rather handle each one at a time.. as I do

[PATCH] D136154: [clang-format] Fix the continuation indenter

2022-10-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Doesn't something like this achieve the same CurrentState.Indent = State.Column; CurrentState.LastSpace = State.Column; - } else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr, - TT_CtorInitializerColon

[PATCH] D136154: [clang-format] Fix the continuation indenter

2022-10-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I think in hindsight D50699: [clang-format] fix PR38525 - Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All was overly aggressive again the `TT_CtorInitializerColon` case CHANGES SINCE LAST ACTION

[PATCH] D136154: [clang-format] Fix the continuation indenter

2022-10-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > Beyonce rule "If they liked it they should have put a test on it" CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136154/new/ https://reviews.llvm.org/D136154 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D136154: [clang-format] Fix the continuation indenter

2022-10-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D136154#3868151 , @hel-ableton wrote: > If you think this would be the better fix, AFAICS it does what we need. > What's the Beyonce rule, by the way? I do, but I'd like to hear @owenpan and @HazardyKnusperkeks view..

[PATCH] D136154: [clang-format] Fix the continuation indenter

2022-10-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:6615 + " fourthArgWithLongName) {}\n" + "};", Style);} missing newline CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136154/new/ https://review

[PATCH] D136154: [clang-format] Fix the continuation indenter

2022-10-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:814 +if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None || +Style.BreakBeforeBinaryOperators == FormatStyle::BOS_NonAssignment) { CurrentState.LastSpace = Sta

[PATCH] D136154: [clang-format] Fix the continuation indenter

2022-10-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Looking at the documentation, I think BreakBeforeBinaryOperators is unrelated to your test case. I would remove any references of it from your fix. F24968180: image.png CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D136154: [clang-format] Fix the continuation indenter

2022-10-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Going back to the original bug that caused this.. if you leave in your BOS_NonAssignment I think you'll reintroduce the original bug.. which possibly should have been called "incorrectly adds extra continuation indent spaces with BreakBeforeBinaryOperators set

[PATCH] D136154: [clang-format] Fix the continuation indenter

2022-10-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. You can remove this line in my view... > Style.BreakBeforeBinaryOperators == FormatStyle::BOS_NonAssignment) { CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136154/new/ https://reviews.llvm.org/D136154 ___ cfe

[PATCH] D136437: [clang-format] Insert closing braces of unaffected lines

2022-10-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Does this need a unit test? or are we good? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136437/new/ https://reviews.llvm.org/D

[PATCH] D136830: [clang-format][NFC] Move BracesRemover tests out of FormatTest.cpp

2022-10-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay 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/D136830/new/ https://reviews.llvm.org/D136830 ___

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-10-30 Thread MyDeveloperDay 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 rG3edc1210a49d: [clang-format] Adds a formatter for aligning trailing comments over empty lines (authored by yusuke-kadowaki, committed by MyDeveloper

[PATCH] D137052: [clang-format] Don't skip #else/#elif of #if 0

2022-11-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added subscribers: curdeius, djasper, klimek. MyDeveloperDay added a comment. In D137052#3899094 , @sstwcw wrote: > It just occurred to me that klimek is the one who added all this stuff. Why > didn't you include him as a reviewer? @djas

[PATCH] D132295: [clang-format] Change heuristic for locating lambda template arguments

2022-08-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. can you add some format tests to show examples of how you think it should change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132295/new/ https://reviews.llvm.org/D132295 ___

[PATCH] D132256: [clang-format] Add DefinitionBlockSpacing option

2022-08-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. nice Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132256/new/ https://reviews.llvm.org/D132256 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D132189: [clang-format] Don't put `noexcept` on empty line following constructor

2022-08-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This looks pretty good to me, but wait for @HazardyKnusperkeks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132189/new/ https://reviews.llvm.org/D132189 ___ cfe-commits m

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-08-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:1602 Style.AlignTrailingComments = false; + Style.AlignTrailingCommentsIgnoreEmptyLine = false; Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty; ditto remove =

[PATCH] D132295: [clang-format] Change heuristic for locating lambda template arguments

2022-08-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. If none of the previous tests fail then this looks like it might be a good fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132295/new/ https://reviews.llvm.org/D132295 ___

[PATCH] D132295: [clang-format] Change heuristic for locating lambda template arguments

2022-08-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM but maybe wait for the others Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132295/new/ https://reviews.llvm.org/D132295 _

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-08-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Format/Format.cpp:649 IO.mapOptional("AlignOperands", Style.AlignOperands); -IO.mapOptional("AlignTrailingComment

<    10   11   12   13   14   15   16   17   18   19   >