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
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
https://lists.llvm.or
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 ForEachMa
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
clang/unittest
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);
verifyForm
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
clang/
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:
clang/lib/Format/UnwrappedLineFormatter.cp
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/
https://reviews.llvm.org/D9495
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
cf
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
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 b
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"
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/
https://revie
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
clang/lib/Format/Unwrap
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:
> >
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 s
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
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
clang/lib
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
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.
20 matches
Mail list logo
