Skip to content

Commit 161d7b6

Browse files
committed
[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
1 parent 0bfa0bc commit 161d7b6

File tree

2 files changed

+52
-8
lines changed

2 files changed

+52
-8
lines changed

clang/lib/Lex/DependencyDirectivesScanner.cpp

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ static bool isQuoteCppDigitSeparator(const char *const Start,
419419
}
420420

421421
void Scanner::skipLine(const char *&First, const char *const End) {
422-
char LastNonWhitespace = ' ';
422+
const char * const OldFirst = First;
423423
for (;;) {
424424
assert(First <= End);
425425
if (First == End)
@@ -453,8 +453,6 @@ void Scanner::skipLine(const char *&First, const char *const End) {
453453
// Iterate over comments correctly.
454454
if (*First != '/' || End - First < 2) {
455455
LastTokenPtr = First;
456-
if (!isWhitespace(*First))
457-
LastNonWhitespace = *First;
458456
++First;
459457
continue;
460458
}
@@ -467,8 +465,6 @@ void Scanner::skipLine(const char *&First, const char *const End) {
467465

468466
if (First[1] != '*') {
469467
LastTokenPtr = First;
470-
if (!isWhitespace(*First))
471-
LastNonWhitespace = *First;
472468
++First;
473469
continue;
474470
}
@@ -480,9 +476,26 @@ void Scanner::skipLine(const char *&First, const char *const End) {
480476
return;
481477

482478
// Skip over the newline.
483-
skipNewline(First, End);
484-
485-
if (LastNonWhitespace != '\\')
479+
auto Len = skipNewline(First, End);
480+
// Since P2223R2 allows the line-continuation slash \ to be followed by
481+
// additional whitespace, we need to check that here if `First`
482+
// follows a `\\` and whitespaces.
483+
484+
// `LookBack` points to the character before the newline:
485+
const char *LookBack = First - Len;
486+
bool LineContinuationFound = false;
487+
488+
// Move `LookBack` backwards to find line-continuation and whitespaces:
489+
while (LookBack >= OldFirst) { // bound `LookBack` by `OldFirst`:
490+
if (isWhitespace(*LookBack)) {
491+
--LookBack; // whitespace before '\\' is ok
492+
continue;
493+
}
494+
if (*LookBack == '\\')
495+
LineContinuationFound = true;
496+
break; // not a whitespace, end loop
497+
}
498+
if (!LineContinuationFound)
486499
break;
487500
}
488501
}

clang/unittests/Lex/DependencyDirectivesScannerTest.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,37 @@ TEST(MinimizeSourceToDependencyDirectivesTest,
880880
EXPECT_EQ(pp_eof, Directives[22].Kind);
881881
}
882882

883+
TEST(MinimizeSourceToDependencyDirectivesTest,
884+
TestFixedBugThatReportUnterminatedDirectiveFalsely) {
885+
SmallVector<char, 512> Out;
886+
SmallVector<dependency_directives_scan::Token, 16> Tokens;
887+
SmallVector<Directive, 16> Directives;
888+
889+
StringRef Input = "#ifndef __TEST \n"
890+
"#define __TEST \n"
891+
"#if defined(__TEST_DUMMY) \n"
892+
"#if defined(__TEST_DUMMY2) \n"
893+
"#pragma GCC warning \\ \n"
894+
"\"hello!\"\n"
895+
"#else\n"
896+
"#pragma GCC error \\ \n"
897+
"\"world!\" \n"
898+
"#endif // defined(__TEST_DUMMY2) \n"
899+
"#endif // defined(__TEST_DUMMY) \n"
900+
"#endif // #ifndef __TEST \n";
901+
ASSERT_FALSE( // False on no error:
902+
minimizeSourceToDependencyDirectives(Input, Out, Tokens, Directives));
903+
ASSERT_TRUE(Directives.size() == 8);
904+
EXPECT_EQ(pp_ifndef, Directives[0].Kind);
905+
EXPECT_EQ(pp_define, Directives[1].Kind);
906+
EXPECT_EQ(pp_if, Directives[2].Kind);
907+
EXPECT_EQ(pp_if, Directives[3].Kind);
908+
EXPECT_EQ(pp_endif, Directives[4].Kind);
909+
EXPECT_EQ(pp_endif, Directives[5].Kind);
910+
EXPECT_EQ(pp_endif, Directives[6].Kind);
911+
EXPECT_EQ(pp_eof, Directives[7].Kind);
912+
}
913+
883914
TEST(MinimizeSourceToDependencyDirectivesTest, PoundWarningAndError) {
884915
SmallVector<char, 128> Out;
885916

0 commit comments

Comments
 (0)