Fix recording preamble's conditional stack in skipped PP branches.

[ilya-biryukov]

Summary

This fixes PR34547.
Lexer::LexEndOfFile handles recording of ConditionalStack for
preamble and reporting errors about unmatched conditionalal PP
directives.
However, SkipExcludedConditionalBlock contianed duplicated logic for
reporting errors and clearing ConditionalStack, but not for preamble
recording.

This fix removes error reporting logic from
SkipExcludedConditionalBlock, unmatched PP conditionals are now
reported inside Lexer::LexEndOfFile.

[ilya-biryukov]

Fixed description of the change.

[nik]

Fixes the reported issue, thanks!

I still run into another case that is not yet properly covered. The reported issue was extracted from the following case. If you prefer, I'll create a separate report for the remaining issues.

The follow code outlines the problems in the comments:

#ifdef MYCPLUSPLUS
extern "C" { // On parse this is skipped as expected. On reparse, this is surprisingly not skipped anymore.
#endif

#ifdef MYCPLUSPLUS
}
#endif

int main()
{
    return 0;
} // On reparse a diagnostic is added here: "expected '}' to match this '{' (line 2)"

Here are some c-index-test invocations for this to make this testable/reproducible ($FILE does not contain the above comments):

Do a parse and save the tokens:

% ./c-index-test -test-annotate-tokens=$FILE:1:1:7:1 $FILE > /tmp/1

No diagnostics are emitted as expected.

Do a parse and reparse and save also the tokens:

% CINDEXTEST_EDITING=1 ./c-index-test -test-annotate-tokens=$FILE:1:1:7:1 $FILE > /tmp/2
$FILE:12:2: error: expected '}'
Number FIX-ITs = 0
$FILE:2:12: note: to match this '{'
Number FIX-ITs = 0

Note that a new diagnostic is emitted on reparse at the bottom of the file (issue1).
Also, the first skipped source range is not skipped anymore on reparse (issue2):

% diff -u /tmp/{1,2}
--- /tmp/1	2017-09-12 09:33:42.210204021 +0200
+++ /tmp/2	2017-09-12 09:33:51.458334870 +0200
@@ -1,4 +1,3 @@
-Skipping: [1:2 - 3:7]
 Skipping: [5:2 - 7:7]
 Punctuation: "#" [1:1 - 1:2] preprocessing directive=
 Identifier: "ifdef" [1:2 - 1:7] preprocessing directive=
zsh: exit 1     colordiff -u /tmp/{1,2}
%

[erikjv]

I'd put/fix Nik's issue in a separate patch.

[ilya-biryukov]

In D37700#867625, @erikjv wrote:

I'd put/fix Nik's issue in a separate patch.

Totally agree. It seems like a separate issue, though maybe related.

@nik, could you file a separate bug so that we won't forget about it?

[nik]

In D37700#867646, @ilya-biryukov wrote:

@nik, could you file a separate bug so that we won't forget about it?

Done, it's https://bugs.llvm.org/show_bug.cgi?id=34570 .

lib/Lex/PPDirectives.cpp

if (Tok.is(tok::code_completion)) {
       if (CodeComplete)
         CodeComplete->CodeCompleteInConditionalExclusion();
       setCodeCompletionReached();
       continue;
     }
 
     // If this is the end of the buffer, we have an error.
     if (Tok.is(tok::eof)) {
-      // Emit errors for each unterminated conditional on the stack, including
-      // the current one.
-      while (!CurPPLexer->ConditionalStack.empty()) {
-        if (CurLexer->getFileLoc() != CodeCompletionFileLoc)
-          Diag(CurPPLexer->ConditionalStack.back().IfLoc,
-               diag::err_pp_unterminated_conditional);
-        CurPPLexer->ConditionalStack.pop_back();
-      }
-
+      // We don't emit errors for unterminated conditionals here,
+      // Lexer::LexEndOfFile can do that propertly.
       // Just return and let the caller lex after this #include.
       break;
     }
 
     // If this token is not a preprocessor directive, just skip it.
     if (Tok.isNot(tok::hash) || !Tok.isAtStartOfLine())
       continue;
 

test/Index/preamble-conditionals-inverted-with-error.cpp

+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-load-source-reparse 5 \
+// RUN:                                       local -std=c++14 %s 2>&1 \
+// RUN: | FileCheck %s
+#ifdef FOO_H
+
+void foo();
+
+// CHECK: preamble-conditionals-inverted-with-error.cpp:4:2: error: unterminated conditional directive

test/Index/preamble-conditionals-inverted.cpp

+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-load-source-reparse 5 \
+// RUN:                                       local -std=c++14 %s 2>&1 \
+// RUN: | FileCheck %s --implicit-check-not "error:"
+#ifdef FOO_H
+
+void foo();
+
+#endif