https://github.com/ziqingluo-90 updated
https://github.com/llvm/llvm-project/pull/146645
>From b9c3c4e0eca567937e59b2ae21f805d8156f7204 Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziq...@udel.edu>
Date: Wed, 2 Jul 2025 15:46:37 +0800
Subject: [PATCH 1/3] [clang-scan-deps] Fix "unterminated conditional
directive" bug
`clang-scan-deps` threw "unterminated conditional directive" error
falsely on the following example:
```
#if defined(__TEST_DUMMY2)
#pragma GCC warning \
"Hello!"
#else
#pragma GCC error \
"World!"
#endif // defined(__TEST_DUMMY2)
```
The issue comes from PR #143950, where the flag `LastNonWhitespace`
does not correctly represent the state in the example above. The PR
aimed to support that a line-continuation can be followed by
whitespaces. This fix uses a more straightforward but less efficient
way to do the same thing---it looks back for a line-continuation and
skips any number of whitespaces before that.
rdar://153742186
---
clang/lib/Lex/DependencyDirectivesScanner.cpp | 29 ++++++++++++-----
.../Lex/DependencyDirectivesScannerTest.cpp | 31 +++++++++++++++++++
2 files changed, 52 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp
b/clang/lib/Lex/DependencyDirectivesScanner.cpp
index 1b6b16c561141..b1f1285bd959d 100644
--- a/clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -393,7 +393,7 @@ static bool isQuoteCppDigitSeparator(const char *const
Start,
}
void Scanner::skipLine(const char *&First, const char *const End) {
- char LastNonWhitespace = ' ';
+ const char * const OldFirst = First;
for (;;) {
assert(First <= End);
if (First == End)
@@ -419,8 +419,6 @@ 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;
}
@@ -433,8 +431,6 @@ void Scanner::skipLine(const char *&First, const char
*const End) {
if (First[1] != '*') {
LastTokenPtr = First;
- if (!isWhitespace(*First))
- LastNonWhitespace = *First;
++First;
continue;
}
@@ -446,9 +442,26 @@ void Scanner::skipLine(const char *&First, const char
*const End) {
return;
// Skip over the newline.
- skipNewline(First, End);
-
- if (LastNonWhitespace != '\\')
+ auto Len = skipNewline(First, End);
+ // Since P2223R2 allows the line-continuation slash \ to be followed by
+ // additional whitespace, we need to check that here if `First`
+ // follows a `\\` and whitespaces.
+
+ // `LookBack` points to the character before the newline:
+ const char *LookBack = First - Len;
+ bool LineContinuationFound = false;
+
+ // Move `LookBack` backwards to find line-continuation and whitespaces:
+ while (LookBack >= OldFirst) { // bound `LookBack` by `OldFirst`:
+ if (isWhitespace(*LookBack)) {
+ --LookBack; // whitespace before '\\' is ok
+ continue;
+ }
+ if (*LookBack == '\\')
+ LineContinuationFound = true;
+ break; // not a whitespace, end loop
+ }
+ if (!LineContinuationFound)
break;
}
}
diff --git a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
index 61f74929c1e98..4f64ed80cb7bf 100644
--- a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -880,6 +880,37 @@ TEST(MinimizeSourceToDependencyDirectivesTest,
EXPECT_EQ(pp_eof, Directives[22].Kind);
}
+TEST(MinimizeSourceToDependencyDirectivesTest,
+ TestFixedBugThatReportUnterminatedDirectiveFalsely) {
+ SmallVector<char, 512> Out;
+ SmallVector<dependency_directives_scan::Token, 16> Tokens;
+ SmallVector<Directive, 16> Directives;
+
+ StringRef Input = "#ifndef __TEST \n"
+ "#define __TEST \n"
+ "#if defined(__TEST_DUMMY) \n"
+ "#if defined(__TEST_DUMMY2) \n"
+ "#pragma GCC warning \\ \n"
+ "\"hello!\"\n"
+ "#else\n"
+ "#pragma GCC error \\ \n"
+ "\"world!\" \n"
+ "#endif // defined(__TEST_DUMMY2) \n"
+ "#endif // defined(__TEST_DUMMY) \n"
+ "#endif // #ifndef __TEST \n";
+ ASSERT_FALSE( // False on no error:
+ minimizeSourceToDependencyDirectives(Input, Out, Tokens, Directives));
+ ASSERT_TRUE(Directives.size() == 8);
+ EXPECT_EQ(pp_ifndef, Directives[0].Kind);
+ EXPECT_EQ(pp_define, Directives[1].Kind);
+ EXPECT_EQ(pp_if, Directives[2].Kind);
+ EXPECT_EQ(pp_if, Directives[3].Kind);
+ EXPECT_EQ(pp_endif, Directives[4].Kind);
+ EXPECT_EQ(pp_endif, Directives[5].Kind);
+ EXPECT_EQ(pp_endif, Directives[6].Kind);
+ EXPECT_EQ(pp_eof, Directives[7].Kind);
+}
+
TEST(MinimizeSourceToDependencyDirectivesTest, PoundWarningAndError) {
SmallVector<char, 128> Out;
>From 7b1490d38265358658fd06393c1ec43493c7e86d Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziq...@udel.edu>
Date: Wed, 2 Jul 2025 17:26:37 +0800
Subject: [PATCH 2/3] fix format issue
---
clang/lib/Lex/DependencyDirectivesScanner.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp
b/clang/lib/Lex/DependencyDirectivesScanner.cpp
index b1f1285bd959d..de0d14825f0c9 100644
--- a/clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -393,7 +393,7 @@ static bool isQuoteCppDigitSeparator(const char *const
Start,
}
void Scanner::skipLine(const char *&First, const char *const End) {
- const char * const OldFirst = First;
+ const char *const OldFirst = First;
for (;;) {
assert(First <= End);
if (First == End)
>From b6f626b0640171b6ed72bcec30df0c1c17ba504a Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziq...@udel.edu>
Date: Thu, 3 Jul 2025 14:59:42 +0800
Subject: [PATCH 3/3] Take naveen-seth's suggestion.
---
clang/lib/Lex/DependencyDirectivesScanner.cpp | 31 ++++++-------------
.../Lex/DependencyDirectivesScannerTest.cpp | 4 +--
2 files changed, 12 insertions(+), 23 deletions(-)
diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp
b/clang/lib/Lex/DependencyDirectivesScanner.cpp
index de0d14825f0c9..c3325a943b4fc 100644
--- a/clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -393,7 +393,6 @@ static bool isQuoteCppDigitSeparator(const char *const
Start,
}
void Scanner::skipLine(const char *&First, const char *const End) {
- const char *const OldFirst = First;
for (;;) {
assert(First <= End);
if (First == End)
@@ -404,6 +403,9 @@ void Scanner::skipLine(const char *&First, const char
*const End) {
return;
}
const char *Start = First;
+ // Use `LastNonWhitespace`to track if a line-continuation has ever been
seen
+ // before a new-line character:
+ char LastNonWhitespace = ' ';
while (First != End && !isVerticalWhitespace(*First)) {
// Iterate over strings correctly to avoid comments and newlines.
if (*First == '"' ||
@@ -419,6 +421,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 +435,8 @@ void Scanner::skipLine(const char *&First, const char
*const End) {
if (First[1] != '*') {
LastTokenPtr = First;
+ if (!isWhitespace(*First))
+ LastNonWhitespace = *First;
++First;
continue;
}
@@ -442,26 +448,9 @@ void Scanner::skipLine(const char *&First, const char
*const End) {
return;
// Skip over the newline.
- auto Len = skipNewline(First, End);
- // Since P2223R2 allows the line-continuation slash \ to be followed by
- // additional whitespace, we need to check that here if `First`
- // follows a `\\` and whitespaces.
-
- // `LookBack` points to the character before the newline:
- const char *LookBack = First - Len;
- bool LineContinuationFound = false;
-
- // Move `LookBack` backwards to find line-continuation and whitespaces:
- while (LookBack >= OldFirst) { // bound `LookBack` by `OldFirst`:
- if (isWhitespace(*LookBack)) {
- --LookBack; // whitespace before '\\' is ok
- continue;
- }
- if (*LookBack == '\\')
- LineContinuationFound = true;
- break; // not a whitespace, end loop
- }
- if (!LineContinuationFound)
+ skipNewline(First, End);
+
+ if (LastNonWhitespace != '\\')
break;
}
}
diff --git a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
index 4f64ed80cb7bf..d2ef27155df94 100644
--- a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -890,10 +890,10 @@ TEST(MinimizeSourceToDependencyDirectivesTest,
"#define __TEST \n"
"#if defined(__TEST_DUMMY) \n"
"#if defined(__TEST_DUMMY2) \n"
- "#pragma GCC warning \\ \n"
+ "#pragma GCC warning \\ \n"
"\"hello!\"\n"
"#else\n"
- "#pragma GCC error \\ \n"
+ "#pragma GCC error \\ \n"
"\"world!\" \n"
"#endif // defined(__TEST_DUMMY2) \n"
"#endif // defined(__TEST_DUMMY) \n"
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
