[clang] [clang-format] Fix operator overload inconsistency in `BreakAfterAttributes: Always` (PR #74943)

via cfe-commits cfe-commits at lists.llvm.org
Sun Dec 10 14:23:05 PST 2023


================
@@ -583,20 +583,31 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
       return true;
   }
 
-  // If the return type spans multiple lines, wrap before the function name.
-  if (((Current.is(TT_FunctionDeclarationName) &&
-        !State.Line->ReturnTypeWrapped &&
-        // Don't break before a C# function when no break after return type.
-        (!Style.isCSharp() ||
-         Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None) &&
-        // Don't always break between a JavaScript `function` and the function
-        // name.
-        !Style.isJavaScript()) ||
-       (Current.is(tok::kw_operator) && Previous.isNot(tok::coloncolon))) &&
-      Previous.isNot(tok::kw_template) && CurrentState.BreakBeforeParameter) {
+  const auto WrapBeforeName = [&]() {
----------------
XDeme wrote:

The original condition could be rewritten like this:
```cpp
  const auto WrapBeforeName = [&] {
    if(Previous.is(tok::kw_template) || !CurrentState.BreakBeforeParameter)
      return false;
    if((Current.isNot(tok::kw_operator) || Previous.is(tok::coloncolon)) && (Current.isNot(TT_FunctionDeclarationName) || State.Line->ReturnTypeWrapped))
      return false;
    // Don't break before a C# function when no break after return type.
    return (!Style.isCSharp() ||
            Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None) &&
           // Don't always break between a JavaScript `function` and the
           // function name.
           !Style.isJavaScript();
  };
```

The problem is that `Current.isNot(tok::kw_operator)` is returning false and then the last check will return true, forcing the return type of a operator overload to wrap.
We could fix this by adding a `Curret.is(tok::kw_operator) return false` before that line like this:
```cpp
  const auto WrapBeforeName = [&] {
    if(Previous.is(tok::kw_template) || !CurrentState.BreakBeforeParameter)
      return false;
    if(Current.is(tok::kw_operator))
      return false;
    if((Current.isNot(tok::kw_operator) || Previous.is(tok::coloncolon)) && (Current.isNot(TT_FunctionDeclarationName) || State.Line->ReturnTypeWrapped))
      return false;
    return (!Style.isCSharp() ||
            Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None) &&
           // Don't always break between a JavaScript `function` and the
           // function name.
           !Style.isJavaScript();
  };
```

I am not sure what the original conditional was exactly checking for, but my change didn't break any test.

And for the `ReferenceAlignment: Left` with `BreakAfterAttributes: Always` not working on non-member operator overload, it fixed by just changing this condition.

https://github.com/llvm/llvm-project/pull/74943


More information about the cfe-commits mailing list