Skip to content

Commit

Permalink
[clang-format] [C++20] [Module] clang-format couldn't recognize parti…
Browse files Browse the repository at this point in the history
…tions

https://bugs.llvm.org/show_bug.cgi?id=52517

clang-format is butchering modules, this could easily become a barrier to entry for modules given clang-formats wide spread use.

Prevent the following from adding spaces around the  `:`  (cf was considering the ':' as an InheritanceColon)

Reviewed By: HazardyKnusperkeks, owenpan, ChuanqiXu

Differential Revision: https://reviews.llvm.org/D114151
  • Loading branch information
mydeveloperday committed Nov 25, 2021
1 parent 165545c commit c2fe2b5
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 3 deletions.
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
Expand Up @@ -265,6 +265,8 @@ clang-format
space before parentheses. The custom options can be set using
``SpaceBeforeParensOptions``.

- Improved Cpp20 Modules support.

libclang
--------

Expand Down
1 change: 1 addition & 0 deletions clang/lib/Format/FormatToken.h
Expand Up @@ -76,6 +76,7 @@ namespace format {
TYPE(LineComment) \
TYPE(MacroBlockBegin) \
TYPE(MacroBlockEnd) \
TYPE(ModulePartitionColon) \
TYPE(NamespaceMacro) \
TYPE(NonNullAssertion) \
TYPE(NullCoalescingEqual) \
Expand Down
30 changes: 27 additions & 3 deletions clang/lib/Format/TokenAnnotator.cpp
Expand Up @@ -903,9 +903,13 @@ class AnnotatingParser {
break;
}
}
if (Contexts.back().ColonIsDictLiteral ||
Style.Language == FormatStyle::LK_Proto ||
Style.Language == FormatStyle::LK_TextProto) {
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 (Contexts.back().ColonIsDictLiteral ||
Style.Language == FormatStyle::LK_Proto ||
Style.Language == FormatStyle::LK_TextProto) {
Tok->setType(TT_DictLiteral);
if (Style.Language == FormatStyle::LK_TextProto) {
if (FormatToken *Previous = Tok->getPreviousNonComment())
Expand Down Expand Up @@ -3244,6 +3248,7 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
auto HasExistingWhitespace = [&Right]() {
return Right.WhitespaceRange.getBegin() != Right.WhitespaceRange.getEnd();
};

if (Right.Tok.getIdentifierInfo() && Left.Tok.getIdentifierInfo())
return true; // Never ever merge two identifiers.

Expand All @@ -3253,6 +3258,25 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
return true;

if (Style.isCpp()) {
// Space between import <iostream>.
// or import .....;
if (Left.is(Keywords.kw_import) && Right.isOneOf(tok::less, tok::ellipsis))
return true;
// No space between module :.
if (Left.isOneOf(Keywords.kw_module, Keywords.kw_import) &&
Right.is(TT_ModulePartitionColon))
return true;
// No space between import foo:bar but keep a space between import :bar;
if (Left.is(tok::identifier) && Right.is(TT_ModulePartitionColon))
return false;
// No space between :bar;
if (Left.is(TT_ModulePartitionColon) &&
Right.isOneOf(tok::identifier, tok::kw_private))
return false;
if (Left.is(tok::ellipsis) && Right.is(tok::identifier) &&
Line.First->is(Keywords.kw_import))
return false;

if (Left.is(tok::kw_operator))
return Right.is(tok::coloncolon);
if (Right.is(tok::l_brace) && Right.is(BK_BracedInit) &&
Expand Down
33 changes: 33 additions & 0 deletions clang/lib/Format/UnwrappedLineParser.cpp
Expand Up @@ -1114,6 +1114,35 @@ static bool isC78ParameterDecl(const FormatToken *Tok, const FormatToken *Next,
return Tok->Previous && Tok->Previous->isOneOf(tok::l_paren, tok::comma);
}

void UnwrappedLineParser::parseModuleImport() {
nextToken();
while (!eof()) {
if (FormatTok->is(tok::colon)) {
FormatTok->setType(TT_ModulePartitionColon);
}
// Handle import <foo/bar.h> as we would an include statement.
else if (FormatTok->is(tok::less)) {
nextToken();
while (!FormatTok->isOneOf(tok::semi, tok::greater, tok::eof)) {
// Mark tokens up to the trailing line comments as implicit string
// literals.
if (FormatTok->isNot(tok::comment) &&
!FormatTok->TokenText.startswith("//"))
FormatTok->setType(TT_ImplicitStringLiteral);
nextToken();
}
}
if (FormatTok->is(tok::semi)) {
nextToken();
break;
}
nextToken();
}

addUnwrappedLine();
return;
}

// readTokenWithJavaScriptASI reads the next token and terminates the current
// line if JavaScript Automatic Semicolon Insertion must
// happen between the current token and the next token.
Expand Down Expand Up @@ -1312,6 +1341,10 @@ void UnwrappedLineParser::parseStructuralElement(bool IsTopLevel) {
addUnwrappedLine();
return;
}
if (Style.isCpp()) {
parseModuleImport();
return;
}
}
if (Style.isCpp() &&
FormatTok->isOneOf(Keywords.kw_signals, Keywords.kw_qsignals,
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Format/UnwrappedLineParser.h
Expand Up @@ -110,6 +110,7 @@ class UnwrappedLineParser {
void parseCaseLabel();
void parseSwitch();
void parseNamespace();
void parseModuleImport();
void parseNew();
void parseAccessSpecifier();
bool parseEnum();
Expand Down
65 changes: 65 additions & 0 deletions clang/unittests/Format/FormatTest.cpp
Expand Up @@ -22616,6 +22616,71 @@ TEST_F(FormatTest, FormatDecayCopy) {
verifyFormat("auto(*p)() = f;"); // actually a declaration; TODO FIXME
}

TEST_F(FormatTest, Cpp20ModulesSupport) {
FormatStyle Style = getLLVMStyle();
Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Never;
Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;

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);
verifyFormat("export module foo.bar;", Style);
verifyFormat("export module foo.bar:baz;", Style);
verifyFormat("export import <string_view>;", Style);

verifyFormat("export type_name var;", Style);
verifyFormat("template <class T> export using A = B<T>;", Style);
verifyFormat("export using A = B;", Style);
verifyFormat("export int func() {\n"
" foo();\n"
"}",
Style);
verifyFormat("export struct {\n"
" int foo;\n"
"};",
Style);
verifyFormat("export {\n"
" int foo;\n"
"};",
Style);
verifyFormat("export export char const *hello() { return \"hello\"; }");

verifyFormat("import bar;", Style);
verifyFormat("import foo.bar;", Style);
verifyFormat("import foo:bar;", Style);
verifyFormat("import :bar;", Style);
verifyFormat("import <ctime>;", Style);
verifyFormat("import \"header\";", Style);

verifyFormat("module foo;", Style);
verifyFormat("module foo:bar;", Style);
verifyFormat("module foo.bar;", Style);
verifyFormat("module;", Style);

verifyFormat("export namespace hi {\n"
"const char *sayhi();\n"
"}",
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);
verifyFormat("export module a", Style);

verifyFormat("import", Style);
verifyFormat("module", Style);
verifyFormat("export", Style);
}

} // namespace
} // namespace format
} // namespace clang

0 comments on commit c2fe2b5

Please sign in to comment.