https://github.com/ziqingluo-90 created
https://github.com/llvm/llvm-project/pull/146645
`clang-scan-deps` threw "unterminated conditional directive" error falsely on
the following example:
```
#ifndef __TEST
#define __TEST
#if defined(__TEST_DUMMY)
#if defined(__TEST_DUMMY2)
#pragma GCC warning \
"Hello!"
#else
#pragma GCC error \
"World!"
#endif // defined(__TEST_DUMMY2)
#endif // defined(__TEST_DUMMY)
#endif // #ifndef __TEST
```
The issue comes from PR #143950, where the flag `LastNonWhitespace` does not
correctly represent the state for 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
>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] [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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
