This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] AllowShortRequiresExpressionOnASingleLine
Needs ReviewPublic

Authored by Backl1ght on Dec 11 2022, 7:39 AM.

Details

Summary

clang-format brace wrapping did not take requires into consideration, compound requirements will be affected by BraceWrapping.AfterFunction.

I plan to add 3 options AllowShortRequiresExpressionOnASingleLine, AllowShortCompoundRequirementOnASingleLine and BraceWrapping.AfterRequiresExpression.

This patch is for AllowShortRequiresExpressionOnASingleLine.

Diff Detail

Event Timeline

Backl1ght created this revision.Dec 11 2022, 7:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 11 2022, 7:39 AM
Backl1ght requested review of this revision.Dec 11 2022, 7:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 11 2022, 7:39 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
Backl1ght edited the summary of this revision. (Show Details)Dec 11 2022, 7:45 AM
Backl1ght edited the summary of this revision. (Show Details)Dec 11 2022, 7:48 AM
rymiel added a subscriber: rymiel.Dec 11 2022, 8:43 AM
clang/lib/Format/Format.cpp
811

I'm pretty sure these options should be in alphabetical order (and below too)

Backl1ght updated this revision to Diff 481918.Dec 11 2022, 9:13 AM

I don't know if adding two options at once (and I think you want to add a third), splitting that up might be a better idea.

Anyway you need to tun clang/docs/tools/dump_format_style.py and add an entry in the changelog.

clang/include/clang/Format/Format.h
542

We have both, but I like the variant with true and false next to each other, not below.

clang/lib/Format/UnwrappedLineParser.cpp
1105

Unrelated.

clang/unittests/Format/FormatTest.cpp
2911

You need to add the Style.

Backl1ght updated this revision to Diff 482062.Dec 12 2022, 3:48 AM
Backl1ght retitled this revision from [clang-format] AllowShortRequiresExpressionOnASingleLine and AllowShortCompoundRequirementOnASingleLine to [clang-format] AllowShortRequiresExpressionOnASingleLine.
Backl1ght edited the summary of this revision. (Show Details)
Backl1ght set the repository for this revision to rG LLVM Github Monorepo.
Backl1ght updated this revision to Diff 482066.Dec 12 2022, 3:51 AM

I just find out I miss one case like below

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

Since I split AllowShortRequiresExpressionOnASingleLine and AllowShortCompoundRequirementOnASingleLine into two pathes, I can not fix this until AllowShortCompoundRequirementOnASingleLine got merged.

Please mark comments as done.

clang/lib/Format/Format.cpp
1270

Please initialize to false, because that is what we are doing right now.

clang/unittests/Format/FormatTest.cpp
2904

Please add a test where the clause is too long and will still be wrapped. Reduce the column limit and use some longer identifiers.

ditto my concerns with D139834: [clang-format] AllowShortCompoundRequirementOnASingleLine as to if we should have a Leave option