Skip to content

[clang-format] Fix a bug in indenting lambda trailing arrows #94560

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 10, 2024

Conversation

c8ef
Copy link
Contributor

@c8ef c8ef commented Jun 6, 2024

Closes #94181

@llvmbot
Copy link
Member

llvmbot commented Jun 6, 2024

@llvm/pr-subscribers-clang-format

Author: None (c8ef)

Changes

close: #94181


Full diff: https://github.com/llvm/llvm-project/pull/94560.diff

2 Files Affected:

  • (modified) clang/lib/Format/ContinuationIndenter.cpp (+5)
  • (modified) clang/unittests/Format/FormatTest.cpp (+25)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 6b9fbfe0ebf53..630a4ebff9843 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -1457,6 +1457,11 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
       !Current.isOneOf(tok::colon, tok::comment)) {
     return ContinuationIndent;
   }
+  if (Style.isCpp() && Current.is(tok::arrow) &&
+      Previous.isOneOf(tok::kw_noexcept, tok::kw_mutable, tok::kw_constexpr,
+                       tok::kw_consteval, tok::kw_static)) {
+    return ContinuationIndent;
+  }
   if (Current.is(TT_ProtoExtensionLSquare))
     return CurrentState.Indent;
   if (Current.isBinaryOperator() && CurrentState.UnindentOperator) {
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 4e427268fb82a..d117efc06c2a7 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -22858,6 +22858,31 @@ TEST_F(FormatTest, FormatsLambdas) {
       "      //\n"
       "    });");
 
+  verifyFormat("int main() {\n"
+               "  very_long_function_name_yes_it_is_really_long(\n"
+               "      [](auto n)\n"
+               "          -> std::unordered_map<very_long_type_name_A, "
+               "very_long_type_name_B> {\n"
+               "        really_do_something();\n"
+               "      });\n"
+               "}");
+  verifyFormat("int main() {\n"
+               "  very_long_function_name_yes_it_is_really_long(\n"
+               "      [](auto n) noexcept\n"
+               "          -> std::unordered_map<very_long_type_name_A, "
+               "very_long_type_name_B> {\n"
+               "        really_do_something();\n"
+               "      });\n"
+               "}");
+  verifyFormat("int main() {\n"
+               "  very_long_function_name_yes_it_is_really_long(\n"
+               "      [](auto n) constexpr\n"
+               "          -> std::unordered_map<very_long_type_name_A, "
+               "very_long_type_name_B> {\n"
+               "        really_do_something();\n"
+               "      });\n"
+               "}");
+
   FormatStyle DoNotMerge = getLLVMStyle();
   DoNotMerge.AllowShortLambdasOnASingleLine = FormatStyle::SLS_None;
   verifyFormat("auto c = []() {\n"

@@ -1457,6 +1457,11 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
!Current.isOneOf(tok::colon, tok::comment)) {
return ContinuationIndent;
}
if (Style.isCpp() && Current.is(tok::arrow) &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (Style.isCpp() && Current.is(tok::arrow) &&
if (Style.isCpp() && Current.is(TT_TrailingReturnArrow) &&

This is what you need, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, thanks for your suggestions!

@c8ef c8ef requested a review from HazardyKnusperkeks June 7, 2024 02:17
@@ -1457,7 +1457,7 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
!Current.isOneOf(tok::colon, tok::comment)) {
return ContinuationIndent;
}
if (Style.isCpp() && Current.is(tok::arrow) &&
if (Style.isCpp() && Current.is(TT_TrailingReturnArrow) &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think you can drop the isCpp.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@c8ef c8ef requested a review from HazardyKnusperkeks June 8, 2024 08:13
@rymiel
Copy link
Member

rymiel commented Jun 9, 2024

Technically also applies to attributes applied to the return type of the lambda, i.e.

int main() {
  very_long_function_name_yes_it_is_really_long(
      // also happens with constexpr specifier
      [](auto n) [[attribute]]
      -> std::enable_if_t<
          std::is_arithmetic<decltype(n)>::value &&
          !std::is_same<std::remove_cv_t<decltype(n)>, bool>::value> {
        do_something(n * 2);
      });
}

...but currently at least no valid C++ attribute can go in that spot

@owenca
Copy link
Contributor

owenca commented Jun 10, 2024

Technically also applies to attributes applied to the return type of the lambda, i.e.

int main() {
  very_long_function_name_yes_it_is_really_long(
      // also happens with constexpr specifier
      [](auto n) [[attribute]]
      -> std::enable_if_t<
          std::is_arithmetic<decltype(n)>::value &&
          !std::is_same<std::remove_cv_t<decltype(n)>, bool>::value> {
        do_something(n * 2);
      });
}

...but currently at least no valid C++ attribute can go in that spot

We can easily take care of it though. See #94560 (comment).

@c8ef
Copy link
Contributor Author

c8ef commented Jun 10, 2024

Dear reviewers, is there anything else I need to do for this PR? If not, could you please assist me in merging it? Thank you!

@owenca owenca changed the title [clang-format] fix incorrectly indents lambda trailing return [clang-format] Fix a bug in indenting lambda trailing arrows Jun 10, 2024
@owenca owenca merged commit d9593c1 into llvm:main Jun 10, 2024
7 checks passed
@c8ef c8ef deleted the format branch June 11, 2024 03:35
Lukacma pushed a commit to Lukacma/llvm-project that referenced this pull request Jun 12, 2024
@HerrCai0907 HerrCai0907 mentioned this pull request Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants