Skip to content

Commit 6855b9c

Browse files
authored
[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.
1 parent 6b371ca commit 6855b9c

File tree

4 files changed

+41
-26
lines changed

4 files changed

+41
-26
lines changed

clang/lib/Lex/DependencyDirectivesScanner.cpp

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -560,15 +560,13 @@ bool Scanner::lexModuleDirectiveBody(DirectiveKind Kind, const char *&First,
560560
if (Tok.is(tok::semi))
561561
break;
562562
}
563+
564+
const auto &Tok = lexToken(First, End);
563565
pushDirective(Kind);
564-
skipWhitespace(First, End);
565-
if (First == End)
566+
if (Tok.is(tok::eof) || Tok.is(tok::eod))
566567
return false;
567-
if (!isVerticalWhitespace(*First))
568-
return reportError(
569-
DirectiveLoc, diag::err_dep_source_scanner_unexpected_tokens_at_import);
570-
skipNewline(First, End);
571-
return false;
568+
return reportError(DirectiveLoc,
569+
diag::err_dep_source_scanner_unexpected_tokens_at_import);
572570
}
573571

574572
dependency_directives_scan::Token &Scanner::lexToken(const char *&First,
@@ -735,6 +733,13 @@ bool Scanner::lexModule(const char *&First, const char *const End) {
735733
return false;
736734
break;
737735
}
736+
case ';': {
737+
// Handle the global module fragment `module;`.
738+
if (Id == "module" && !Export)
739+
break;
740+
skipLine(First, End);
741+
return false;
742+
}
738743
case '<':
739744
case '"':
740745
break;
@@ -905,14 +910,6 @@ bool Scanner::lexPPLine(const char *&First, const char *const End) {
905910
CurDirToks.clear();
906911
});
907912

908-
// Handle "@import".
909-
if (*First == '@')
910-
return lexAt(First, End);
911-
912-
// Handle module directives for C++20 modules.
913-
if (*First == 'i' || *First == 'e' || *First == 'm')
914-
return lexModule(First, End);
915-
916913
if (*First == '_') {
917914
if (isNextIdentifierOrSkipLine("_Pragma", First, End))
918915
return lex_Pragma(First, End);
@@ -925,6 +922,14 @@ bool Scanner::lexPPLine(const char *&First, const char *const End) {
925922
auto ScEx2 = make_scope_exit(
926923
[&]() { TheLexer.setParsingPreprocessorDirective(false); });
927924

925+
// Handle "@import".
926+
if (*First == '@')
927+
return lexAt(First, End);
928+
929+
// Handle module directives for C++20 modules.
930+
if (*First == 'i' || *First == 'e' || *First == 'm')
931+
return lexModule(First, End);
932+
928933
// Lex '#'.
929934
const dependency_directives_scan::Token &HashTok = lexToken(First, End);
930935
if (HashTok.is(tok::hashhash)) {

clang/lib/Lex/Preprocessor.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -950,6 +950,8 @@ void Preprocessor::Lex(Token &Result) {
950950
case tok::period:
951951
ModuleDeclState.handlePeriod();
952952
break;
953+
case tok::eod:
954+
break;
953955
case tok::identifier:
954956
// Check "import" and "module" when there is no open bracket. The two
955957
// identifiers are not meaningful with open brackets.

clang/lib/Parse/Parser.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2519,6 +2519,7 @@ Decl *Parser::ParseModuleImport(SourceLocation AtLoc,
25192519
break;
25202520
}
25212521
ExpectAndConsumeSemi(diag::err_module_expected_semi);
2522+
TryConsumeToken(tok::eod);
25222523

25232524
if (SeenError)
25242525
return nullptr;

clang/unittests/Lex/DependencyDirectivesScannerTest.cpp

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -640,14 +640,14 @@ TEST(MinimizeSourceToDependencyDirectivesTest, AtImport) {
640640
EXPECT_STREQ("@import A;\n", Out.data());
641641

642642
ASSERT_FALSE(minimizeSourceToDependencyDirectives("@import A\n;", Out));
643-
EXPECT_STREQ("@import A;\n", Out.data());
643+
EXPECT_STREQ("@import A\n;\n", Out.data());
644644

645645
ASSERT_FALSE(minimizeSourceToDependencyDirectives("@import A.B;\n", Out));
646646
EXPECT_STREQ("@import A.B;\n", Out.data());
647647

648648
ASSERT_FALSE(minimizeSourceToDependencyDirectives(
649-
"@import /*x*/ A /*x*/ . /*x*/ B /*x*/ \n /*x*/ ; /*x*/", Out));
650-
EXPECT_STREQ("@import A.B;\n", Out.data());
649+
"@import /*x*/ A /*x*/ . /*x*/ B /*x*/ \\n /*x*/ ; /*x*/", Out));
650+
EXPECT_STREQ("@import A.B\\n;\n", Out.data());
651651
}
652652

653653
TEST(MinimizeSourceToDependencyDirectivesTest, EmptyIncludesAndImports) {
@@ -1122,16 +1122,23 @@ ort \
11221122
)";
11231123
ASSERT_FALSE(
11241124
minimizeSourceToDependencyDirectives(Source, Out, Tokens, Directives));
1125-
EXPECT_STREQ("#include \"textual-header.h\"\nexport module m;"
1126-
"exp\\\nort import:l[[rename]];"
1127-
"import<<=3;import a b d e d e f e;"
1128-
"import foo[[no_unique_address]];import foo();"
1129-
"import f(:sefse);import f(->a=3);"
1125+
1126+
EXPECT_STREQ("module;\n"
1127+
"#include \"textual-header.h\"\n"
1128+
"export module m;\n"
1129+
"exp\\\nort import:l[[rename]];\n"
1130+
"import<<=3;\n"
1131+
"import a b d e d e f e;\n"
1132+
"import foo[[no_unique_address]];\n"
1133+
"import foo();\n"
1134+
"import f(:sefse);\n"
1135+
"import f(->a=3);\n"
11301136
"<TokBeforeEOF>\n",
11311137
Out.data());
1132-
ASSERT_EQ(Directives.size(), 11u);
1133-
EXPECT_EQ(Directives[0].Kind, pp_include);
1134-
EXPECT_EQ(Directives[1].Kind, cxx_export_module_decl);
1138+
ASSERT_EQ(Directives.size(), 12u);
1139+
EXPECT_EQ(Directives[0].Kind, cxx_module_decl);
1140+
EXPECT_EQ(Directives[1].Kind, pp_include);
1141+
EXPECT_EQ(Directives[2].Kind, cxx_export_module_decl);
11351142
}
11361143

11371144
TEST(MinimizeSourceToDependencyDirectivesTest, ObjCMethodArgs) {

0 commit comments

Comments
 (0)