[clang-format] Keep C++20 module/import decls on a single line#199459
Open
owenca wants to merge 1 commit into
Open
[clang-format] Keep C++20 module/import decls on a single line#199459owenca wants to merge 1 commit into
owenca wants to merge 1 commit into
Conversation
|
@llvm/pr-subscribers-clang-format Author: owenca (owenca) ChangesFixes #193676 Full diff: https://github.com/llvm/llvm-project/pull/199459.diff 5 Files Affected:
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 9536a233def58..5a73e7d9d015d 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,66 @@ 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 (;; 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);
+ if (FormatTok->is(tok::less)) {
+ for (nextToken(); FormatTok->isNoneOf(tok::semi, tok::eof); nextToken()) {
+ if (FormatTok->is(tok::greater)) {
+ nextToken();
+ break;
}
- 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)) {
+ if (FormatTok->isOneOf(tok::semi, tok::eof)) {
nextToken();
break;
}
- nextToken();
}
+ Line->IsModuleOrImportDecl = true;
addUnwrappedLine();
return true;
}
@@ -1624,14 +1651,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)) {
@@ -1642,8 +1661,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:
@@ -1664,6 +1694,8 @@ void UnwrappedLineParser::parseStructuralElement(
return;
}
if (FormatTok->is(Keywords.kw_import)) {
+ if (IsCpp && parseImportDecl())
+ return;
if (Style.isJavaScript()) {
parseJavaScriptEs6ImportExport();
return;
@@ -1684,25 +1716,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 8181eec1495f7..bc049f5669f6a 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) {
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This patch fixes #193676.
UnwrappedLineParser::parseModuleDecl()to parse C++20 module declarations.parseCppModuleImport()from [clang-format] Don't break module names #193834 and renamed it toparseImportDecl().FormatTest.cpp.Co-authored-by: Björn Schäpers github@hazardy.de