search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.06k stars 11.98k forks source link

Excessive line breaks when using requires-clauses and requires-expressions #48250

Open 976866b7-fc02-4d23-a1c0-197b08fd7c0e opened 3 years ago

976866b7-fc02-4d23-a1c0-197b08fd7c0e commented 3 years ago
Bugzilla Link 48906
Version trunk
OS Linux
CC @JohelEGP

Extended Description

This was mentioned in D79773.

Since a compound requirement is neither a control statement nor a function, I suppose it might eventually need its own BraceWrapping nested configuration flag. For now, I'd prefer if they never break.

Input and expected formatted result:

template <typename T>
concept EqualityComparable = requires(T a, T b) {
  { a == b } -> bool;
};

.clang-format:

BraceWrapping:
  AfterFunction: true
BreakBeforeBraces: Custom

Actual output:

template <typename T>
concept EqualityComparable = requires(T a, T b)
{
  {
    a == b
    } -> bool;
};

.clang-format:

BraceWrapping:
  AfterControlStatement: MultiLine
BreakBeforeBraces: Custom

Actual output:

template <typename T>
concept EqualityComparable = requires(T a, T b) {
  {
    a == b
    } -> bool;
};

.clang-format:

BraceWrapping:
  AfterFunction: true
BreakBeforeBraces: Custom

Input and expected formatted result:

template <typename T> void f(T) requires requires(T a, T b) {
  { a == b } -> bool;
}
{}

Actual output:

template <typename T> void f(T) requires requires(T a, T b)
{
  {
    a == b
    } -> bool;
} {}
976866b7-fc02-4d23-a1c0-197b08fd7c0e commented 3 years ago

With the same .clang-format as in comment 2, having a requires-clause disrespects the options to keep things on a single line.

Input and expected formatted result:

template <class T> struct A {
    static constexpr bool B{sizeof(T) == 42};
    template <class T> struct rep { using type = T::value_type; };
    template <class T> requires B struct rep { using type = T::value_type; };
    template <class T> requires(B) struct rep { using type = T::value_type; };
    template <class T> void f() { }
    template <class T> requires B void f() { }
    template <class T> requires(B) void f() { }
};

Actual output:

template <class T> struct A {
    static constexpr bool B{sizeof(T) == 42};
    template <class T> struct rep { using type = T::value_type; };
    template <class T>
    requires B struct rep {
        using type = T::value_type;
    };
    template <class T>
        requires(B)
    struct rep {
        using type = T::value_type;
    };
    template <class T> void f() { }
    template <class T>
    requires B void f() { }
    template <class T>
        requires(B)
    void f() { }
};
976866b7-fc02-4d23-a1c0-197b08fd7c0e commented 3 years ago

Here's another case of more breaks than necessary.

Input and output:

/*export*/ template <units::Dimension D, units::UnitOf<D> U, units::QuantityValue Rep> struct subplane {
    [[nodiscard]] friend constexpr bool operator==(const subplane& l, const specialization_of<jge::subplane> auto& r) //
        requires std::equality_comparable_with<decltype(l.size), decltype(r.size)> {
        return l.top_left == r.top_left and l.size == r.size;
    }
};

Notice the // comment. Without it:

Input and expected formatted result:

/*export*/ template <units::Dimension D, units::UnitOf<D> U, units::QuantityValue Rep> struct subplane {
    [[nodiscard]] friend constexpr bool operator==(const subplane& l, const specialization_of<jge::subplane> auto& r)
        requires std::equality_comparable_with<decltype(l.size), decltype(r.size)> {
        return l.top_left == r.top_left and l.size == r.size;
    }
};

Actual output:

/*export*/ template <units::Dimension D, units::UnitOf<D> U, units::QuantityValue Rep> struct subplane {
    [[nodiscard]] friend constexpr bool
    operator==(const subplane& l, const specialization_of<jge::subplane> auto& r) requires
        std::equality_comparable_with<decltype(l.size), decltype(r.size)> {
        return l.top_left == r.top_left and l.size == r.size;
    }
};

.clang-format:

# Clang 11
AccessModifierOffset: -4
AlignAfterOpenBracket: AlwaysBreak
AlignConsecutiveAssignments: true
AlignConsecutiveBitFields: true
AlignConsecutiveDeclarations: false
AlignEscapedNewlines: Right
AlignOperands: Align
AlignTrailingComments: true
AllowAllArgumentsOnNextLine: true
AllowAllConstructorInitializersOnNextLine: true
AllowAllParametersOfDeclarationOnNextLine: true
AllowShortBlocksOnASingleLine: Always
AllowShortCaseLabelsOnASingleLine: true
AllowShortEnumsOnASingleLine: true
AllowShortFunctionsOnASingleLine: All
AllowShortIfStatementsOnASingleLine: Always
AllowShortLambdasOnASingleLine: All
AllowShortLoopsOnASingleLine: true
AlwaysBreakAfterReturnType: None
AlwaysBreakBeforeMultilineStrings: false
AlwaysBreakTemplateDeclarations: No
BinPackArguments: true
BinPackParameters: true
BitFieldColonSpacing: Both
BreakBeforeBinaryOperators: None
BreakBeforeBraces: Attach
BreakBeforeConceptDeclarations: false
BreakBeforeTernaryOperators: true
BreakConstructorInitializers: BeforeColon
BreakInheritanceList: BeforeColon
BreakStringLiterals: true
ColumnLimit: 120
ConstructorInitializerAllOnOneLineOrOnePerLine: true
ConstructorInitializerIndentWidth: 2
ContinuationIndentWidth: 4
Cpp11BracedListStyle: true
DeriveLineEnding: false
DerivePointerAlignment: false
DisableFormat: false
FixNamespaceComments: true
IncludeBlocks: Merge
IncludeCategories:
    -   Regex:      '<[a-z_]+>' # C++ standard library
        Priority:   1
IndentCaseBlocks: false
IndentCaseLabels: false
IndentPPDirectives: AfterHash
IndentRequires: true
IndentWidth: 4
IndentWrappedFunctionNames: false
KeepEmptyLinesAtTheStartOfBlocks: false
Language: Cpp
MaxEmptyLinesToKeep: 1
NamespaceIndentation: Inner
PointerAlignment: Left
ReflowComments: true
SortIncludes: true
SortUsingDeclarations: true
SpaceAfterLogicalNot: true
SpaceAfterTemplateKeyword: true
SpaceBeforeAssignmentOperators: true
SpaceBeforeCaseColon: false
SpaceBeforeCpp11BracedList: false
SpaceBeforeCtorInitializerColon: true
SpaceBeforeInheritanceColon: true
SpaceBeforeParens: ControlStatements
SpaceBeforeRangeBasedForLoopColon: true
SpaceBeforeSquareBrackets: false
SpaceInEmptyBlock: true
SpaceInEmptyParentheses: false
SpacesBeforeTrailingComments: 1
SpacesInAngles: false
SpacesInConditionalStatement: false
SpacesInParentheses: false
SpacesInSquareBrackets: false
Standard: c++20
TabWidth: 4
UseCRLF: false
UseTab: Never
976866b7-fc02-4d23-a1c0-197b08fd7c0e commented 3 years ago

Actually:

Input and expected formatted result:

template <typename T>
concept EqualityComparable = requires(T a, T b) { { a == b } -> bool; };

Input and expected formatted result:

template <typename T> void f(T) requires requires(T a, T b) { { a == b } -> bool; }
{}

That's because for other constructs, ClangFormat tries to minimize lines while following the config. Similar to functions and lambda bodies with two statements, I'd expect those to always be broken into multiple lines.

JohelEGP commented 2 years ago

Notwithstanding #47925, https://github.com/llvm/llvm-project/issues/48250#issuecomment-981037929 seems fixed.

llvmbot commented 1 year ago

@llvm/issue-subscribers-c-20

JohelEGP commented 11 months ago

I suspect this is resolved with commit 69209e30a7168b0493d8fb34989ddccd8c574670.

tuero commented 10 months ago

I'm still getting a similar issue after building on main, so I don't think this issue is fixed.

Expected behaviour with AllowShortCompoundRequirementOnASingleLine: true

template <typename T>
concept c = requires(T x) {
  { x + 1 } -> std::same_as<int>;
};

Actual behaviour:

template <typename T>
concept c = requires(T x) {
  {
    x + 1
  } -> std::same_as<int>;
};

From this, it appears there are missing configuration options: AllowShortRequiresExpressionOnASingleLine and BraceWrapping.AfterRequiresExpression, but I don't see any follow up. https://reviews.llvm.org/D139786