This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Add missing test for std::function<void( int, int )> spacing also fixed by r230473.
ClosedPublic

Authored by jeanphilippeD on Feb 26 2015, 4:37 PM.

Details

Reviewers
djasper
Summary

The fix in r230473 was done to enable fixing the spacing for std::function<void( int, int )>.
I did not realized that it also fixed this issue.

Since it is fairly different from "Deleted &operator=(const Deleted &)& = default;" fixed in r230473, it seems sensible to add the regression test for it.

Also cleaned up the test by removing duplicated code and comment, and kept repeated test set consistent.

Result of running the new tests with r230473 backed out:

[ RUN      ] FormatTest.ConfigurableSpacesInParentheses
Actual: "std::function<void(int, int)> callback;"
Expected: "std::function<void( int, int )> callback;"
Actual: "std::function<void( int, int )> callback;"
Expected: "std::function<void(int, int)> callback;"
Actual: "std::function<void( int, int ) > callback;"
Expected: "std::function<void(int, int)> callback;"
[  FAILED  ] FormatTest.ConfigurableSpacesInParentheses (402 ms)

Result of new tests with r230473:
[ RUN ] FormatTest.ConfigurableSpacesInParentheses
[ OK ] FormatTest.ConfigurableSpacesInParentheses (209 ms)

Diff Detail

Event Timeline

jeanphilippeD retitled this revision from to [clang-format] Add missing test for std::function<void( int, int )> spacing also fixed by r230473..
jeanphilippeD updated this object.
jeanphilippeD edited the test plan for this revision. (Show Details)
jeanphilippeD added a subscriber: Unknown Object (MLST).

Hello,
Not sure if this was missed or was just not really interesting.
Thanks,
Jean-Philippe

2015-02-27 0:37 GMT+00:00 Jean-Philippe Dufraigne <j.dufraigne@gmail.com>:

The fix in r230473 was done to enable fixing the spacing for
std::function<void( int, int )>.
I did not realized that it also fixed this issue.

Since it is fairly different from "Deleted &operator=(const Deleted &)& =
default;" fixed in r230473, it seems sensible to add the regression test
for it.

Also cleaned up the test by removing duplicated code and comment, and kept
repeated test set consistent.

Result of running the new tests with r230473 backed out:

[ RUN      ] FormatTest.ConfigurableSpacesInParentheses
Actual: "std::function<void(int, int)> callback;"
Expected: "std::function<void( int, int )> callback;"
Actual: "std::function<void( int, int )> callback;"
Expected: "std::function<void(int, int)> callback;"
Actual: "std::function<void( int, int ) > callback;"
Expected: "std::function<void(int, int)> callback;"
[  FAILED  ] FormatTest.ConfigurableSpacesInParentheses (402 ms)

Result of new tests with r230473:
[ RUN ] FormatTest.ConfigurableSpacesInParentheses
[ OK ] FormatTest.ConfigurableSpacesInParentheses (209 ms)

http://reviews.llvm.org/D7922

Files:

unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp

  • unittests/Format/FormatTest.cpp

+++ unittests/Format/FormatTest.cpp
@@ -8148,6 +8148,8 @@

Spaces.SpacesInParentheses = true;
verifyFormat("call( x, y, z );", Spaces);

+ verifyFormat("call();", Spaces);
+ verifyFormat("std::function<void( int, int )> callback;", Spaces);

verifyFormat("while ( (bool)1 )\n"
             "  continue;", Spaces);
verifyFormat("for ( ;; )\n"

@@ -8174,19 +8176,13 @@

verifyFormat("my_int a = ( my_int )sizeof(int);", Spaces);
verifyFormat("#define x (( int )-1)", Spaces);
  • Spaces.SpacesInParentheses = false;
  • Spaces.SpaceInEmptyParentheses = true;
  • verifyFormat("call(x, y, z);", Spaces);
  • verifyFormat("call( )", Spaces); -
  • // Run the first set of tests again with
  • // Spaces.SpacesInParentheses = false,
  • // Spaces.SpaceInEmptyParentheses = true and
  • // Spaces.SpacesInCStyleCastParentheses = true

+ // Run the first set of tests again with:

Spaces.SpacesInParentheses = false,
Spaces.SpaceInEmptyParentheses = true;
Spaces.SpacesInCStyleCastParentheses = true;
verifyFormat("call(x, y, z);", Spaces);

+ verifyFormat("call( );", Spaces);
+ verifyFormat("std::function<void(int, int)> callback;", Spaces);

verifyFormat("while (( bool )1)\n"
             "  continue;", Spaces);
verifyFormat("for (;;)\n"

@@ -8203,8 +8199,11 @@

"  break;\n"
"}", Spaces);

+ // Run the first set of tests again with:

Spaces.SpaceAfterCStyleCast = true;
verifyFormat("call(x, y, z);", Spaces);

+ verifyFormat("call( );", Spaces);
+ verifyFormat("std::function<void(int, int)> callback;", Spaces);

verifyFormat("while (( bool ) 1)\n"
             "  continue;",
             Spaces);

@@ -8225,6 +8224,8 @@

"  break;\n"
"}",
Spaces);

+
+ // Run subset of tests again with:

Spaces.SpacesInCStyleCastParentheses = false;
Spaces.SpaceAfterCStyleCast = true;
verifyFormat("while ((bool) 1)\n"

EMAIL PREFERENCES

http://reviews.llvm.org/settings/panel/emailpreferences/
djasper accepted this revision.Mar 17 2015, 3:15 PM
djasper added a reviewer: djasper.
djasper added a subscriber: djasper.

Yes, it was missed, sorry. Looks good.

This revision is now accepted and ready to land.Mar 17 2015, 3:15 PM

No problem, thanks a lot for the review.
I do not have commit access, so I'd be happy for you to commit this.

I hope the patch still applies cleanly since this test function was not modified since the patch was created, but let me know if you need me to rebase it and I'll do it an re-run the tests.

djasper closed this revision.Mar 18 2015, 6:02 AM

Submitted in r232632.