[PATCH] D37700: Fix recording preamble's conditional stack in skipped PP branches.

2017-09-12 Thread Nikolai Kosjar via Phabricator via cfe-commits 
nik added a comment.

In https://reviews.llvm.org/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 .


Repository:
  rL LLVM

https://reviews.llvm.org/D37700



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37700: Fix recording preamble's conditional stack in skipped PP branches.

2017-09-12 Thread Ilya Biryukov via Phabricator via cfe-commits 
ilya-biryukov added a comment.

In https://reviews.llvm.org/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?


Repository:
  rL LLVM

https://reviews.llvm.org/D37700



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37700: Fix recording preamble's conditional stack in skipped PP branches.

2017-09-12 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL313014: Fix recording preamble's conditional stack in 
skipped PP branches. (authored by ibiryukov).

Repository:
  rL LLVM

https://reviews.llvm.org/D37700

Files:
  cfe/trunk/lib/Lex/PPDirectives.cpp
  cfe/trunk/test/Index/preamble-conditionals-inverted-with-error.cpp
  cfe/trunk/test/Index/preamble-conditionals-inverted.cpp


Index: cfe/trunk/lib/Lex/PPDirectives.cpp
===
--- cfe/trunk/lib/Lex/PPDirectives.cpp
+++ cfe/trunk/lib/Lex/PPDirectives.cpp
@@ -383,15 +383,8 @@
 
 // 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;
 }
Index: cfe/trunk/test/Index/preamble-conditionals-inverted-with-error.cpp
===
--- cfe/trunk/test/Index/preamble-conditionals-inverted-with-error.cpp
+++ cfe/trunk/test/Index/preamble-conditionals-inverted-with-error.cpp
@@ -0,0 +1,8 @@
+// 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
Index: cfe/trunk/test/Index/preamble-conditionals-inverted.cpp
===
--- cfe/trunk/test/Index/preamble-conditionals-inverted.cpp
+++ cfe/trunk/test/Index/preamble-conditionals-inverted.cpp
@@ -0,0 +1,8 @@
+// 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


Index: cfe/trunk/lib/Lex/PPDirectives.cpp
===
--- cfe/trunk/lib/Lex/PPDirectives.cpp
+++ cfe/trunk/lib/Lex/PPDirectives.cpp
@@ -383,15 +383,8 @@
 
 // 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;
 }
Index: cfe/trunk/test/Index/preamble-conditionals-inverted-with-error.cpp
===
--- cfe/trunk/test/Index/preamble-conditionals-inverted-with-error.cpp
+++ cfe/trunk/test/Index/preamble-conditionals-inverted-with-error.cpp
@@ -0,0 +1,8 @@
+// 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
Index: cfe/trunk/test/Index/preamble-conditionals-inverted.cpp
===
--- cfe/trunk/test/Index/preamble-conditionals-inverted.cpp
+++ cfe/trunk/test/Index/preamble-conditionals-inverted.cpp
@@ -0,0 +1,8 @@
+// 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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37700: Fix recording preamble's conditional stack in skipped PP branches.

2017-09-12 Thread Erik Verbruggen via Phabricator via cfe-commits 
erikjv added a comment.

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


https://reviews.llvm.org/D37700



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37700: Fix recording preamble's conditional stack in skipped PP branches.

2017-09-12 Thread Nikolai Kosjar via Phabricator via cfe-commits 
nik added a comment.

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/12017-09-12 09:33:42.210204021 +0200
  +++ /tmp/22017-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}
  %


https://reviews.llvm.org/D37700



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37700: Fix recording preamble's conditional stack in skipped PP branches.

2017-09-11 Thread Ilya Biryukov via Phabricator via cfe-commits 
ilya-biryukov updated this revision to Diff 114634.
ilya-biryukov added a comment.

Fixed description of the change.


https://reviews.llvm.org/D37700

Files:
  lib/Lex/PPDirectives.cpp
  test/Index/preamble-conditionals-inverted-with-error.cpp
  test/Index/preamble-conditionals-inverted.cpp


Index: test/Index/preamble-conditionals-inverted.cpp
===
--- /dev/null
+++ test/Index/preamble-conditionals-inverted.cpp
@@ -0,0 +1,8 @@
+// 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
Index: test/Index/preamble-conditionals-inverted-with-error.cpp
===
--- /dev/null
+++ test/Index/preamble-conditionals-inverted-with-error.cpp
@@ -0,0 +1,8 @@
+// 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
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -381,15 +381,8 @@
 
 // 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;
 }


Index: test/Index/preamble-conditionals-inverted.cpp
===
--- /dev/null
+++ test/Index/preamble-conditionals-inverted.cpp
@@ -0,0 +1,8 @@
+// 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
Index: test/Index/preamble-conditionals-inverted-with-error.cpp
===
--- /dev/null
+++ test/Index/preamble-conditionals-inverted-with-error.cpp
@@ -0,0 +1,8 @@
+// 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
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -381,15 +381,8 @@
 
 // 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;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37700: Fix recording preamble's conditional stack in skipped PP branches.

2017-09-11 Thread Ilya Biryukov via Phabricator via cfe-commits 
ilya-biryukov created this revision.

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`.


https://reviews.llvm.org/D37700

Files:
  lib/Lex/PPDirectives.cpp
  test/Index/preamble-conditionals-inverted-with-error.cpp
  test/Index/preamble-conditionals-inverted.cpp


Index: test/Index/preamble-conditionals-inverted.cpp
===
--- /dev/null
+++ test/Index/preamble-conditionals-inverted.cpp
@@ -0,0 +1,8 @@
+// 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
Index: test/Index/preamble-conditionals-inverted-with-error.cpp
===
--- /dev/null
+++ test/Index/preamble-conditionals-inverted-with-error.cpp
@@ -0,0 +1,8 @@
+// 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
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -381,15 +381,8 @@
 
 // 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;
 }


Index: test/Index/preamble-conditionals-inverted.cpp
===
--- /dev/null
+++ test/Index/preamble-conditionals-inverted.cpp
@@ -0,0 +1,8 @@
+// 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
Index: test/Index/preamble-conditionals-inverted-with-error.cpp
===
--- /dev/null
+++ test/Index/preamble-conditionals-inverted-with-error.cpp
@@ -0,0 +1,8 @@
+// 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
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -381,15 +381,8 @@
 
 // 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;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits