Author: owenca Date: 2026-06-05T22:03:18-07:00 New Revision: 11d7939060e874cdc8378e2b711c719b0d4ba8e1
URL: https://github.com/llvm/llvm-project/commit/11d7939060e874cdc8378e2b711c719b0d4ba8e1 DIFF: https://github.com/llvm/llvm-project/commit/11d7939060e874cdc8378e2b711c719b0d4ba8e1.diff LOG: [clang-format] Keep C++20 module/import decls on a single line (#199459) This patch fixes #193676. - Added `UnwrappedLineParser::parseModuleDecl()` to parse C++20 module declarations. - Adapted `parseCppModuleImport()` from #193834 and renamed it to `parseImportDecl()`. - Used the test cases from the same PR. - Removed the invalid test cases and fixed an incorrect one in `FormatTest.cpp`. --------- Co-authored-by: Björn Schäpers <[email protected]> Added: Modified: clang/lib/Format/TokenAnnotator.cpp clang/lib/Format/TokenAnnotator.h clang/lib/Format/UnwrappedLineParser.cpp clang/lib/Format/UnwrappedLineParser.h clang/unittests/Format/FormatTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 43e4f6796b6dd..90c9466fdf431 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -1399,11 +1399,7 @@ class AnnotatingParser { } break; } - if (Line.First->isOneOf(Keywords.kw_module, Keywords.kw_import) || - Line.First->startsSequence(tok::kw_export, Keywords.kw_module) || - Line.First->startsSequence(tok::kw_export, Keywords.kw_import)) { - Tok->setType(TT_ModulePartitionColon); - } else if (Line.First->is(tok::kw_asm)) { + if (Line.First->is(tok::kw_asm)) { Tok->setType(TT_InlineASMColon); } else if (Contexts.back().ColonIsDictLiteral || Style.isProto()) { Tok->setType(TT_DictLiteral); @@ -4314,7 +4310,8 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) const { } Current->CanBreakBefore = - Current->MustBreakBefore || canBreakBefore(Line, *Current); + !Line.IsModuleOrImportDecl && + (Current->MustBreakBefore || canBreakBefore(Line, *Current)); if (Current->is(TT_FunctionDeclarationLParen)) { InParameterList = true; diff --git a/clang/lib/Format/TokenAnnotator.h b/clang/lib/Format/TokenAnnotator.h index 52d6e5ca56915..264f39b7b1d60 100644 --- a/clang/lib/Format/TokenAnnotator.h +++ b/clang/lib/Format/TokenAnnotator.h @@ -58,6 +58,7 @@ class AnnotatedLine { InPPDirective(Line.InPPDirective), InPragmaDirective(Line.InPragmaDirective), InMacroBody(Line.InMacroBody), + IsModuleOrImportDecl(Line.IsModuleOrImportDecl), MustBeDeclaration(Line.MustBeDeclaration), MightBeFunctionDecl(false), IsMultiVariableDeclStmt(false), Affected(false), LeadingEmptyLinesAffected(false), ChildrenAffected(false), @@ -184,6 +185,7 @@ class AnnotatedLine { bool InPPDirective; bool InPragmaDirective; bool InMacroBody; + bool IsModuleOrImportDecl; bool MustBeDeclaration; bool MightBeFunctionDecl; bool IsMultiVariableDeclStmt; diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index f73fe0fce4a9e..c83e82674dee1 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -108,6 +108,7 @@ class ScopedLineState { Parser.Line->PPLevel = PreBlockLine->PPLevel; Parser.Line->InPPDirective = PreBlockLine->InPPDirective; Parser.Line->InMacroBody = PreBlockLine->InMacroBody; + Parser.Line->IsModuleOrImportDecl = PreBlockLine->IsModuleOrImportDecl; Parser.Line->UnbracedBodyLevel = PreBlockLine->UnbracedBodyLevel; } @@ -1350,40 +1351,60 @@ static bool isC78ParameterDecl(const FormatToken *Tok, const FormatToken *Next, return Tok->Previous && Tok->Previous->isOneOf(tok::l_paren, tok::comma); } -bool UnwrappedLineParser::parseModuleImport() { - assert(FormatTok->is(Keywords.kw_import) && "'import' expected"); +bool UnwrappedLineParser::parseModuleDecl() { + assert(IsCpp); + assert(FormatTok->is(Keywords.kw_module)); - if (auto Token = Tokens->peekNextToken(/*SkipComment=*/true); - !Token->Tok.getIdentifierInfo() && - Token->isNoneOf(tok::colon, tok::less, tok::string_literal)) { + if (Style.Language == FormatStyle::LK_C || + Style.Standard < FormatStyle::LS_Cpp20) { return false; } nextToken(); - while (!eof()) { - if (FormatTok->is(tok::colon)) { + if (FormatTok->isNot(tok::identifier)) + return false; + + for (nextToken(); FormatTok->isNoneOf(tok::semi, tok::eof); nextToken()) + if (FormatTok->is(tok::colon)) FormatTok->setFinalizedType(TT_ModulePartitionColon); - } + + nextToken(); + Line->IsModuleOrImportDecl = true; + addUnwrappedLine(); + return true; +} + +bool UnwrappedLineParser::parseImportDecl() { + assert(IsCpp); + assert(FormatTok->is(Keywords.kw_import) && "'import' expected"); + + if (Style.Language == FormatStyle::LK_C || + Style.Standard < FormatStyle::LS_Cpp20) { + return false; + } + + nextToken(); + if (FormatTok->is(tok::colon)) { + FormatTok->setFinalizedType(TT_ModulePartitionColon); + nextToken(); + } + if (FormatTok->isNoneOf(tok::identifier, tok::less, tok::string_literal)) + return false; + + for (; FormatTok->isNoneOf(tok::semi, tok::eof); nextToken()) { // Handle import <foo/bar.h> as we would an include statement. - else if (FormatTok->is(tok::less)) { - nextToken(); - while (FormatTok->isNoneOf(tok::semi, tok::greater) && !eof()) { - // Mark tokens up to the trailing line comments as implicit string - // literals. - if (FormatTok->isNot(tok::comment) && - !FormatTok->TokenText.starts_with("//")) { - FormatTok->setFinalizedType(TT_ImplicitStringLiteral); - } - nextToken(); + if (FormatTok->is(tok::less)) { + for (nextToken(); FormatTok->isNoneOf(tok::greater, tok::semi, tok::eof); + nextToken()) { + // Mark tokens as implicit string literals, so that import <A/Foo> will + // neither be broken nor have a space added. + FormatTok->setFinalizedType(TT_ImplicitStringLiteral); } } - if (FormatTok->is(tok::semi)) { - nextToken(); - break; - } - nextToken(); } + nextToken(); + Line->IsModuleOrImportDecl = true; addUnwrappedLine(); return true; } @@ -1625,14 +1646,6 @@ void UnwrappedLineParser::parseStructuralElement( } break; case tok::kw_export: - if (Style.isJavaScript()) { - parseJavaScriptEs6ImportExport(); - return; - } - if (Style.isVerilog()) { - parseVerilogExtern(); - return; - } if (IsCpp) { nextToken(); if (FormatTok->is(tok::kw_namespace)) { @@ -1643,8 +1656,19 @@ void UnwrappedLineParser::parseStructuralElement( parseCppExportBlock(); return; } - if (FormatTok->is(Keywords.kw_import) && parseModuleImport()) + if (FormatTok->is(Keywords.kw_module) && parseModuleDecl()) return; + if (FormatTok->is(Keywords.kw_import) && parseImportDecl()) + return; + break; + } + if (Style.isJavaScript()) { + parseJavaScriptEs6ImportExport(); + return; + } + if (Style.isVerilog()) { + parseVerilogExtern(); + return; } break; case tok::kw_inline: @@ -1665,6 +1689,8 @@ void UnwrappedLineParser::parseStructuralElement( return; } if (FormatTok->is(Keywords.kw_import)) { + if (IsCpp && parseImportDecl()) + return; if (Style.isJavaScript()) { parseJavaScriptEs6ImportExport(); return; @@ -1685,25 +1711,27 @@ void UnwrappedLineParser::parseStructuralElement( parseVerilogExtern(); return; } - if (IsCpp && parseModuleImport()) - return; } - if (IsCpp && FormatTok->isOneOf(Keywords.kw_signals, Keywords.kw_qsignals, - Keywords.kw_slots, Keywords.kw_qslots)) { - nextToken(); - if (FormatTok->is(tok::colon)) { + if (IsCpp) { + if (FormatTok->is(Keywords.kw_module) && parseModuleDecl()) + return; + if (FormatTok->isOneOf(Keywords.kw_signals, Keywords.kw_qsignals, + Keywords.kw_slots, Keywords.kw_qslots)) { nextToken(); - addUnwrappedLine(); + if (FormatTok->is(tok::colon)) { + nextToken(); + addUnwrappedLine(); + return; + } + } + if (FormatTok->is(TT_StatementMacro)) { + parseStatementMacro(); + return; + } + if (FormatTok->is(TT_NamespaceMacro)) { + parseNamespace(); return; } - } - if (IsCpp && FormatTok->is(TT_StatementMacro)) { - parseStatementMacro(); - return; - } - if (IsCpp && FormatTok->is(TT_NamespaceMacro)) { - parseNamespace(); - return; } // In Verilog labels can be any expression, so we don't do them here. // JS doesn't have macros, and within classes colons indicate fields, not diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h index cd92866756c5a..5014fad7e6ef1 100644 --- a/clang/lib/Format/UnwrappedLineParser.h +++ b/clang/lib/Format/UnwrappedLineParser.h @@ -48,6 +48,8 @@ struct UnwrappedLine { bool InPragmaDirective = false; /// Whether it is part of a macro body. bool InMacroBody = false; + /// Whether it is a C++20 module/import declaration. + bool IsModuleOrImportDecl = false; /// Nesting level of unbraced body of a control statement. unsigned UnbracedBodyLevel = 0; @@ -164,7 +166,8 @@ class UnwrappedLineParser { void parseCaseLabel(); void parseSwitch(bool IsExpr); void parseNamespace(); - bool parseModuleImport(); + bool parseModuleDecl(); + bool parseImportDecl(); void parseNew(); void parseAccessSpecifier(); bool parseEnum(); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 83e2c5b38ceaf..3c22f4fe8866c 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -14279,18 +14279,16 @@ TEST_F(FormatTest, HandlesIncludeDirectives) { verifyFormat("#define F __has_include_next(<a/b>)"); // Protocol buffer definition or missing "#". - verifyFormat("import \"aaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaa\";", - getLLVMStyleWithColumns(30)); + constexpr StringRef Code("import \"aaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaa\";"); + auto Style = getLLVMStyleWithColumns(30); + verifyFormat(Code, Style); + Style.Language = FormatStyle::LK_Proto; + verifyFormat(Code, Style); - FormatStyle Style = getLLVMStyle(); + Style.Language = FormatStyle::LK_Cpp; Style.AlwaysBreakBeforeMultilineStrings = true; Style.ColumnLimit = 0; verifyFormat("#import \"abc.h\"", Style); - - // But 'import' might also be a regular C++ namespace. - verifyFormat("import::SomeFunction(aaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" - " aaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); - verifyFormat("import::Bar foo(val ? 2 : 1);"); } //===----------------------------------------------------------------------===// @@ -24791,9 +24789,7 @@ TEST_F(FormatTest, Cpp20ModulesSupport) { Style.AllowShortFunctionsOnASingleLine = FormatStyle::ShortFunctionStyle(); verifyFormat("export import foo;", Style); - verifyFormat("export import foo:bar;", Style); verifyFormat("export import foo.bar;", Style); - verifyFormat("export import foo.bar:baz;", Style); verifyFormat("export import :bar;", Style); verifyFormat("export module foo:bar;", Style); verifyFormat("export module foo;", Style); @@ -24821,7 +24817,6 @@ TEST_F(FormatTest, Cpp20ModulesSupport) { verifyFormat("import bar;", Style); verifyFormat("import foo.bar;", Style); - verifyFormat("import foo:bar;", Style); verifyFormat("import :bar;", Style); verifyFormat("import /* module partition */ :bar;", Style); verifyFormat("import <ctime>;", Style); @@ -24837,11 +24832,10 @@ TEST_F(FormatTest, Cpp20ModulesSupport) { "}", Style); - verifyFormat("module :private;", Style); + verifyFormat("module : private;", Style); verifyFormat("import <foo/bar.h>;", Style); verifyFormat("import foo...bar;", Style); verifyFormat("import ..........;", Style); - verifyFormat("module foo:private;", Style); verifyFormat("import a", Style); verifyFormat("module a", Style); verifyFormat("export import a", Style); @@ -24851,8 +24845,25 @@ TEST_F(FormatTest, Cpp20ModulesSupport) { verifyFormat("module", Style); verifyFormat("export", Style); + verifyFormat("import <Foo/Bar> /* comment */;", Style); + verifyFormat("import <Foo/Bar>; // Trailing comment", Style); + + Style.ColumnLimit = 10; + verifyFormat("import Foo.Bar;", Style); + verifyFormat("export import Foo.Bar;", Style); + verifyFormat("export module Foo.Bar;", Style); + verifyFormat("export module Foo.Bar:Baz;", Style); + verifyFormat("import <Foo/Bar> /* comment */;", Style); + verifyFormat("import <Foo/Bar>; // Trailing comment", Style); + + // Somewhat gracefully handle import in pre-C++20 code. verifyFormat("import /* not keyword */ = val ? 2 : 1;"); verifyFormat("_world->import<engine_module>();"); + + // But 'import' might also be a regular C++ namespace. + verifyFormat("import::SomeFunction(aaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); + verifyFormat("import::Bar foo(val ? 2 : 1);"); } TEST_F(FormatTest, CoroutineForCoawait) { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
