[clang] [clang][deps] Recognize 'module;' in dependency directive scanner (PR #148685)
Naveen Seth Hanig via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 16 13:18:51 PDT 2025
https://github.com/naveen-seth updated https://github.com/llvm/llvm-project/pull/148685
>From 7a759271c54b78eb940bf12274bcc535a3df22e3 Mon Sep 17 00:00:00 2001
From: Naveen Seth Hanig <naveen.hanig at outlook.com>
Date: Mon, 14 Jul 2025 19:17:10 +0200
Subject: [PATCH 1/3] [clang][deps] Recognize 'module;' in dependency directive
scanner
With this change, the dependency directive scanner now properly identifies
"module;" as a directive, as per P1857R3.
Previously, the global module fragment was not recognized by the
scanner.
---
clang/lib/Lex/DependencyDirectivesScanner.cpp | 7 +++++++
clang/unittests/Lex/DependencyDirectivesScannerTest.cpp | 9 +++++----
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp b/clang/lib/Lex/DependencyDirectivesScanner.cpp
index d894c265a07a2..8822e760274d0 100644
--- a/clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -728,6 +728,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;
diff --git a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
index d2ef27155df94..92f6f401ec6b7 100644
--- a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -1122,16 +1122,17 @@ ort \
)";
ASSERT_FALSE(
minimizeSourceToDependencyDirectives(Source, Out, Tokens, Directives));
- EXPECT_STREQ("#include \"textual-header.h\"\nexport module m;"
+ EXPECT_STREQ("module;#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);"
"<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) {
>From 25553f716701cb627e221deddace2d2956efc593 Mon Sep 17 00:00:00 2001
From: Naveen Seth Hanig <naveen.hanig at outlook.com>
Date: Mon, 14 Jul 2025 19:53:07 +0200
Subject: [PATCH 2/3] Add newline after 'module;' to be valid C++
---
clang/unittests/Lex/DependencyDirectivesScannerTest.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
index 92f6f401ec6b7..a28989e6cf174 100644
--- a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -1122,7 +1122,7 @@ ort \
)";
ASSERT_FALSE(
minimizeSourceToDependencyDirectives(Source, Out, Tokens, Directives));
- EXPECT_STREQ("module;#include \"textual-header.h\"\nexport module m;"
+ EXPECT_STREQ("module\n;#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();"
>From 586b4389f28ab98bcb1229144004b74e73cccf4b Mon Sep 17 00:00:00 2001
From: Naveen Seth Hanig <naveen.hanig at outlook.com>
Date: Wed, 16 Jul 2025 21:56:51 +0200
Subject: [PATCH 3/3] [clang][deps] Properly capture the global module and '\n'
for all module directives
Previously, the newline after a module directive was not properly
captured (or printed).
According to P1857R3, each directive must, after skipping horizontal
whitespace, be at the start of a logical line.
Because the newline after module directives was missing, this
invalidated the following line.
This also fixes or removes tests that were in violation of P1857R3.
This extends to Objective-C directives, which should also
follow P1857R3.
This also ensures that the global module fragment `module;` is
captured by the dependency directives scanner.
---
clang/lib/Lex/DependencyDirectivesScanner.cpp | 28 +++++++++----------
.../Lex/DependencyDirectivesScannerTest.cpp | 23 ++++++++-------
2 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp b/clang/lib/Lex/DependencyDirectivesScanner.cpp
index 8822e760274d0..a1876f868474a 100644
--- a/clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -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,
@@ -905,14 +903,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 +915,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)) {
diff --git a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
index a28989e6cf174..1f1dea58a6cc5 100644
--- a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -639,15 +639,12 @@ TEST(MinimizeSourceToDependencyDirectivesTest, AtImport) {
ASSERT_FALSE(minimizeSourceToDependencyDirectives(" @ import A;\n", Out));
EXPECT_STREQ("@import A;\n", Out.data());
- ASSERT_FALSE(minimizeSourceToDependencyDirectives("@import A\n;", Out));
- EXPECT_STREQ("@import A;\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,11 +1119,17 @@ ort \
)";
ASSERT_FALSE(
minimizeSourceToDependencyDirectives(Source, Out, Tokens, Directives));
- EXPECT_STREQ("module\n;#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(), 12u);
More information about the cfe-commits
mailing list