rymiel created this revision. rymiel added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay. Herald added a project: All. rymiel requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
Previously, clang-format relied on a special method to parse concept definitions, `UnwrappedLineParser::parseConcept()`, which deferred to `UnwrappedLineParser::parseConstraintExpression()`. This is problematic, because the C++ grammar treats concepts and requires clauses differently, causing issues such as https://github.com/llvm/llvm-project/issues/55898 and https://github.com/llvm/llvm-project/issues/58130. This patch removes `parseConcept`, letting the formatter parse concept definitions as more like what they actually are, fancy bool definitions. NOTE that because of this, some long concept definitions change in their formatting, as can be seen in the changed tests. This is because of a change in split penalties, caused by a change in MightBeFunctionDecl on the concept definition line, which was previously `true` but with this patch is now `false`. One might argue that `false` is a more "correct" value for concept definitions, but I'd be fine with setting it to `true` again to maintain compatibility with previous versions. Fixes https://github.com/llvm/llvm-project/issues/58130 Depends on D140330 <https://reviews.llvm.org/D140330> Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D140339 Files: clang/lib/Format/TokenAnnotator.cpp clang/lib/Format/UnwrappedLineParser.cpp clang/lib/Format/UnwrappedLineParser.h clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp =================================================================== --- clang/unittests/Format/FormatTest.cpp +++ clang/unittests/Format/FormatTest.cpp @@ -23588,10 +23588,10 @@ "concept DelayedCheck = !!false || requires(T t) { t.bar(); } " "&& sizeof(T) <= 8;"); - verifyFormat( - "template <typename T>\n" - "concept DelayedCheck = static_cast<bool>(0) ||\n" - " requires(T t) { t.bar(); } && sizeof(T) <= 8;"); + verifyFormat("template <typename T>\n" + "concept DelayedCheck =\n" + " static_cast<bool>(0) || requires(T t) { t.bar(); } && " + "sizeof(T) <= 8;"); verifyFormat("template <typename T>\n" "concept DelayedCheck = bool(0) || requires(T t) { t.bar(); } " @@ -23599,8 +23599,8 @@ verifyFormat( "template <typename T>\n" - "concept DelayedCheck = (bool)(0) ||\n" - " requires(T t) { t.bar(); } && sizeof(T) <= 8;"); + "concept DelayedCheck =\n" + " (bool)(0) || requires(T t) { t.bar(); } && sizeof(T) <= 8;"); verifyFormat("template <typename T>\n" "concept DelayedCheck = (bool)0 || requires(T t) { t.bar(); } " @@ -23636,6 +23636,9 @@ verifyFormat("template <typename T>\n" "concept True = S<T>::Value;"); + verifyFormat("template <S T>\n" + "concept True = T.field;"); + verifyFormat( "template <typename T>\n" "concept C = []() { return true; }() && requires(T t) { t.bar(); } &&\n" @@ -23914,11 +23917,9 @@ verifyFormat("template <typename T> concept True = true;", Style); verifyFormat( - "template <typename T> concept C = decltype([]() -> std::true_type {\n" - " return {};\n" - " }())::value &&\n" - " requires(T t) { t.bar(); } && " - "sizeof(T) <= 8;", + "template <typename T> concept C =\n" + " decltype([]() -> std::true_type { return {}; }())::value &&\n" + " requires(T t) { t.bar(); } && sizeof(T) <= 8;", Style); verifyFormat("template <typename T> concept Semiregular =\n" @@ -23936,15 +23937,15 @@ // The following tests are invalid C++, we just want to make sure we don't // assert. - verifyFormat("template <typename T>\n" - "concept C = requires C2<T>;"); - - verifyFormat("template <typename T>\n" - "concept C = 5 + 4;"); - verifyFormat("template <typename T>\n" "concept C =\n" - "class X;"); + " requires C2<T>;"); + + verifyFormat("template <typename T>\n" + "concept C = 5 + 4;"); + + verifyFormat("template <typename T>\n" + "concept C = class X;"); verifyFormat("template <typename T>\n" "concept C = [] && true;"); Index: clang/lib/Format/UnwrappedLineParser.h =================================================================== --- clang/lib/Format/UnwrappedLineParser.h +++ clang/lib/Format/UnwrappedLineParser.h @@ -158,7 +158,6 @@ void parseAccessSpecifier(); bool parseEnum(); bool parseStructLike(); - void parseConcept(); bool parseRequires(); void parseRequiresClause(FormatToken *RequiresToken); void parseRequiresExpression(FormatToken *RequiresToken); Index: clang/lib/Format/UnwrappedLineParser.cpp =================================================================== --- clang/lib/Format/UnwrappedLineParser.cpp +++ clang/lib/Format/UnwrappedLineParser.cpp @@ -1813,9 +1813,6 @@ break; } break; - case tok::kw_concept: - parseConcept(); - return; case tok::kw_requires: { if (Style.isCpp()) { bool ParsedClause = parseRequires(); @@ -3272,26 +3269,6 @@ } } -/// \brief Parses a concept definition. -/// \pre The current token has to be the concept keyword. -/// -/// Returns if either the concept has been completely parsed, or if it detects -/// that the concept definition is incorrect. -void UnwrappedLineParser::parseConcept() { - assert(FormatTok->is(tok::kw_concept) && "'concept' expected"); - nextToken(); - if (!FormatTok->is(tok::identifier)) - return; - nextToken(); - if (!FormatTok->is(tok::equal)) - return; - nextToken(); - parseConstraintExpression(); - if (FormatTok->is(tok::semi)) - nextToken(); - addUnwrappedLine(); -} - /// \brief Parses a requires, decides if it is a clause or an expression. /// \pre The current token has to be the requires keyword. /// \returns true if it parsed a clause. Index: clang/lib/Format/TokenAnnotator.cpp =================================================================== --- clang/lib/Format/TokenAnnotator.cpp +++ clang/lib/Format/TokenAnnotator.cpp @@ -1680,8 +1680,8 @@ if (!Tok) return false; - if (Tok->isOneOf(tok::kw_class, tok::kw_enum, tok::kw_concept, - tok::kw_struct, tok::kw_using)) { + if (Tok->isOneOf(tok::kw_class, tok::kw_enum, tok::kw_struct, + tok::kw_using)) { return false; }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits