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

2022-01-17 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 rG1e512f022ad5: [clang-format] Treat ForEachMacros as loops (authored by curdeius). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

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

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

2022-01-14 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:2209 + + FormatStyle ShortBlocks = getLLVMStyle(); + ShortBlocks.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Always; If wanted, I might commit the new tests without

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

2022-01-14 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 36. curdeius added a comment. Clean up tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94955/new/ https://reviews.llvm.org/D94955 Files: clang/lib/Format/UnwrappedLineFormatter.cpp

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

2022-01-14 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius marked 5 inline comments as done. curdeius added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:2181-2185 + EXPECT_EQ(Style.AllowShortBlocksOnASingleLine, FormatStyle::SBS_Never); + EXPECT_EQ(Style.AllowShortLoopsOnASingleLine, false);

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

2022-01-14 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 399987. curdeius added a comment. Assert + test for loop. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94955/new/ https://reviews.llvm.org/D94955 Files: clang/lib/Format/UnwrappedLineFormatter.cpp

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

2022-01-14 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 399982. curdeius added a comment. Add more tests. Fix missed cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94955/new/ https://reviews.llvm.org/D94955 Files:

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

2022-01-14 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius commandeered this revision. curdeius edited reviewers, added: GoBigorGoHome; removed: curdeius. curdeius added a comment. Commandeering as it's stale.. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94955/new/

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

2022-01-10 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. @GoBigorGoHome, are you still interested in this review? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94955/new/ https://reviews.llvm.org/D94955 ___ cfe-commits mailing list

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

2021-02-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I guess I'm not making myself clear, I was just hoping you could be super explicit about what you are changing so anyone coming into this review would understand why the behaviour had changed (this is just pseudo code it may not be correct for all cases, but you

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

2021-02-02 Thread Jiashu Zou via Phabricator via cfe-commits
GoBigorGoHome added a comment. @MyDeveloperDay I changed the `verifyFormat` to `EXPECT_NE` because I don't know the proper way "to show that the previous tests were wrong", and I agree with you that it is a dirty hack. However, I think it is already clear why the tests were changed, that was

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

2021-02-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I'm never going to be the one to accept patches where people change tests without making it really clear why they are changing it. You have to prove you are not regressing behaviour, I work on the Beyonce rule, "if you liked it you should have put a test on it"

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

2021-02-01 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. I let you coordinate with @Mydeveloperday concerning the tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94955/new/

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

2021-02-01 Thread Jiashu Zou via Phabricator via cfe-commits
GoBigorGoHome updated this revision to Diff 320426. GoBigorGoHome added a comment. Update tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94955/new/ https://reviews.llvm.org/D94955 Files: clang/docs/ReleaseNotes.rst

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

2021-01-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:995 TEST_F(FormatTest, ForEachLoops) { verifyFormat("void f() {\n" + " foreach (Item *item, itemlist) {\n" GoBigorGoHome wrote: > MyDeveloperDay wrote: > >

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

2021-01-25 Thread Jiashu Zou via Phabricator via cfe-commits
GoBigorGoHome added a comment. Comment at: clang/unittests/Format/FormatTest.cpp:995 TEST_F(FormatTest, ForEachLoops) { verifyFormat("void f() {\n" + " foreach (Item *item, itemlist) {\n" MyDeveloperDay wrote: > I'd like you to assert that

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

2021-01-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:995 TEST_F(FormatTest, ForEachLoops) { verifyFormat("void f() {\n" + " foreach (Item *item, itemlist) {\n" I'd like you to assert that short loops are off

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

2021-01-20 Thread Jiashu Zou via Phabricator via cfe-commits
GoBigorGoHome updated this revision to Diff 317794. GoBigorGoHome added a comment. Update Clang release notes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94955/new/ https://reviews.llvm.org/D94955 Files: clang/docs/ReleaseNotes.rst

[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

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

2021-01-19 Thread Jiashu Zou via Phabricator via cfe-commits
GoBigorGoHome created this revision. GoBigorGoHome added a reviewer: MyDeveloperDay. GoBigorGoHome requested review of this revision. Herald added a project: clang. TT_ForEachMacro should be considered in rules AllowShortBlocksOnASingleLine and AllowShortLoopsOnASingleLine. This fixes bug-46087.