Skip to content

[clang-format] Incorrect method body when combining trailing return type & decltype in requires clause #78645

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

Closed
helmesjo opened this issue Jan 18, 2024 · 2 comments · Fixed by #78847
Assignees
Labels
bug Indicates an unexpected problem or unintended behavior clang-format

Comments

@helmesjo
Copy link

helmesjo commented Jan 18, 2024

Continuation of #56176 (@HazardyKnusperkeks).

Tweaking the original example in that issue by adding & to the trailing return and using decltype(...) in the requires clause breaks it:

  template<typename T>
  struct Foo
  {
    auto bar(T in) -> int& requires (is_integral_v<decltype(in)>) {
                             call_something();
                             return 0;
                           }
  };

Changing one of the following fixes the formatting again:

  • -> int& to -> int
  • decltype(in) to T
  • requires (...) to requires ...

You need parens if you negate ((!...)), but I guess a (highly arbitrary looking) workaround is to change from
requires (is_integral_v<decltype(in)>)
to
requires is_integral_v<decltype(in)> || true
works (tested).

$ clang++ --version
Homebrew clang version 17.0.6
@helmesjo helmesjo changed the title clang-format: Incorrect method body when combined with trailing return type & decltype in requires clause clang-format: Incorrect method body when combining with trailing return type & decltype in requires clause Jan 18, 2024
@helmesjo helmesjo changed the title clang-format: Incorrect method body when combining with trailing return type & decltype in requires clause clang-format: Incorrect method body when combining trailing return type & decltype in requires clause Jan 18, 2024
@helmesjo helmesjo changed the title clang-format: Incorrect method body when combining trailing return type & decltype in requires clause [clang-format] Incorrect method body when combining trailing return type & decltype in requires clause Jan 18, 2024
@EugeneZelenko EugeneZelenko added the invalid-code-generation Tool (e.g. clang-format) produced invalid code that no longer compiles label Jan 19, 2024
@HazardyKnusperkeks HazardyKnusperkeks added bug Indicates an unexpected problem or unintended behavior and removed invalid-code-generation Tool (e.g. clang-format) produced invalid code that no longer compiles labels Jan 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 19, 2024

@llvm/issue-subscribers-bug

Author: Fred Helmesjö (helmesjo)

Continuation of #56176 (@HazardyKnusperkeks).

Tweaking the original example in that issue by adding &amp; to the trailing return and using decltype(...) in the requires statement breaks it:

  template&lt;typename T&gt;
  struct Foo
  {
    auto bar(T in) -&gt; int&amp; requires (is_integral_v&lt;decltype(in)&gt;) {
                             call_something();
                             return 0;
                           }
  };

Changing one of the following fixes the formatting again:

  • -&gt; int&amp; to -&gt; int
  • decltype(in) to T
  • requires (...) to requires ...

You need parens if you negate ((!...)), but I guess a (highly arbitrary looking) workaround is to change from
requires (is_integral_v&lt;decltype(in)&gt;)
to
requires is_integral_v&lt;decltype(in)&gt; || true
works (tested).

$ clang++ --version
Homebrew clang version 17.0.6

@rymiel
Copy link
Member

rymiel commented Jan 20, 2024

Note that this can only be reproduced if using the option RequiresExpressionIndentation: Keyword (please provide the configuration in the future so it can be reproduced better)

The requires keyword is annotated as TT_RequiresExpression, instead of TT_RequiresClause as expected

@rymiel rymiel self-assigned this Jan 20, 2024
rymiel added a commit to rymiel/llvm-project that referenced this issue Jan 20, 2024
If clang-format is not sure whether a `requires` keyword starts a
requires clause or a requires expression, it looks ahead to see if any
token disqualifies it from being a requires clause. Among these tokens
was `decltype`, since it fell through the switch.

This patch allows decltype to exist in a require clause.

I'm not 100% sure this change won't have repercussions, but that just means
we need more test coverage!

Fixes llvm#78645
rymiel added a commit that referenced this issue Feb 1, 2024
If clang-format is not sure whether a `requires` keyword starts a
requires clause or a requires expression, it looks ahead to see if any
token disqualifies it from being a requires clause. Among these tokens
was `decltype`, since it fell through the switch.

This patch allows decltype to exist in a require clause.

I'm not 100% sure this change won't have repercussions, but that just
means we need more test coverage!

Fixes #78645
carlosgalvezp pushed a commit to carlosgalvezp/llvm-project that referenced this issue Feb 1, 2024
If clang-format is not sure whether a `requires` keyword starts a
requires clause or a requires expression, it looks ahead to see if any
token disqualifies it from being a requires clause. Among these tokens
was `decltype`, since it fell through the switch.

This patch allows decltype to exist in a require clause.

I'm not 100% sure this change won't have repercussions, but that just
means we need more test coverage!

Fixes llvm#78645
agozillon pushed a commit to agozillon/llvm-project that referenced this issue Feb 5, 2024
If clang-format is not sure whether a `requires` keyword starts a
requires clause or a requires expression, it looks ahead to see if any
token disqualifies it from being a requires clause. Among these tokens
was `decltype`, since it fell through the switch.

This patch allows decltype to exist in a require clause.

I'm not 100% sure this change won't have repercussions, but that just
means we need more test coverage!

Fixes llvm#78645
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior clang-format
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants