[clang][deps] Properly capture the global module and '\n' for all module directives (#148685)

Previously, the newline after a module directive was not properly
captured and printed by `clang::printDependencyDirectivesAsSource`.

According to P1857R3, each directive must, after skipping horizontal
whitespace, appear at the start of a logical line. Because the newline
after module directives was missing, this invalidated the following
line.
This fixes tests that were previously in violation of P1857R3,
including for Objective-C directives, which should also comply with
P1857R3.

This also ensures that the global module fragment `module;` is captured
by the dependency directives scanner.
This commit is contained in:
Naveen Seth Hanig
2025-07-19 09:47:37 +02:00
committed by GitHub
parent 6b371cab94
commit 6855b9c598
4 changed files with 41 additions and 26 deletions

View File

@@ -560,15 +560,13 @@ bool Scanner::lexModuleDirectiveBody(DirectiveKind Kind, const char *&First,
if (Tok.is(tok::semi))
break;
}
const auto &Tok = lexToken(First, End);
pushDirective(Kind);
skipWhitespace(First, End);
if (First == End)
if (Tok.is(tok::eof) || Tok.is(tok::eod))
return false;
if (!isVerticalWhitespace(*First))
return reportError(
DirectiveLoc, diag::err_dep_source_scanner_unexpected_tokens_at_import);
skipNewline(First, End);
return false;
return reportError(DirectiveLoc,
diag::err_dep_source_scanner_unexpected_tokens_at_import);
}
dependency_directives_scan::Token &Scanner::lexToken(const char *&First,
@@ -735,6 +733,13 @@ bool Scanner::lexModule(const char *&First, const char *const End) {
return false;
break;
}
case ';': {
// Handle the global module fragment `module;`.
if (Id == "module" && !Export)
break;
skipLine(First, End);
return false;
}
case '<':
case '"':
break;
@@ -905,14 +910,6 @@ bool Scanner::lexPPLine(const char *&First, const char *const End) {
CurDirToks.clear();
});
// Handle "@import".
if (*First == '@')
return lexAt(First, End);
// Handle module directives for C++20 modules.
if (*First == 'i' || *First == 'e' || *First == 'm')
return lexModule(First, End);
if (*First == '_') {
if (isNextIdentifierOrSkipLine("_Pragma", First, End))
return lex_Pragma(First, End);
@@ -925,6 +922,14 @@ bool Scanner::lexPPLine(const char *&First, const char *const End) {
auto ScEx2 = make_scope_exit(
[&]() { TheLexer.setParsingPreprocessorDirective(false); });
// Handle "@import".
if (*First == '@')
return lexAt(First, End);
// Handle module directives for C++20 modules.
if (*First == 'i' || *First == 'e' || *First == 'm')
return lexModule(First, End);
// Lex '#'.
const dependency_directives_scan::Token &HashTok = lexToken(First, End);
if (HashTok.is(tok::hashhash)) {

View File

@@ -950,6 +950,8 @@ void Preprocessor::Lex(Token &Result) {
case tok::period:
ModuleDeclState.handlePeriod();
break;
case tok::eod:
break;
case tok::identifier:
// Check "import" and "module" when there is no open bracket. The two
// identifiers are not meaningful with open brackets.

View File

@@ -2519,6 +2519,7 @@ Decl *Parser::ParseModuleImport(SourceLocation AtLoc,
break;
}
ExpectAndConsumeSemi(diag::err_module_expected_semi);
TryConsumeToken(tok::eod);
if (SeenError)
return nullptr;

View File

@@ -640,14 +640,14 @@ TEST(MinimizeSourceToDependencyDirectivesTest, AtImport) {
EXPECT_STREQ("@import A;\n", Out.data());
ASSERT_FALSE(minimizeSourceToDependencyDirectives("@import A\n;", Out));
EXPECT_STREQ("@import A;\n", Out.data());
EXPECT_STREQ("@import A\n;\n", Out.data());
ASSERT_FALSE(minimizeSourceToDependencyDirectives("@import A.B;\n", Out));
EXPECT_STREQ("@import A.B;\n", Out.data());
ASSERT_FALSE(minimizeSourceToDependencyDirectives(
"@import /*x*/ A /*x*/ . /*x*/ B /*x*/ \n /*x*/ ; /*x*/", Out));
EXPECT_STREQ("@import A.B;\n", Out.data());
"@import /*x*/ A /*x*/ . /*x*/ B /*x*/ \\n /*x*/ ; /*x*/", Out));
EXPECT_STREQ("@import A.B\\n;\n", Out.data());
}
TEST(MinimizeSourceToDependencyDirectivesTest, EmptyIncludesAndImports) {
@@ -1122,16 +1122,23 @@ ort \
)";
ASSERT_FALSE(
minimizeSourceToDependencyDirectives(Source, Out, Tokens, Directives));
EXPECT_STREQ("#include \"textual-header.h\"\nexport module m;"
"exp\\\nort import:l[[rename]];"
"import<<=3;import a b d e d e f e;"
"import foo[[no_unique_address]];import foo();"
"import f(:sefse);import f(->a=3);"
EXPECT_STREQ("module;\n"
"#include \"textual-header.h\"\n"
"export module m;\n"
"exp\\\nort import:l[[rename]];\n"
"import<<=3;\n"
"import a b d e d e f e;\n"
"import foo[[no_unique_address]];\n"
"import foo();\n"
"import f(:sefse);\n"
"import f(->a=3);\n"
"<TokBeforeEOF>\n",
Out.data());
ASSERT_EQ(Directives.size(), 11u);
EXPECT_EQ(Directives[0].Kind, pp_include);
EXPECT_EQ(Directives[1].Kind, cxx_export_module_decl);
ASSERT_EQ(Directives.size(), 12u);
EXPECT_EQ(Directives[0].Kind, cxx_module_decl);
EXPECT_EQ(Directives[1].Kind, pp_include);
EXPECT_EQ(Directives[2].Kind, cxx_export_module_decl);
}
TEST(MinimizeSourceToDependencyDirectivesTest, ObjCMethodArgs) {