[clang-scan-deps] Implement P2223R2 for DependencyDirectiveScanner.cpp #143950
Conversation
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
P2223R2 allows the line-continuation slash `\` to be followed by additional whitespace. The Clang lexer already follows this behavior, also for versions prior to C++23. The dependency directive scanner however only implements it for `#define` directives (15d5f5d). This fully implements P2223R2 for the dependency directive scanner (for any C++ standard) and aligns the dependency directive scanner's splicing behavior with that of the Clang lexer.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Naveen Seth Hanig (naveen-seth) ChangesP2223R2 allows the line-continuation slash This fully implements P2223R2 for the dependency directive scanner (for any C++ standard) and aligns the dependency directive scanner's splicing behavior with that of the Clang lexer. For example, the following code was previously not scanned correctly by import \<whitespace here>
A;Full diff: https://github.com/llvm/llvm-project/pull/143950.diff 2 Files Affected:
diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp b/clang/lib/Lex/DependencyDirectivesScanner.cpp
index 4606b85d42fe7..1b6b16c561141 100644
--- a/clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -323,10 +323,6 @@ static unsigned skipNewline(const char *&First, const char *End) {
return Len;
}
-static bool wasLineContinuation(const char *First, unsigned EOLLen) {
- return *(First - (int)EOLLen - 1) == '\\';
-}
-
static void skipToNewlineRaw(const char *&First, const char *const End) {
for (;;) {
if (First == End)
@@ -336,13 +332,16 @@ static void skipToNewlineRaw(const char *&First, const char *const End) {
if (Len)
return;
+ char LastNonWhitespace = ' ';
do {
+ if (!isHorizontalWhitespace(*First))
+ LastNonWhitespace = *First;
if (++First == End)
return;
Len = isEOL(First, End);
} while (!Len);
- if (First[-1] != '\\')
+ if (LastNonWhitespace != '\\')
return;
First += Len;
@@ -394,6 +393,7 @@ static bool isQuoteCppDigitSeparator(const char *const Start,
}
void Scanner::skipLine(const char *&First, const char *const End) {
+ char LastNonWhitespace = ' ';
for (;;) {
assert(First <= End);
if (First == End)
@@ -419,6 +419,8 @@ void Scanner::skipLine(const char *&First, const char *const End) {
// Iterate over comments correctly.
if (*First != '/' || End - First < 2) {
LastTokenPtr = First;
+ if (!isWhitespace(*First))
+ LastNonWhitespace = *First;
++First;
continue;
}
@@ -431,6 +433,8 @@ void Scanner::skipLine(const char *&First, const char *const End) {
if (First[1] != '*') {
LastTokenPtr = First;
+ if (!isWhitespace(*First))
+ LastNonWhitespace = *First;
++First;
continue;
}
@@ -442,8 +446,9 @@ void Scanner::skipLine(const char *&First, const char *const End) {
return;
// Skip over the newline.
- unsigned Len = skipNewline(First, End);
- if (!wasLineContinuation(First, Len)) // Continue past line-continuations.
+ skipNewline(First, End);
+
+ if (LastNonWhitespace != '\\')
break;
}
}
@@ -468,9 +473,16 @@ static void skipWhitespace(const char *&First, const char *const End) {
if (End - First < 2)
return;
- if (First[0] == '\\' && isVerticalWhitespace(First[1])) {
- skipNewline(++First, End);
- continue;
+ if (*First == '\\') {
+ const char *Ptr = First + 1;
+ while (Ptr < End && isHorizontalWhitespace(*Ptr))
+ ++Ptr;
+ if (Ptr != End && isVerticalWhitespace(*Ptr)) {
+ skipNewline(Ptr, End);
+ First = Ptr;
+ continue;
+ }
+ return;
}
// Check for a non-comment character.
diff --git a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
index 377c066f031d3..61f74929c1e98 100644
--- a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -789,6 +789,97 @@ TEST(MinimizeSourceToDependencyDirectivesTest,
Out.data());
}
+TEST(MinimizeSourceToDependencyDirectivesTest,
+ WhitespaceAfterLineContinuationSlashLineComment) {
+ SmallVector<char, 128> Out;
+
+ ASSERT_FALSE(minimizeSourceToDependencyDirectives("// some comment \\ \n"
+ "module A;\n",
+ Out));
+ EXPECT_STREQ("", Out.data());
+}
+
+TEST(MinimizeSourceToDependencyDirectivesTest,
+ WhitespaceAfterLineContinuationSlashAllDirectives) {
+ SmallVector<char, 512> Out;
+ SmallVector<dependency_directives_scan::Token, 16> Tokens;
+ SmallVector<Directive, 16> Directives;
+
+ StringRef Input = "#define \\ \n"
+ "A\n"
+ "#undef\t\\ \n"
+ "A\n"
+ "#endif \\\t\t\n"
+ "\n"
+ "#if \\ \t\n"
+ "A\n"
+ "#ifdef\t\\ \n"
+ "A\n"
+ "#ifndef \\ \t\n"
+ "A\n"
+ "#elifdef \\ \n"
+ "A\n"
+ "#elifndef \\ \n"
+ "A\n"
+ "#elif \\\t\t \n"
+ "A\n"
+ "#else \\\t \t\n"
+ "\n"
+ "#include \\ \n"
+ "<A>\n"
+ "#include_next \\ \n"
+ "<A>\n"
+ "#__include_macros\\ \n"
+ "<A>\n"
+ "#import \\ \t\n"
+ "<A>\n"
+ "@import \\\t \n"
+ "A;\n"
+ "#pragma clang \\ \n"
+ "module \\ \n"
+ "import A\n"
+ "#pragma \\ \n"
+ "push_macro(A)\n"
+ "#pragma \\\t \n"
+ "pop_macro(A)\n"
+ "#pragma \\ \n"
+ "include_alias(<A>,\\ \n"
+ "<B>)\n"
+ "export \\ \n"
+ "module m;\n"
+ "import\t\\\t \n"
+ "m;\n"
+ "#pragma\t\\ \n"
+ "clang\t\\ \t\n"
+ "system_header\n";
+ ASSERT_FALSE(
+ minimizeSourceToDependencyDirectives(Input, Out, Tokens, Directives));
+
+ EXPECT_EQ(pp_define, Directives[0].Kind);
+ EXPECT_EQ(pp_undef, Directives[1].Kind);
+ EXPECT_EQ(pp_endif, Directives[2].Kind);
+ EXPECT_EQ(pp_if, Directives[3].Kind);
+ EXPECT_EQ(pp_ifdef, Directives[4].Kind);
+ EXPECT_EQ(pp_ifndef, Directives[5].Kind);
+ EXPECT_EQ(pp_elifdef, Directives[6].Kind);
+ EXPECT_EQ(pp_elifndef, Directives[7].Kind);
+ EXPECT_EQ(pp_elif, Directives[8].Kind);
+ EXPECT_EQ(pp_else, Directives[9].Kind);
+ EXPECT_EQ(pp_include, Directives[10].Kind);
+ EXPECT_EQ(pp_include_next, Directives[11].Kind);
+ EXPECT_EQ(pp___include_macros, Directives[12].Kind);
+ EXPECT_EQ(pp_import, Directives[13].Kind);
+ EXPECT_EQ(decl_at_import, Directives[14].Kind);
+ EXPECT_EQ(pp_pragma_import, Directives[15].Kind);
+ EXPECT_EQ(pp_pragma_push_macro, Directives[16].Kind);
+ EXPECT_EQ(pp_pragma_pop_macro, Directives[17].Kind);
+ EXPECT_EQ(pp_pragma_include_alias, Directives[18].Kind);
+ EXPECT_EQ(cxx_export_module_decl, Directives[19].Kind);
+ EXPECT_EQ(cxx_import_decl, Directives[20].Kind);
+ EXPECT_EQ(pp_pragma_system_header, Directives[21].Kind);
+ EXPECT_EQ(pp_eof, Directives[22].Kind);
+}
+
TEST(MinimizeSourceToDependencyDirectivesTest, PoundWarningAndError) {
SmallVector<char, 128> Out;
|
|
Hi @hyp, @Bigcheese, would you be available to review this? |
Bigcheese
approved these changes
Jun 13, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/30747 Here is the relevant piece of the build log for the reference |
tomtor
pushed a commit
to tomtor/llvm-project
that referenced
this pull request
Jun 14, 2025
llvm#143950) P2223R2 allows the line-continuation slash `\` to be followed by additional whitespace. The Clang lexer already follows this behavior, also for versions prior to C++23. The dependency directive scanner however only implements it for `#define` directives (15d5f5d). This fully implements P2223R2 for the dependency directive scanner (for any C++ standard) and aligns the dependency directive scanner's splicing behavior with that of the Clang lexer. For example, the following code was previously not scanned correctly by `clang-scan-deps` but now works as expected: ```cpp import \<whitespace here> A; ```
akuhlens
pushed a commit
to akuhlens/llvm-project
that referenced
this pull request
Jun 24, 2025
llvm#143950) P2223R2 allows the line-continuation slash `\` to be followed by additional whitespace. The Clang lexer already follows this behavior, also for versions prior to C++23. The dependency directive scanner however only implements it for `#define` directives (15d5f5d). This fully implements P2223R2 for the dependency directive scanner (for any C++ standard) and aligns the dependency directive scanner's splicing behavior with that of the Clang lexer. For example, the following code was previously not scanned correctly by `clang-scan-deps` but now works as expected: ```cpp import \<whitespace here> A; ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
P2223R2 allows the line-continuation slash
\to be followed by additional whitespace. The Clang lexer already follows this behavior, also for versions prior to C++23. The dependency directive scanner however only implements it for#definedirectives (15d5f5d).This fully implements P2223R2 for the dependency directive scanner (for any C++ standard) and aligns the dependency directive scanner's splicing behavior with that of the Clang lexer.
For example, the following code was previously not scanned correctly by
clang-scan-depsbut now works as expected:import \<whitespace here> A;