from:"Nikolai Kosjar via Phabricator via cfe\-commits"

[PATCH] D46050: [Frontend] Avoid running plugins during code completion parse

2018-05-11 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping


https://reviews.llvm.org/D46050



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

[PATCH] D46050: [Frontend] Avoid running plugins during code completion parse

2018-05-16 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 146999.
nik added a comment.

Addressed inline nit.


Repository:
  rC Clang

https://reviews.llvm.org/D46050

Files:
  lib/Frontend/FrontendAction.cpp
  test/Index/complete-and-plugins.c


Index: test/Index/complete-and-plugins.c
===
--- /dev/null
+++ test/Index/complete-and-plugins.c
@@ -0,0 +1,7 @@
+// RUN: c-index-test -code-completion-at=%s:7:1 -load 
%llvmshlibdir/PrintFunctionNames%pluginext -add-plugin print-fns %s | FileCheck 
%s
+// REQUIRES: plugins, examples
+// CHECK: macro definition:{{.*}}
+// CHECK-NOT: top-level-decl: "x"
+
+void x();
+
Index: lib/Frontend/FrontendAction.cpp
===
--- lib/Frontend/FrontendAction.cpp
+++ lib/Frontend/FrontendAction.cpp
@@ -153,6 +153,10 @@
   if (FrontendPluginRegistry::begin() == FrontendPluginRegistry::end())
 return Consumer;
 
+  // If this is a code completion run, avoid invoking the plugin consumers
+  if (CI.hasCodeCompletionConsumer())
+return Consumer;
+
   // Collect the list of plugins that go before the main action (in Consumers)
   // or after it (in AfterConsumers)
   std::vector> Consumers;


Index: test/Index/complete-and-plugins.c
===
--- /dev/null
+++ test/Index/complete-and-plugins.c
@@ -0,0 +1,7 @@
+// RUN: c-index-test -code-completion-at=%s:7:1 -load %llvmshlibdir/PrintFunctionNames%pluginext -add-plugin print-fns %s | FileCheck %s
+// REQUIRES: plugins, examples
+// CHECK: macro definition:{{.*}}
+// CHECK-NOT: top-level-decl: "x"
+
+void x();
+
Index: lib/Frontend/FrontendAction.cpp
===
--- lib/Frontend/FrontendAction.cpp
+++ lib/Frontend/FrontendAction.cpp
@@ -153,6 +153,10 @@
   if (FrontendPluginRegistry::begin() == FrontendPluginRegistry::end())
 return Consumer;
 
+  // If this is a code completion run, avoid invoking the plugin consumers
+  if (CI.hasCodeCompletionConsumer())
+return Consumer;
+
   // Collect the list of plugins that go before the main action (in Consumers)
   // or after it (in AfterConsumers)
   std::vector> Consumers;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46050: [Frontend] Avoid running plugins during code completion parse

2018-05-16 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

If this is fine now, please submit as I don't have the permissions to do so.


Repository:
  rC Clang

https://reviews.llvm.org/D46050



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

[PATCH] D46862: [libclang] Optionally add code completion results for arrow instead of dot

2018-05-29 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik requested changes to this revision.
nik added a comment.
This revision now requires changes to proceed.

Generates build errors here:

In file included from 
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c:4:0:
/d2/llvm/qtc/source/tools/clang/include/clang-c/Index.h:1332:62: warning: comma 
at end of enumerator list [-Wpedantic]
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c: In function 
'tokens_spelling_at_range':
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c:2263:3: 
error: 'for' loop initial declarations are only allowed in C99 or C11 mode
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c:2263:3: note: 
use option -std=c99, -std=gnu99, -std=c11 or -std=gnu11 to compile your code
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c: In function 
'print_completion_result':
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c:2365:3: 
error: 'for' loop initial declarations are only allowed in C99 or C11 mode
At top level:
/d2/llvm/qtc/source/tools/clang/tools/c-index-test/c-index-test.c:2257:13: 
warning: 'tokens_spelling_at_range' defined but not used [-Wunused-function]




Comment at: include/clang-c/Index.h:4753
+ *
+ * \returns The token starting with the given location or NULL if no such 
tokens
+ * exist. The returned pointer must be freed with clang_disposeTokens before 
the

s/tokens/token



Comment at: include/clang-c/Index.h:5256
 /**
+ * Retrieve the number of fix-its for the given completion string.
+ */

s/string/index



Comment at: include/clang-c/Index.h:5264
+ * FixIts that *must* be applied before inserting the text for the
+ * corresponding completion item.
+ *

* Please use proper documentation style, e.g. make use of "\brief", "\param", 
"\returns". Currently one has to somewhat guess that the returned string is 
related to the replacement_range parameter.

* Is this a behavior change now? What happens to old code not aware of this 
function?



Comment at: include/clang-c/Index.h:5266
+ *
+ * Completion items with non-empty fixits will not be returned by default, they
+ * should be explicitly requested by setting CXCodeComplete_IncludeFixIts. For

Please avoid double negation.



Comment at: include/clang-c/Index.h:5268
+ * should be explicitly requested by setting CXCodeComplete_IncludeFixIts. For
+ * the editors to be able to compute position of the cursor for the completion
+ * item itself, the following conditions are guaranteed to hold for RemoveRange

s/editors/clients/



Comment at: include/clang-c/Index.h:5269
+ * the editors to be able to compute position of the cursor for the completion
+ * item itself, the following conditions are guaranteed to hold for RemoveRange
+ * of the stored fixits:

Do you mean replacement_range instead of RemoveRange?



Comment at: include/clang-c/Index.h:5284
+ *  vec_ptr.^  // completion returns an item 'push_back', replacing '.'
+ *  with '->' vec_ptr->^ // completion returns an item 'release',
+ *  replacing '->' with '.'

The vec_ptr-> example should go to its own line.



Comment at: include/clang-c/Index.h:5326
+   * Whether to include completion items with small
+   * fix-its, e.g. change '.' to '->' on member access, etc.
+   */

Currently there is not more, so I would avoid the "etc.".



Comment at: test/Index/complete-arrow-dot.cpp:20
+
+// CHECK-NOT: CXXMethod:{ResultType void}{TypedText doSomething}{LeftParen 
(}{RightParen )} (34) (requires fix-it: {10:7-10:8} to "->")
+// CHECK-NOT: FieldDecl:{ResultType int}{TypedText field} (35) (requires 
fix-it: {10:7-10:8} to "->")

That's pretty verbose, consider to use wildcards/regexes instead just checking 
for "requires fix-it:".



Comment at: tools/c-index-test/c-index-test.c:2478
 completionOptions |= CXCodeComplete_SkipPreamble;
+  if (getenv("CINDEXTEST_COMPLETION_INCLUDE_CORRECTIONS"))
+completionOptions |= CXCodeComplete_IncludeFixIts;

Keep terminology of the API - s/CORRECTIONS/FIXITS


https://reviews.llvm.org/D46862



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

[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS

2017-09-27 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping 4


https://reviews.llvm.org/D37554



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

[PATCH] D38578: [preamble] Also record the "skipping" state of the preprocessor

2017-10-16 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

This fixes the reported bug for me :)

There is another related issue that is not addressed by this change. I've 
reported it as

[preamble] Skipped ranges vanish after reparse (#ifdef with #include) 
 https://bugs.llvm.org/show_bug.cgi?id=34971


https://reviews.llvm.org/D38578



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

[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS

2017-10-23 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 119823.
nik added a comment.

Rebased and took over better wording/description from Ilya.


https://reviews.llvm.org/D37554

Files:
  tools/libclang/CIndex.cpp
  tools/libclang/CIndexCodeCompletion.cpp
  tools/libclang/Indexing.cpp


Index: tools/libclang/Indexing.cpp
===
--- tools/libclang/Indexing.cpp
+++ tools/libclang/Indexing.cpp
@@ -880,11 +880,6 @@
 TU_options);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-IndexSourceFileImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, IndexSourceFileImpl)) {
@@ -934,11 +929,6 @@
 index_options, TU);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-IndexTranslationUnitImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, IndexTranslationUnitImpl)) {
Index: tools/libclang/CIndexCodeCompletion.cpp
===
--- tools/libclang/CIndexCodeCompletion.cpp
+++ tools/libclang/CIndexCodeCompletion.cpp
@@ -806,11 +806,6 @@
 llvm::makeArrayRef(unsaved_files, num_unsaved_files), options);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-CodeCompleteAtImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, CodeCompleteAtImpl)) {
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3508,11 +3508,6 @@
 llvm::makeArrayRef(unsaved_files, num_unsaved_files), options, out_TU);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-ParseTranslationUnitImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, ParseTranslationUnitImpl)) {
@@ -3921,8 +3916,7 @@
 result = clang_saveTranslationUnit_Impl(TU, FileName, options);
   };
 
-  if (!CXXUnit->getDiagnostics().hasUnrecoverableErrorOccurred() ||
-  getenv("LIBCLANG_NOTHREADS")) {
+  if (!CXXUnit->getDiagnostics().hasUnrecoverableErrorOccurred()) {
 SaveTranslationUnitImpl();
 
 if (getenv("LIBCLANG_RESOURCE_USAGE"))
@@ -4045,11 +4039,6 @@
 TU, llvm::makeArrayRef(unsaved_files, num_unsaved_files), options);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-ReparseTranslationUnitImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, ReparseTranslationUnitImpl)) {
@@ -8164,7 +8153,7 @@
unsigned Size) {
   if (!Size)
 Size = GetSafetyThreadStackSize();
-  if (Size)
+  if (Size && !getenv("LIBCLANG_NOTHREADS"))
 return CRC.RunSafelyOnThread(Fn, Size);
   return CRC.RunSafely(Fn);
 }


Index: tools/libclang/Indexing.cpp
===
--- tools/libclang/Indexing.cpp
+++ tools/libclang/Indexing.cpp
@@ -880,11 +880,6 @@
 TU_options);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-IndexSourceFileImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, IndexSourceFileImpl)) {
@@ -934,11 +929,6 @@
 index_options, TU);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-IndexTranslationUnitImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, IndexTranslationUnitImpl)) {
Index: tools/libclang/CIndexCodeCompletion.cpp
===
--- tools/libclang/CIndexCodeCompletion.cpp
+++ tools/libclang/CIndexCodeCompletion.cpp
@@ -806,11 +806,6 @@
 llvm::makeArrayRef(unsaved_files, num_unsaved_files), options);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-CodeCompleteAtImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, CodeCompleteAtImpl)) {
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3508,11 +3508,6 @@
 llvm::makeArrayRef(unsaved_files, num_unsaved_files), options, out_TU);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-ParseTranslationUnitImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, ParseTranslationUnitImpl)) {
@@ -3921,8 +3916,7 @@
 result = clang_saveTranslationUnit_Impl(TU, FileName, options);
   };
 
-  if (!CXXUnit->getDiagnostics().hasUnrecoverableErrorOccurred() ||
-  getenv("LIBCLANG_NOTHREADS")) {
+  if (!CXXUnit->getDiagnostics().hasUnrecoverableErrorOccurred()) {
 SaveTranslationUnitImpl();
 
 if (getenv("LIBCLANG_RESOURCE_USAGE"))
@@ -4045,11 +4039,6 @@
 TU, llvm::makeArrayRef(unsaved_files, num_unsaved_files), options);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-ReparseTranslationUnitImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, ReparseTranslationUnitImpl)) {
@@ -8164

[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS

2017-10-23 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Hmm, apparently "arc diff --update https://reviews.llvm.org/D37554"; did not 
take the new commit message into account. Changed it manually with the web 
interface.

Ilya, I hope it's OK if I take your description :)

> That said, I am not familiar with the code you're changing, so can't really 
> LGTM this. Hopefully, someone else can do that.

So who would be the appropriate reviewer here? Manuel, any idea?


https://reviews.llvm.org/D37554



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

[PATCH] D40481: [libclang] Fix cursors for arguments of Subscript and Call operators

2018-02-07 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping...


https://reviews.llvm.org/D40481



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

[PATCH] D33042: [libclang] Allow to suspend a translation unit.

2017-05-29 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

I'm new to this review tool. I've addressed the comments and rebased. Does this 
needs a re-review? Note that this is my first change and I probably do not have 
any permissions to submit this change.


https://reviews.llvm.org/D33042



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

[PATCH] D40072: [libclang] Support querying whether a declaration is invalid

2017-12-14 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping... "Please submit as I don't have the permissions."


https://reviews.llvm.org/D40072



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

[PATCH] D40561: [libclang] Fix cursors for functions with trailing return type

2017-12-14 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping..."Please submit as I don't have the permissions for this."


https://reviews.llvm.org/D40561



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

[PATCH] D39903: [libclang] Allow pretty printing declarations

2017-12-14 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping.


Repository:
  rC Clang

https://reviews.llvm.org/D39903



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

[PATCH] D40481: [libclang] Fix cursors for arguments of Subscript and Call operators

2017-12-14 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping.


https://reviews.llvm.org/D40481



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

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2017-12-14 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added inline comments.



Comment at: include/clang/Frontend/ASTUnit.h:196
+  /// \brief Counter indicating how often the preamble was build in total.
+  unsigned PreambleCounter;
+

ilya-biryukov wrote:
> nik wrote:
> > Any better name for this one? Otherwise I would suggest renaming 
> > PreambleRebuildCounter to PreambleRebuildCountdownCounter to make them more 
> > distinct.
> +1, names PreambleRebuildCounter and PreambleCounter are too similar.
> 
OK, will address in the next patch set / diff.



Comment at: lib/Frontend/PrecompiledPreamble.cpp:461
 
 std::map::iterator Overridden =
 OverriddenFiles.find(Status.getUniqueID());

ilya-biryukov wrote:
> Will anything fail if we remove the map from `UniqueID` to hashes of 
> overriden files and the corresponding checks?
> 
> We should either document why having `UniqueID`-based checks is important or 
> remove the code doing those checks.
> Will anything fail if we remove the map from UniqueID to hashes of overriden 
> files and the corresponding checks?

Hmm, I would need to remove/replace it and run the tests.

I see these reasons for UniqueID:
* It's already there :)
* Normalization of file paths is not necessary.
* It potentially can deal with hard links, though I'm not sure whether this is 
real world use case at all.

> We should either document why having UniqueID-based checks is important or 
> remove the code doing those checks.

Agree.



Repository:
  rC Clang

https://reviews.llvm.org/D41005



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

[PATCH] D41495: [clangd] Skip function bodies when building the preamble

2018-01-02 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Hmm, could libclang profit from something like this, too? (or does it already?)


Repository:
  rL LLVM

https://reviews.llvm.org/D41495



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

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2018-01-02 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added inline comments.



Comment at: lib/Frontend/PrecompiledPreamble.cpp:461
 
 std::map::iterator Overridden =
 OverriddenFiles.find(Status.getUniqueID());

nik wrote:
> ilya-biryukov wrote:
> > Will anything fail if we remove the map from `UniqueID` to hashes of 
> > overriden files and the corresponding checks?
> > 
> > We should either document why having `UniqueID`-based checks is important 
> > or remove the code doing those checks.
> > Will anything fail if we remove the map from UniqueID to hashes of 
> > overriden files and the corresponding checks?
> 
> Hmm, I would need to remove/replace it and run the tests.
> 
> I see these reasons for UniqueID:
> * It's already there :)
> * Normalization of file paths is not necessary.
> * It potentially can deal with hard links, though I'm not sure whether this 
> is real world use case at all.
> 
> > We should either document why having UniqueID-based checks is important or 
> > remove the code doing those checks.
> 
> Agree.
> 
> Will anything fail if we remove the map from UniqueID to hashes of overriden 
> files and the corresponding checks?

OK, I've applied the following patch (on this patch set/revision here) applied 
and run check-clang - no failures here.

```
From 8f755128d1e167193c8f6de1456aec01d14307f6 Mon Sep 17 00:00:00 2001
From: Nikolai Kosjar 
Date: Tue, 2 Jan 2018 15:14:07 +0100
Subject: [PATCH] Use file path instead of uniqueID

---
 lib/Frontend/PrecompiledPreamble.cpp | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/Frontend/PrecompiledPreamble.cpp 
b/lib/Frontend/PrecompiledPreamble.cpp
index a43205a1c3..09e2b20a48 100644
--- a/lib/Frontend/PrecompiledPreamble.cpp
+++ b/lib/Frontend/PrecompiledPreamble.cpp
@@ -433,7 +433,7 @@ bool PrecompiledPreamble::CanReuse(const CompilerInvocation 
&Invocation,
   // Check that none of the files used by the preamble have changed.
   // First, make a record of those files that have been overridden via
   // remapping or unsaved_files.
-  std::map OverriddenFiles;
+  std::map OverriddenFiles;
   for (const auto &R : PreprocessorOpts.RemappedFiles) {
 vfs::Status Status;
 if (!moveOnNoError(VFS->status(R.second), Status)) {
@@ -442,7 +442,7 @@ bool PrecompiledPreamble::CanReuse(const CompilerInvocation 
&Invocation,
   return false;
 }
 
-OverriddenFiles[Status.getUniqueID()] = PreambleFileHash::createForFile(
+OverriddenFiles[R.first] = PreambleFileHash::createForFile(
 Status.getSize(), 
llvm::sys::toTimeT(Status.getLastModificationTime()));
   }
 
@@ -470,8 +470,8 @@ bool PrecompiledPreamble::CanReuse(const CompilerInvocation 
&Invocation,
 return false;
 }
 
-std::map::iterator Overridden =
-OverriddenFiles.find(Status.getUniqueID());
+std::map::iterator Overridden =
+OverriddenFiles.find(F.first());
 if (Overridden != OverriddenFiles.end()) {
   // This file was remapped; check whether the newly-mapped file
   // matches up with the previous mapping.
-- 
2.15.1
```


Repository:
  rC Clang

https://reviews.llvm.org/D41005



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

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2018-01-02 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 128416.
nik added a comment.

Rebased and renamed the counter variable only.

I do not feel comfortable changing the "std::map OverriddenFiles". I can do this in a follow-up change if you
want.

@Ivan: Coul you please run the tests with this change on Windows?! If it goes
well (no failures), then please also give it also a try with the additional
change (reply to "Will anything fail if we remove the map from UniqueID to
hashes of overriden files and the corresponding checks?").


Repository:
  rC Clang

https://reviews.llvm.org/D41005

Files:
  include/clang/Frontend/ASTUnit.h
  lib/Frontend/ASTUnit.cpp
  lib/Frontend/PrecompiledPreamble.cpp
  unittests/Frontend/PCHPreambleTest.cpp

Index: unittests/Frontend/PCHPreambleTest.cpp
===
--- unittests/Frontend/PCHPreambleTest.cpp
+++ unittests/Frontend/PCHPreambleTest.cpp
@@ -124,6 +124,22 @@
   }
 };
 
+TEST_F(PCHPreambleTest, ReparseDoesNotInvalidatePreambleDueToNotExistingUnsavedFile) {
+  std::string Header1 = "//./header1.h";
+  std::string MainName = "//./main.cpp";
+  AddFile(MainName, "#include \"//./header1.h\"\n"
+"int main() { return ZERO; }");
+
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_EQ(AST->getPreambleCounter(), 1U);
+
+  RemapFile(Header1, "#define ZERO 0\n");
+  ASSERT_TRUE(ReparseAST(AST));
+
+  ASSERT_EQ(AST->getPreambleCounter(), 1U);
+}
+
 TEST_F(PCHPreambleTest, ReparseWithOverriddenFileDoesNotInvalidatePreamble) {
   std::string Header1 = "//./header1.h";
   std::string Header2 = "//./header2.h";
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -446,21 +446,28 @@
 Status.getSize(), llvm::sys::toTimeT(Status.getLastModificationTime()));
   }
 
+  llvm::StringMap OverridenFileBuffers;
   for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
-vfs::Status Status;
-if (!moveOnNoError(VFS->status(RB.first), Status))
-  return false;
-
-OverriddenFiles[Status.getUniqueID()] =
+OverridenFileBuffers[RB.first] =
 PreambleFileHash::createForMemoryBuffer(RB.second);
   }
 
   // Check whether anything has changed.
   for (const auto &F : FilesInPreamble) {
+auto OverridenFileBuffer = OverridenFileBuffers.find(F.first());
+if (OverridenFileBuffer != OverridenFileBuffers.end()) {
+  // The file's buffer was remapped; check whether it matches up
+  // with the previous mapping.
+  if (OverridenFileBuffer->second != F.second)
+return false;
+  continue;
+}
+
 vfs::Status Status;
 if (!moveOnNoError(VFS->status(F.first()), Status)) {
-  // If we can't stat the file, assume that something horrible happened.
-  return false;
+// If the file's buffer is not remapped and we can't stat it,
+// assume that something horrible happened.
+return false;
 }
 
 std::map::iterator Overridden =
@@ -473,9 +480,10 @@
   continue;
 }
 
-// The file was not remapped; check whether it has changed on disk.
+// Neither the file's buffer nor the file itself was remapped;
+// check whether it has changed on disk.
 if (Status.getSize() != uint64_t(F.second.Size) ||
-llvm::sys::toTimeT(Status.getLastModificationTime()) !=
+ llvm::sys::toTimeT(Status.getLastModificationTime()) !=
 F.second.ModTime)
   return false;
   }
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -189,7 +189,8 @@
 TUKind(TU_Complete), WantTiming(getenv("LIBCLANG_TIMING")),
 OwnsRemappedFileBuffers(true),
 NumStoredDiagnosticsFromDriver(0),
-PreambleRebuildCounter(0),
+PreambleRebuildCountdownCounter(0),
+PreambleCounter(0),
 NumWarningsInPreamble(0),
 ShouldCacheCodeCompletionResults(false),
 IncludeBriefCommentsInCodeCompletion(false), UserFilesAreVolatile(false),
@@ -1253,21 +1254,21 @@
 PreambleInvocationIn.getDiagnosticOpts());
   getDiagnostics().setNumWarnings(NumWarningsInPreamble);
 
-  PreambleRebuildCounter = 1;
+  PreambleRebuildCountdownCounter = 1;
   return MainFileBuffer;
 } else {
   Preamble.reset();
   PreambleDiagnostics.clear();
   TopLevelDeclsInPreamble.clear();
-  PreambleRebuildCounter = 1;
+  PreambleRebuildCountdownCounter = 1;
 }
   }
 
   // If the preamble rebuild counter > 1, it's because we previously
   // failed to build a preamble and we're not yet ready to try
   // again. Decrement the counter and return a failure.
-  if (PreambleRebuildCounter > 1) {
---PreambleRebuildCounter;
+  if (PreambleRebuildCountdownCounter > 1) {
+--PreambleRebuildCountdownCounter;
 return

[PATCH] D40481: [libclang] Fix cursors for arguments of Subscript and Call operators

2018-01-02 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

New year, new hope - ping :)


https://reviews.llvm.org/D40481



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

[PATCH] D40072: [libclang] Support querying whether a declaration is invalid

2018-01-02 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 128491.
nik added a comment.

Had to rebase.


Repository:
  rC Clang

https://reviews.llvm.org/D40072

Files:
  include/clang-c/Index.h
  test/Index/print-type-size.cpp
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp
  tools/libclang/libclang.exports


Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -290,6 +290,7 @@
 clang_isConstQualifiedType
 clang_isCursorDefinition
 clang_isDeclaration
+clang_isInvalidDeclaration
 clang_isExpression
 clang_isFileMultipleIncludeGuarded
 clang_isFunctionTypeVariadic
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -5421,6 +5421,15 @@
  (K >= CXCursor_FirstExtraDecl && K <= CXCursor_LastExtraDecl);
 }
 
+unsigned clang_isInvalidDeclaration(CXCursor C) {
+  if (clang_isDeclaration(C.kind)) {
+if (const Decl *D = getCursorDecl(C))
+  return D->isInvalidDecl();
+  }
+
+  return 0;
+}
+
 unsigned clang_isReference(enum CXCursorKind K) {
   return K >= CXCursor_FirstRef && K <= CXCursor_LastRef;
 }
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -812,6 +812,8 @@
   printf(" (variadic)");
 if (clang_Cursor_isObjCOptional(Cursor))
   printf(" (@optional)");
+if (clang_isInvalidDeclaration(Cursor))
+  printf(" (invalid)");
 
 switch (clang_getCursorExceptionSpecificationType(Cursor))
 {
Index: test/Index/print-type-size.cpp
===
--- test/Index/print-type-size.cpp
+++ test/Index/print-type-size.cpp
@@ -4,8 +4,8 @@
 
 namespace basic {
 
-// CHECK64: VarDecl=v:[[@LINE+2]]:6 (Definition) [type=void] [typekind=Void]
-// CHECK32: VarDecl=v:[[@LINE+1]]:6 (Definition) [type=void] [typekind=Void]
+// CHECK64: VarDecl=v:[[@LINE+2]]:6 (Definition) (invalid) [type=void] 
[typekind=Void]
+// CHECK32: VarDecl=v:[[@LINE+1]]:6 (Definition) (invalid) [type=void] 
[typekind=Void]
 void v;
 
 // CHECK64: VarDecl=v1:[[@LINE+2]]:7 (Definition) [type=void *] 
[typekind=Pointer] [sizeof=8] [alignof=8]
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -32,7 +32,7 @@
  * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
  */
 #define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 45
+#define CINDEX_VERSION_MINOR 46
 
 #define CINDEX_VERSION_ENCODE(major, minor) ( \
   ((major) * 1)   \
@@ -2641,6 +2641,16 @@
  */
 CINDEX_LINKAGE unsigned clang_isDeclaration(enum CXCursorKind);
 
+/**
+ * \brief Determine whether the given declaration is invalid.
+ *
+ * A declaration is invalid if it could not be parsed successfully.
+ *
+ * \returns non-zero if the cursor represents a declaration and it is
+ * invalid, otherwise NULL.
+ */
+CINDEX_LINKAGE unsigned clang_isInvalidDeclaration(CXCursor);
+
 /**
  * \brief Determine whether the given cursor kind represents a simple
  * reference.


Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -290,6 +290,7 @@
 clang_isConstQualifiedType
 clang_isCursorDefinition
 clang_isDeclaration
+clang_isInvalidDeclaration
 clang_isExpression
 clang_isFileMultipleIncludeGuarded
 clang_isFunctionTypeVariadic
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -5421,6 +5421,15 @@
  (K >= CXCursor_FirstExtraDecl && K <= CXCursor_LastExtraDecl);
 }
 
+unsigned clang_isInvalidDeclaration(CXCursor C) {
+  if (clang_isDeclaration(C.kind)) {
+if (const Decl *D = getCursorDecl(C))
+  return D->isInvalidDecl();
+  }
+
+  return 0;
+}
+
 unsigned clang_isReference(enum CXCursorKind K) {
   return K >= CXCursor_FirstRef && K <= CXCursor_LastRef;
 }
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -812,6 +812,8 @@
   printf(" (variadic)");
 if (clang_Cursor_isObjCOptional(Cursor))
   printf(" (@optional)");
+if (clang_isInvalidDeclaration(Cursor))
+  printf(" (invalid)");
 
 switch (clang_getCursorExceptionSpecificationType(Cursor))
 {
Index: test/Index/print-type-size.cpp
===
--- test/Index/print-type-size.cpp
+++ test/Index/print-type-size.cpp
@@ -4,8 +4,8 @@
 
 namespace basic {
 
-// CHECK64: VarDecl=v:[[@LINE+2]]:

[PATCH] D39903: [libclang] Allow pretty printing declarations

2018-01-10 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping...


Repository:
  rC Clang

https://reviews.llvm.org/D39903



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

[PATCH] D40481: [libclang] Fix cursors for arguments of Subscript and Call operators

2018-01-10 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping...


https://reviews.llvm.org/D40481



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

[PATCH] D39903: [libclang] Allow pretty printing declarations

2018-01-10 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 129258.
nik added a comment.

Rebased only, please review.


Repository:
  rC Clang

https://reviews.llvm.org/D39903

Files:
  include/clang-c/Index.h
  test/Index/print-display-names.cpp
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp
  tools/libclang/libclang.exports

Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -178,6 +178,8 @@
 clang_getCursorCompletionString
 clang_getCursorDefinition
 clang_getCursorDisplayName
+clang_getCursorPrintingPolicy
+clang_getCursorPrettyPrinted
 clang_getCursorExtent
 clang_getCursorExceptionSpecificationType
 clang_getCursorKind
@@ -359,3 +361,56 @@
 clang_EvalResult_getAsDouble
 clang_EvalResult_getAsStr
 clang_EvalResult_dispose
+clang_PrintingPolicy_dispose
+clang_PrintingPolicy_getIndentation
+clang_PrintingPolicy_getSuppressSpecifiers
+clang_PrintingPolicy_getSuppressTagKeyword
+clang_PrintingPolicy_getIncludeTagDefinition
+clang_PrintingPolicy_getSuppressScope
+clang_PrintingPolicy_getSuppressUnwrittenScope
+clang_PrintingPolicy_getSuppressInitializers
+clang_PrintingPolicy_getConstantArraySizeAsWritten
+clang_PrintingPolicy_getAnonymousTagLocations
+clang_PrintingPolicy_getSuppressStrongLifetime
+clang_PrintingPolicy_getSuppressLifetimeQualifiers
+clang_PrintingPolicy_getSuppressTemplateArgsInCXXConstructors
+clang_PrintingPolicy_getBool
+clang_PrintingPolicy_getRestrict
+clang_PrintingPolicy_getAlignof
+clang_PrintingPolicy_getUnderscoreAlignof
+clang_PrintingPolicy_getUseVoidForZeroParams
+clang_PrintingPolicy_getTerseOutput
+clang_PrintingPolicy_getPolishForDeclaration
+clang_PrintingPolicy_getHalf
+clang_PrintingPolicy_getMSWChar
+clang_PrintingPolicy_getIncludeNewlines
+clang_PrintingPolicy_getMSVCFormatting
+clang_PrintingPolicy_getConstantsAsWritten
+clang_PrintingPolicy_getSuppressImplicitBase
+clang_PrintingPolicy_getFullyQualifiedName
+clang_PrintingPolicy_setIndentation
+clang_PrintingPolicy_setSuppressSpecifiers
+clang_PrintingPolicy_setSuppressTagKeyword
+clang_PrintingPolicy_setIncludeTagDefinition
+clang_PrintingPolicy_setSuppressScope
+clang_PrintingPolicy_setSuppressUnwrittenScope
+clang_PrintingPolicy_setSuppressInitializers
+clang_PrintingPolicy_setConstantArraySizeAsWritten
+clang_PrintingPolicy_setAnonymousTagLocations
+clang_PrintingPolicy_setSuppressStrongLifetime
+clang_PrintingPolicy_setSuppressLifetimeQualifiers
+clang_PrintingPolicy_setSuppressTemplateArgsInCXXConstructors
+clang_PrintingPolicy_setBool
+clang_PrintingPolicy_setRestrict
+clang_PrintingPolicy_setAlignof
+clang_PrintingPolicy_setUnderscoreAlignof
+clang_PrintingPolicy_setUseVoidForZeroParams
+clang_PrintingPolicy_setTerseOutput
+clang_PrintingPolicy_setPolishForDeclaration
+clang_PrintingPolicy_setHalf
+clang_PrintingPolicy_setMSWChar
+clang_PrintingPolicy_setIncludeNewlines
+clang_PrintingPolicy_setMSVCFormatting
+clang_PrintingPolicy_setConstantsAsWritten
+clang_PrintingPolicy_setSuppressImplicitBase
+clang_PrintingPolicy_setFullyQualifiedName
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -4706,6 +4706,107 @@
   return cxstring::createSet(Manglings);
 }
 
+CXPrintingPolicy clang_getCursorPrintingPolicy(CXCursor C) {
+  if (clang_Cursor_isNull(C))
+return 0;
+  return new PrintingPolicy(getCursorContext(C).getPrintingPolicy());
+}
+
+void clang_PrintingPolicy_dispose(CXPrintingPolicy policy) {
+  if (policy)
+delete static_cast(policy);
+}
+
+#define DEFINE_PRINTING_POLICY_GETTER(name)\
+  unsigned clang_PrintingPolicy_get##name(CXPrintingPolicy policy) {   \
+if (!policy)   \
+  return 0;\
+return static_cast(policy)->name;\
+  }
+DEFINE_PRINTING_POLICY_GETTER(Indentation)
+DEFINE_PRINTING_POLICY_GETTER(SuppressSpecifiers)
+DEFINE_PRINTING_POLICY_GETTER(SuppressTagKeyword)
+DEFINE_PRINTING_POLICY_GETTER(IncludeTagDefinition)
+DEFINE_PRINTING_POLICY_GETTER(SuppressScope)
+DEFINE_PRINTING_POLICY_GETTER(SuppressUnwrittenScope)
+DEFINE_PRINTING_POLICY_GETTER(SuppressInitializers)
+DEFINE_PRINTING_POLICY_GETTER(ConstantArraySizeAsWritten)
+DEFINE_PRINTING_POLICY_GETTER(AnonymousTagLocations)
+DEFINE_PRINTING_POLICY_GETTER(SuppressStrongLifetime)
+DEFINE_PRINTING_POLICY_GETTER(SuppressLifetimeQualifiers)
+DEFINE_PRINTING_POLICY_GETTER(SuppressTemplateArgsInCXXConstructors)
+DEFINE_PRINTING_POLICY_GETTER(Bool)
+DEFINE_PRINTING_POLICY_GETTER(Restrict)
+DEFINE_PRINTING_POLICY_GETTER(Alignof)
+DEFINE_PRINTING_POLICY_GETTER(UnderscoreAlignof)
+DEFINE_PRINTING_POLICY_GETTER(UseVoidForZeroParams)
+DEFINE_PRINTING_POLICY_GETTER(TerseOutput)
+DEFINE_PRINTING_POLICY_GETTER(PolishFor

[PATCH] D39903: [libclang] Allow pretty printing declarations

2018-01-11 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 129426.
nik added a comment.

> Could one use an enum to get/set different properties of the policy?
> 
> I've seen other C-API's (for Linear and Quadratic programming) follow a 
> similar approach quite extensibly.
> 
> It would significantly reduce the size of the API.

You are right and rethinking this, I see no problem with it. I thought of that 
too (see my very first comment to this change), but then for some reason the 
PrintingPolicy::Indentation (unsigned : 8) has driven me to this version. I 
probably have only thought of strictly false/true values to set/get.

OK, adapted to enum version.


Repository:
  rC Clang

https://reviews.llvm.org/D39903

Files:
  include/clang-c/Index.h
  test/Index/print-display-names.cpp
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp
  tools/libclang/libclang.exports

Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -178,6 +178,8 @@
 clang_getCursorCompletionString
 clang_getCursorDefinition
 clang_getCursorDisplayName
+clang_getCursorPrintingPolicy
+clang_getCursorPrettyPrinted
 clang_getCursorExtent
 clang_getCursorExceptionSpecificationType
 clang_getCursorKind
@@ -359,3 +361,6 @@
 clang_EvalResult_getAsDouble
 clang_EvalResult_getAsStr
 clang_EvalResult_dispose
+clang_PrintingPolicy_get
+clang_PrintingPolicy_set
+clang_PrintingPolicy_dispose
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -4706,6 +4706,191 @@
   return cxstring::createSet(Manglings);
 }
 
+CXPrintingPolicy clang_getCursorPrintingPolicy(CXCursor C) {
+  if (clang_Cursor_isNull(C))
+return 0;
+  return new PrintingPolicy(getCursorContext(C).getPrintingPolicy());
+}
+
+void clang_PrintingPolicy_dispose(CXPrintingPolicy Policy) {
+  if (Policy)
+delete static_cast(Policy);
+}
+
+unsigned clang_PrintingPolicy_get(CXPrintingPolicy Policy,
+  enum CXPrintingPolicyProperty Property) {
+  if (!Policy)
+return 0;
+
+  PrintingPolicy *P = static_cast(Policy);
+  switch (Property) {
+  case CXPrintingPolicy_Indentation:
+return P->Indentation;
+  case CXPrintingPolicy_SuppressSpecifiers:
+return P->SuppressSpecifiers;
+  case CXPrintingPolicy_SuppressTagKeyword:
+return P->SuppressTagKeyword;
+  case CXPrintingPolicy_IncludeTagDefinition:
+return P->IncludeTagDefinition;
+  case CXPrintingPolicy_SuppressScope:
+return P->SuppressScope;
+  case CXPrintingPolicy_SuppressUnwrittenScope:
+return P->SuppressUnwrittenScope;
+  case CXPrintingPolicy_SuppressInitializers:
+return P->SuppressInitializers;
+  case CXPrintingPolicy_ConstantArraySizeAsWritten:
+return P->ConstantArraySizeAsWritten;
+  case CXPrintingPolicy_AnonymousTagLocations:
+return P->AnonymousTagLocations;
+  case CXPrintingPolicy_SuppressStrongLifetime:
+return P->SuppressStrongLifetime;
+  case CXPrintingPolicy_SuppressLifetimeQualifiers:
+return P->SuppressLifetimeQualifiers;
+  case CXPrintingPolicy_SuppressTemplateArgsInCXXConstructors:
+return P->SuppressTemplateArgsInCXXConstructors;
+  case CXPrintingPolicy_Bool:
+return P->Bool;
+  case CXPrintingPolicy_Restrict:
+return P->Restrict;
+  case CXPrintingPolicy_Alignof:
+return P->Alignof;
+  case CXPrintingPolicy_UnderscoreAlignof:
+return P->UnderscoreAlignof;
+  case CXPrintingPolicy_UseVoidForZeroParams:
+return P->UseVoidForZeroParams;
+  case CXPrintingPolicy_TerseOutput:
+return P->TerseOutput;
+  case CXPrintingPolicy_PolishForDeclaration:
+return P->PolishForDeclaration;
+  case CXPrintingPolicy_Half:
+return P->Half;
+  case CXPrintingPolicy_MSWChar:
+return P->MSWChar;
+  case CXPrintingPolicy_IncludeNewlines:
+return P->IncludeNewlines;
+  case CXPrintingPolicy_MSVCFormatting:
+return P->MSVCFormatting;
+  case CXPrintingPolicy_ConstantsAsWritten:
+return P->ConstantsAsWritten;
+  case CXPrintingPolicy_SuppressImplicitBase:
+return P->SuppressImplicitBase;
+  case CXPrintingPolicy_FullyQualifiedName:
+return P->FullyQualifiedName;
+  }
+
+  return 0;
+}
+
+void clang_PrintingPolicy_set(CXPrintingPolicy Policy,
+  enum CXPrintingPolicyProperty Property,
+  unsigned Value) {
+  if (!Policy)
+return;
+
+  PrintingPolicy *P = static_cast(Policy);
+  switch (Property) {
+  case CXPrintingPolicy_Indentation:
+P->Indentation = Value;
+break;
+  case CXPrintingPolicy_SuppressSpecifiers:
+P->SuppressSpecifiers = Value;
+break;
+  case CXPrintingPolicy_SuppressTagKeyword:
+P->SuppressTagKeyword = Value;
+break;
+  case CXPrintingPolicy_IncludeTagDefinition:
+P->IncludeTagDefinition = Value;
+break;
+  case CXPrintingPolicy_SuppressScope:
+P->SuppressScope =

[PATCH] D39903: [libclang] Allow pretty printing declarations

2018-01-11 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 129427.
nik added a comment.

Used macros as in a previous version to make it less verbose and error prone.


Repository:
  rC Clang

https://reviews.llvm.org/D39903

Files:
  include/clang-c/Index.h
  test/Index/print-display-names.cpp
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp
  tools/libclang/libclang.exports

Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -178,6 +178,8 @@
 clang_getCursorCompletionString
 clang_getCursorDefinition
 clang_getCursorDisplayName
+clang_getCursorPrintingPolicy
+clang_getCursorPrettyPrinted
 clang_getCursorExtent
 clang_getCursorExceptionSpecificationType
 clang_getCursorKind
@@ -359,3 +361,6 @@
 clang_EvalResult_getAsDouble
 clang_EvalResult_getAsStr
 clang_EvalResult_dispose
+clang_PrintingPolicy_get
+clang_PrintingPolicy_set
+clang_PrintingPolicy_dispose
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -4706,6 +4706,125 @@
   return cxstring::createSet(Manglings);
 }
 
+CXPrintingPolicy clang_getCursorPrintingPolicy(CXCursor C) {
+  if (clang_Cursor_isNull(C))
+return 0;
+  return new PrintingPolicy(getCursorContext(C).getPrintingPolicy());
+}
+
+void clang_PrintingPolicy_dispose(CXPrintingPolicy Policy) {
+  if (Policy)
+delete static_cast(Policy);
+}
+
+#define HANDLE_CASE(P, PROPERTY_NAME)  \
+  case CXPrintingPolicy_##PROPERTY_NAME:   \
+return P->PROPERTY_NAME;
+
+unsigned clang_PrintingPolicy_get(CXPrintingPolicy Policy,
+  enum CXPrintingPolicyProperty Property) {
+  if (!Policy)
+return 0;
+
+  PrintingPolicy *P = static_cast(Policy);
+  switch (Property) {
+HANDLE_CASE(P, Indentation)
+HANDLE_CASE(P, SuppressSpecifiers)
+HANDLE_CASE(P, SuppressTagKeyword)
+HANDLE_CASE(P, IncludeTagDefinition)
+HANDLE_CASE(P, SuppressScope)
+HANDLE_CASE(P, SuppressUnwrittenScope)
+HANDLE_CASE(P, SuppressInitializers)
+HANDLE_CASE(P, ConstantArraySizeAsWritten)
+HANDLE_CASE(P, AnonymousTagLocations)
+HANDLE_CASE(P, SuppressStrongLifetime)
+HANDLE_CASE(P, SuppressLifetimeQualifiers)
+HANDLE_CASE(P, SuppressTemplateArgsInCXXConstructors)
+HANDLE_CASE(P, Bool)
+HANDLE_CASE(P, Restrict)
+HANDLE_CASE(P, Alignof)
+HANDLE_CASE(P, UnderscoreAlignof)
+HANDLE_CASE(P, UseVoidForZeroParams)
+HANDLE_CASE(P, TerseOutput)
+HANDLE_CASE(P, PolishForDeclaration)
+HANDLE_CASE(P, Half)
+HANDLE_CASE(P, MSWChar)
+HANDLE_CASE(P, IncludeNewlines)
+HANDLE_CASE(P, MSVCFormatting)
+HANDLE_CASE(P, ConstantsAsWritten)
+HANDLE_CASE(P, SuppressImplicitBase)
+HANDLE_CASE(P, FullyQualifiedName)
+  }
+
+  return 0;
+}
+
+#undef HANDLE_CASE
+#define HANDLE_CASE(P, PROPERTY_NAME, VALUE)   \
+  case CXPrintingPolicy_##PROPERTY_NAME:   \
+P->PROPERTY_NAME = VALUE;  \
+break;
+
+void clang_PrintingPolicy_set(CXPrintingPolicy Policy,
+  enum CXPrintingPolicyProperty Property,
+  unsigned Value) {
+  if (!Policy)
+return;
+
+  PrintingPolicy *P = static_cast(Policy);
+  switch (Property) {
+HANDLE_CASE(P, Indentation, Value)
+HANDLE_CASE(P, SuppressSpecifiers, Value)
+HANDLE_CASE(P, SuppressTagKeyword, Value)
+HANDLE_CASE(P, IncludeTagDefinition, Value)
+HANDLE_CASE(P, SuppressScope, Value)
+HANDLE_CASE(P, SuppressUnwrittenScope, Value)
+HANDLE_CASE(P, SuppressInitializers, Value)
+HANDLE_CASE(P, ConstantArraySizeAsWritten, Value)
+HANDLE_CASE(P, AnonymousTagLocations, Value)
+HANDLE_CASE(P, SuppressStrongLifetime, Value)
+HANDLE_CASE(P, SuppressLifetimeQualifiers, Value)
+HANDLE_CASE(P, SuppressTemplateArgsInCXXConstructors, Value)
+HANDLE_CASE(P, Bool, Value)
+HANDLE_CASE(P, Restrict, Value)
+HANDLE_CASE(P, Alignof, Value)
+HANDLE_CASE(P, UnderscoreAlignof, Value)
+HANDLE_CASE(P, UseVoidForZeroParams, Value)
+HANDLE_CASE(P, TerseOutput, Value)
+HANDLE_CASE(P, PolishForDeclaration, Value)
+HANDLE_CASE(P, Half, Value)
+HANDLE_CASE(P, MSWChar, Value)
+HANDLE_CASE(P, IncludeNewlines, Value)
+HANDLE_CASE(P, MSVCFormatting, Value)
+HANDLE_CASE(P, ConstantsAsWritten, Value)
+HANDLE_CASE(P, SuppressImplicitBase, Value)
+HANDLE_CASE(P, FullyQualifiedName, Value)
+  }
+}
+
+#undef HANDLE_CASE
+
+CXString clang_getCursorPrettyPrinted(CXCursor C, CXPrintingPolicy cxPolicy) {
+  if (clang_Cursor_isNull(C))
+return cxstring::createEmpty();
+
+  if (clang_isDeclaration(C.kind)) {
+const Decl *D = getCursorDecl(C);
+if

[PATCH] D44480: [Sema] Don't skip function bodies with 'auto' without trailing return type

2018-05-30 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

I've stumbled about this bug too and was looking into it and then I saw the 
mail about this change being submitted :)

I've ended up with a slightly different change (https://dpaste.de/PSpM/raw , 
instead of FD->getReturnType()->getContainedDeducedType() I have 
FD->getReturnType()->getAs()). It looks like the tests from this 
change pass with my change and my tests pass with this change (consider to add 
decltype(auto) tests, too!). So I'm wondering whether there is a test case that 
fails for one change but not the other (if there is, the test case should 
probably be added), or if there are differences in the performance.

I'm not familiar enough with the code base to justify the getAs(), 
I've just observed that it made my tests pass.


Repository:
  rL LLVM

https://reviews.llvm.org/D44480



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

[PATCH] D44480: [Sema] Don't skip function bodies with 'auto' without trailing return type

2018-05-30 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

In https://reviews.llvm.org/D44480#1117147, @cpplearner wrote:

> Does `getAs()` work correctly with function returning `auto&`?


No.  For

  class Foo {} foo;
  auto& return_auto_ref() { return foo; }
  auto r6 = return_auto_ref();

the "getAs()" version will skip the function body and generate an 
error message on use, but "FD->getReturnType()->getContainedDeducedType()" 
works fine (will not skip the body, as it should here). OK, so now I have a 
rough idea what the "Contained" was referring too :)


Repository:
  rL LLVM

https://reviews.llvm.org/D44480



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

[PATCH] D46862: [libclang] Optionally add code completion results for arrow instead of dot

2018-06-06 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik requested changes to this revision.
nik added inline comments.
This revision now requires changes to proceed.



Comment at: include/clang-c/Index.h:5264
+ * FixIts that *must* be applied before inserting the text for the
+ * corresponding completion item.
+ *

yvvan wrote:
> nik wrote:
> > * Please use proper documentation style, e.g. make use of "\brief", 
> > "\param", "\returns". Currently one has to somewhat guess that the returned 
> > string is related to the replacement_range parameter.
> > 
> > * Is this a behavior change now? What happens to old code not aware of this 
> > function?
> '\brief' is not used anymore.
> 
> > Is this a behavior change now?
> 
> No, without CXCodeComplete_IncludeFixIts set you won't event have extra 
> completions. And existing ones won't get any fix-its (because why would they?)
Still \param and \returns are useful since there the range is an output 
argument.



Comment at: include/clang-c/Index.h:4747
+ * Tokenize the source code starting with the given location into raw lexical
+ * token.
+ *

"Tokenize the source code" sounds as if more than one token will be handled.

Suggestion: Reduce to "Get the raw lexical token starting with the given 
location".



Comment at: include/clang-c/Index.h:5256
 /**
+ * Retrieve the number of fix-its for the given completion index.
+ */

Indicate that calling this makes only sense if CXCodeComplete_IncludeFixIts was 
set.



Comment at: include/clang-c/Index.h:5266
+ *
+ * Only completion items with empty fixits will be returned by default.
+ * Extra completion items with non-empty fixits should be explicitly requested

Clarify that you refer to clang_codeCompleteAt here.

Suggestion: By default, clang_codeCompleteAt()



Comment at: include/clang-c/Index.h:5268
+ * Extra completion items with non-empty fixits should be explicitly requested
+ * by setting CXCodeComplete_IncludeFixIts. For the clients to be able to
+ * compute position of the cursor for the completion item itself, the following

Add empty line before "For the client..." for readability.



Comment at: include/clang-c/Index.h:5269
+ * by setting CXCodeComplete_IncludeFixIts. For the clients to be able to
+ * compute position of the cursor for the completion item itself, the following
+ * conditions are guaranteed to hold for replacement_range of the stored 
fixits:

...of the cursor after applying the fixits



Comment at: include/clang-c/Index.h:5277
+ *  least one character. It allows to unambiguously recompute completion
+ *  point after applying the fixit.
+ * The intuition is that provided fixits change code around the identifier we

new line



Comment at: include/clang-c/Index.h:5281
+ * completion point. One example of completion items with corrections are the
+ * ones replacing '.' with '->' and vice versa:
+ * std::unique_ptr> vec_ptr;

new line



Comment at: include/clang-c/Index.h:5283
+ * std::unique_ptr> vec_ptr;
+ * In 'vec_ptr.^', one of completion items is 'push_back', it requires
+ * replacing '.' with '->'.

"one of the ...", same below.



Comment at: include/clang-c/Index.h:5329
+   */
+  CXCodeComplete_IncludeFixIts = 0x10
 };

Suggestion: CXCodeComplete_IncludeCompletionsWithFixIts 


https://reviews.llvm.org/D46862



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

[PATCH] D46862: [libclang] Optionally add code completion results for arrow instead of dot

2018-06-06 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

- Sometimes you refer to "fixits", sometimes "fix-its" and some times "FixIts". 
Unify to what is already there.
- The term "completion items" is new so far. Use "completions" for consistency.


https://reviews.llvm.org/D46862



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

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

2018-06-13 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik created this revision.
Herald added a subscriber: cfe-commits.

Depending on the included files and the used warning flags, e.g. -
Weverything, a huge number of warnings can be reported for included
files. As processing that many diagnostics comes with a performance
impact and not all clients are interested in those diagnostics, add a
flag to skip them.


Repository:
  rC Clang

https://reviews.llvm.org/D48116

Files:
  include/clang-c/Index.h
  include/clang/Basic/Diagnostic.h
  lib/Basic/DiagnosticIDs.cpp
  test/Index/ignore-warnings-from-headers.cpp
  test/Index/ignore-warnings-from-headers.h
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3397,6 +3397,9 @@
   if (options & CXTranslationUnit_KeepGoing)
 Diags->setSuppressAfterFatalError(false);
 
+  if (options & CXTranslationUnit_IgnoreWarningsFromIncludedFiles)
+  Diags->setSuppressWarningsFromIncludedFiles(true);
+
   // Recover resources if we crash before exiting this function.
   llvm::CrashRecoveryContextCleanupRegistrar >
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -84,6 +84,8 @@
 options |= CXTranslationUnit_KeepGoing;
   if (getenv("CINDEXTEST_LIMIT_SKIP_FUNCTION_BODIES_TO_PREAMBLE"))
 options |= CXTranslationUnit_LimitSkipFunctionBodiesToPreamble;
+  if (getenv("CINDEXTEST_IGNORE_WARNINGS_FROM_INCLUDED_FILES"))
+options |= CXTranslationUnit_IgnoreWarningsFromIncludedFiles;
 
   return options;
 }
Index: test/Index/ignore-warnings-from-headers.h
===
--- /dev/null
+++ test/Index/ignore-warnings-from-headers.h
@@ -0,0 +1 @@
+void f(int unusedInHeader) {}
Index: test/Index/ignore-warnings-from-headers.cpp
===
--- /dev/null
+++ test/Index/ignore-warnings-from-headers.cpp
@@ -0,0 +1,7 @@
+#include "ignore-warnings-from-headers.h"
+
+void g(int unusedInMainFile) {}
+
+// RUN: env CINDEXTEST_IGNORE_WARNINGS_FROM_INCLUDED_FILES=1 c-index-test -test-load-source function %s -Wunused-parameter 2>&1 | FileCheck %s
+// CHECK-NOT: warning: unused parameter 'unusedInHeader'
+// CHECK: warning: unused parameter 'unusedInMainFile'
Index: lib/Basic/DiagnosticIDs.cpp
===
--- lib/Basic/DiagnosticIDs.cpp
+++ lib/Basic/DiagnosticIDs.cpp
@@ -477,6 +477,13 @@
   Result = diag::Severity::Fatal;
   }
 
+  // If requested, ignore warnings from all headers.
+  if (Diag.SuppressWarningsFromIncludedFiles && Result <= diag::Severity::Warning &&
+  Loc.isValid() &&
+  !Diag.getSourceManager().isInMainFile(
+  Diag.getSourceManager().getExpansionLoc(Loc)))
+return diag::Severity::Ignored;
+
   // Custom diagnostics always are emitted in system headers.
   bool ShowInSystemHeader =
   !GetDiagInfo(DiagID) || GetDiagInfo(DiagID)->WarnShowInSystemHeader;
Index: include/clang/Basic/Diagnostic.h
===
--- include/clang/Basic/Diagnostic.h
+++ include/clang/Basic/Diagnostic.h
@@ -213,6 +213,9 @@
   // Suppress all diagnostics.
   bool SuppressAllDiagnostics = false;
 
+  // Suppress warnings from all included files.
+  bool SuppressWarningsFromIncludedFiles = false;
+
   // Elide common types of templates.
   bool ElideType = true;
 
@@ -634,6 +637,10 @@
   }
   bool getSuppressAllDiagnostics() const { return SuppressAllDiagnostics; }
 
+  void setSuppressWarningsFromIncludedFiles(bool Val = true) {
+SuppressWarningsFromIncludedFiles = Val;
+  }
+
   /// Set type eliding, to skip outputting same types occurring in
   /// template types.
   void setElideType(bool Val = true) { ElideType = Val; }
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -1332,7 +1332,17 @@
*
* The function bodies of the main file are not skipped.
*/
-  CXTranslationUnit_LimitSkipFunctionBodiesToPreamble = 0x800
+  CXTranslationUnit_LimitSkipFunctionBodiesToPreamble = 0x800,
+
+  /**
+   * Used to indicate that warnings from included files should be ignored.
+   *
+   * If set, clang_getDiagnosticSetFromTU() will not report warnings from
+   * included files anymore. This speeds up clang_getDiagnosticSetFromTU() for
+   * the case where these warnings are not of interest, as for an IDE for
+   * example, which typically shows only the diagnostics in the main file.
+   */
+  CXTranslationUnit_IgnoreWarningsFromIncludedFiles = 0x1000
 };
 
 /**
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bi

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

2018-06-13 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 151116.
nik added a comment.

Addressed comments.


Repository:
  rC Clang

https://reviews.llvm.org/D48116

Files:
  include/clang-c/Index.h
  include/clang/Basic/Diagnostic.h
  lib/Basic/DiagnosticIDs.cpp
  test/Index/ignore-warnings-from-headers.cpp
  test/Index/ignore-warnings-from-headers.h
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3397,6 +3397,9 @@
   if (options & CXTranslationUnit_KeepGoing)
 Diags->setSuppressAfterFatalError(false);
 
+  if (options & CXTranslationUnit_IgnoreNonErrorsFromIncludedFiles)
+  Diags->setSuppressNonErrorsFromIncludedFiles(true);
+
   // Recover resources if we crash before exiting this function.
   llvm::CrashRecoveryContextCleanupRegistrar >
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -84,6 +84,8 @@
 options |= CXTranslationUnit_KeepGoing;
   if (getenv("CINDEXTEST_LIMIT_SKIP_FUNCTION_BODIES_TO_PREAMBLE"))
 options |= CXTranslationUnit_LimitSkipFunctionBodiesToPreamble;
+  if (getenv("CINDEXTEST_IGNORE_NONERRORS_FROM_INCLUDED_FILES"))
+options |= CXTranslationUnit_IgnoreNonErrorsFromIncludedFiles;
 
   return options;
 }
Index: test/Index/ignore-warnings-from-headers.h
===
--- /dev/null
+++ test/Index/ignore-warnings-from-headers.h
@@ -0,0 +1 @@
+void f(int unusedInHeader) {}
Index: test/Index/ignore-warnings-from-headers.cpp
===
--- /dev/null
+++ test/Index/ignore-warnings-from-headers.cpp
@@ -0,0 +1,7 @@
+#include "ignore-warnings-from-headers.h"
+
+void g(int unusedInMainFile) {}
+
+// RUN: env CINDEXTEST_IGNORE_NONERRORS_FROM_INCLUDED_FILES=1 c-index-test -test-load-source function %s -Wunused-parameter 2>&1 | FileCheck %s
+// CHECK-NOT: warning: unused parameter 'unusedInHeader'
+// CHECK: warning: unused parameter 'unusedInMainFile'
Index: lib/Basic/DiagnosticIDs.cpp
===
--- lib/Basic/DiagnosticIDs.cpp
+++ lib/Basic/DiagnosticIDs.cpp
@@ -477,6 +477,14 @@
   Result = diag::Severity::Fatal;
   }
 
+  // If requested, ignore non-errors from all included files.
+  if (Diag.SuppressNonErrorsFromIncludedFiles &&
+  Result <= diag::Severity::Warning && Loc.isValid() &&
+  !Diag.getSourceManager().isInMainFile(
+  Diag.getSourceManager().getExpansionLoc(Loc))) {
+return diag::Severity::Ignored;
+  }
+
   // Custom diagnostics always are emitted in system headers.
   bool ShowInSystemHeader =
   !GetDiagInfo(DiagID) || GetDiagInfo(DiagID)->WarnShowInSystemHeader;
Index: include/clang/Basic/Diagnostic.h
===
--- include/clang/Basic/Diagnostic.h
+++ include/clang/Basic/Diagnostic.h
@@ -213,6 +213,9 @@
   // Suppress all diagnostics.
   bool SuppressAllDiagnostics = false;
 
+  // Suppress non-errors from all included files.
+  bool SuppressNonErrorsFromIncludedFiles = false;
+
   // Elide common types of templates.
   bool ElideType = true;
 
@@ -634,6 +637,10 @@
   }
   bool getSuppressAllDiagnostics() const { return SuppressAllDiagnostics; }
 
+  void setSuppressNonErrorsFromIncludedFiles(bool Val = true) {
+SuppressNonErrorsFromIncludedFiles = Val;
+  }
+
   /// Set type eliding, to skip outputting same types occurring in
   /// template types.
   void setElideType(bool Val = true) { ElideType = Val; }
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -1332,7 +1332,17 @@
*
* The function bodies of the main file are not skipped.
*/
-  CXTranslationUnit_LimitSkipFunctionBodiesToPreamble = 0x800
+  CXTranslationUnit_LimitSkipFunctionBodiesToPreamble = 0x800,
+
+  /**
+   * Used to indicate that non-errors from included files should be ignored.
+   *
+   * If set, clang_getDiagnosticSetFromTU() will not report e.g. warnings from
+   * included files anymore. This speeds up clang_getDiagnosticSetFromTU() for
+   * the case where these warnings are not of interest, as for an IDE for
+   * example, which typically shows only the diagnostics in the main file.
+   */
+  CXTranslationUnit_IgnoreNonErrorsFromIncludedFiles = 0x1000
 };
 
 /**
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46862: [libclang] Optionally add code completion results for arrow instead of dot

2018-06-13 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added inline comments.



Comment at: include/clang-c/Index.h:5302
+CXCodeCompleteResults *results, unsigned completion_index,
+unsigned fixit_index, CXSourceRange *replacement_range);
+

Please document the parameters. It's more important here than for any other 
function that was introduced.



Comment at: include/clang-c/Index.h:5338
+  /**
+   * Whether to include completion items with small
+   * fix-its, e.g. change '.' to '->' on member access, etc.

s/completion items/completions/g


https://reviews.llvm.org/D46862



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

[PATCH] D53866: [Preamble] Fix preamble for circular #includes

2018-10-30 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik created this revision.
Herald added subscribers: cfe-commits, arphaman.

If a header file was processed for the second time, we could end up with
a wrong conditional stack and skipped ranges:

In the particular example, if the header guard is evaluated the second
time and it is decided to skip the conditional block, the corresponding
"#endif" is never seen since the preamble does not include it and we end
up in the Tok.is(tok::eof) case with a wrong conditional stack.

Fix this by resetting the conditional state in such a case.


Repository:
  rC Clang

https://reviews.llvm.org/D53866

Files:
  lib/Lex/PPDirectives.cpp
  test/Index/preamble-cyclic-include.cpp


Index: test/Index/preamble-cyclic-include.cpp
===
--- /dev/null
+++ test/Index/preamble-cyclic-include.cpp
@@ -0,0 +1,8 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-annotate-tokens=%s:4:1:8:1 
%s 2>&1 | FileCheck %s
+// CHECK-NOT: error: unterminated conditional directive
+// CHECK-NOT: Skipping: [4:1 - 8:7]
+#ifndef A_H
+#define A_H
+#  include "preamble-cyclic-include.cpp"
+int bar();
+#endif
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -361,6 +361,14 @@
   }
 }
 
+static bool isMainFileIncludedAgain(const SourceManager &sourceManager,
+HeaderSearch &headerSearch,
+const FileEntry *fileEntry) {
+  return sourceManager.translateFile(fileEntry) ==
+ sourceManager.getMainFileID() &&
+ headerSearch.getFileInfo(fileEntry).NumIncludes > 1;
+}
+
 /// SkipExcludedConditionalBlock - We just read a \#if or related directive and
 /// decided that the subsequent tokens are in the \#if'd out portion of the
 /// file.  Lex the rest of the file, until we see an \#endif.  If
@@ -377,6 +385,8 @@
   ++NumSkipped;
   assert(!CurTokenLexer && CurPPLexer && "Lexing a macro, not a file?");
 
+  const auto InitialConditionalStack = CurPPLexer->ConditionalStack;
+
   if (PreambleConditionalStack.reachedEOFWhileSkipping())
 PreambleConditionalStack.clearSkipInfo();
   else
@@ -407,9 +417,16 @@
   // 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.
-  if (PreambleConditionalStack.isRecording())
-PreambleConditionalStack.SkipInfo.emplace(
-HashTokenLoc, IfTokenLoc, FoundNonSkipPortion, FoundElse, ElseLoc);
+  if (PreambleConditionalStack.isRecording()) {
+if (isMainFileIncludedAgain(getSourceManager(), HeaderInfo,
+CurLexer->getFileEntry())) {
+  CurPPLexer->ConditionalStack = InitialConditionalStack;
+} else {
+  PreambleConditionalStack.SkipInfo.emplace(HashTokenLoc, IfTokenLoc,
+FoundNonSkipPortion,
+FoundElse, ElseLoc);
+}
+  }
   break;
 }
 


Index: test/Index/preamble-cyclic-include.cpp
===
--- /dev/null
+++ test/Index/preamble-cyclic-include.cpp
@@ -0,0 +1,8 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-annotate-tokens=%s:4:1:8:1 %s 2>&1 | FileCheck %s
+// CHECK-NOT: error: unterminated conditional directive
+// CHECK-NOT: Skipping: [4:1 - 8:7]
+#ifndef A_H
+#define A_H
+#  include "preamble-cyclic-include.cpp"
+int bar();
+#endif
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -361,6 +361,14 @@
   }
 }
 
+static bool isMainFileIncludedAgain(const SourceManager &sourceManager,
+HeaderSearch &headerSearch,
+const FileEntry *fileEntry) {
+  return sourceManager.translateFile(fileEntry) ==
+ sourceManager.getMainFileID() &&
+ headerSearch.getFileInfo(fileEntry).NumIncludes > 1;
+}
+
 /// SkipExcludedConditionalBlock - We just read a \#if or related directive and
 /// decided that the subsequent tokens are in the \#if'd out portion of the
 /// file.  Lex the rest of the file, until we see an \#endif.  If
@@ -377,6 +385,8 @@
   ++NumSkipped;
   assert(!CurTokenLexer && CurPPLexer && "Lexing a macro, not a file?");
 
+  const auto InitialConditionalStack = CurPPLexer->ConditionalStack;
+
   if (PreambleConditionalStack.reachedEOFWhileSkipping())
 PreambleConditionalStack.clearSkipInfo();
   else
@@ -407,9 +417,16 @@
   // 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.
-  if (PreambleConditionalStack.isRecording())
-Preambl

[PATCH] D53866: [Preamble] Fix preamble for circular #includes

2018-10-31 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

In https://reviews.llvm.org/D53866#1281978, @ilya-biryukov wrote:

> Why does resetting the conditional stack is the right thing to do here?

Because this case can be detected and handled without loosing the benefits of 
the preamble.

> Failing to build the preamble in that case seems like the best thing we could 
> do. WDYT?

See above - we would not have the benefits of the preamble anymore.

>   I can see how it can hide the problem, but can't come up with with a 
> consistent model to handle the fact that the file contents were trimmed.

If we are in preamble-generation-mode and the main file #includes itself, then 
it should see the full file content of the file, not the truncated preamble 
version of it.
Would this be more consistent?

Repository:
  rC Clang

https://reviews.llvm.org/D53866

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

[PATCH] D53866: [Preamble] Fix preamble for circular #includes

2018-11-01 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

I've only the minimal example at hand right know - I'm waiting for feedback 
about the real world case.


Repository:
  rC Clang

https://reviews.llvm.org/D53866



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

[PATCH] D54077: [clangd] Implemented DraftFileSystem

2018-11-06 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

In https://reviews.llvm.org/D54077#1288404, @ioeric wrote:

> I would be very happy if `A.cc` can see the unsaved `A.h` when I am editing 
> `A.cc`.

We have that in Qt Creator (with libclang) and that's quite handy as it can 
save you some build cycles.

> Not sure if I want all files that depend on `A.h` to be updated when I edit 
> `A.h` though; it seems much more complicated and less important (to me).

If it helps (and the LSP allows it): In case A.h is edited, we flag it dirty. 
If the user makes some file depending on it visible (or the current file), we 
trigger a reparse for that. Not sure whether the LSP has a notion of current or 
visible files...

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54077

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

[PATCH] D54077: [clangd] Implemented DraftFileSystem

2018-11-06 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

In https://reviews.llvm.org/D54077#1288413, @nik wrote:

> If it helps (and the LSP allows it): In case A.h is edited, we flag it dirty. 
> If the user makes some file depending on it visible (or the current file), we 
> trigger a reparse for that. Not sure whether the LSP has a notion of current 
> or visible files...


Huch..., I meant that we flag the files dirty that depend on A.h.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54077



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

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2018-11-06 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Coming back to this one, I see a failing test: 
PCHPreambleTest.ReparseWithOverriddenFileDoesNotInvalidatePreamble

Note that PCHPreambleTest.ReparseWithOverriddenFileDoesNotInvalidatePreamble 
references the header paths in different ways ("//./header1.h" vs 
"//./foo/../header1.h"). Before this change, this was not a problem because 
OverriddenFiles were keyed on Status.getUniqueID(). Starting with this change, 
the key is the file path.

Is there a nice way to map different file paths of the same file to the same id 
without touching the real file system? Would it be sufficient to normalize the 
file paths? If yes, is there a utility function for this (can't find it right 
now).




Comment at: include/clang/Frontend/ASTUnit.h:193
   /// some number of calls.
-  unsigned PreambleRebuildCounter;
+  unsigned PreambleRebuildCountdownCounter;
+

ilya-biryukov wrote:
> NIT: Maybe shorten to `PreambleRebuildCountdown`?
> It's not a great name, but a bit shorter than now and seems to convey the 
> same meaning.
Will do.



Comment at: unittests/Frontend/PCHPreambleTest.cpp:130
+  std::string MainName = "//./main.cpp";
+  AddFile(MainName, "#include \"//./header1.h\"\n"
+"int main() { return ZERO; }");

ilya-biryukov wrote:
> NIT: Maybe use raw string literals? Escpated string are hard to read.
> E.g., 
> 
> ```R"cpp(
> #include "//./header1.h"
> int main() { return ZERO; }
> )cpp"
> ```
Will do.



Comment at: unittests/Frontend/PCHPreambleTest.cpp:133
+
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());

ilya-biryukov wrote:
> Are we testing the right thing here?
> 
> We're testing that preamble does not get rebuild when some header that was 
> previously unresolved when seen inside `#include` directive is now added to 
> the filesystem. That is actually a bug, we should rebuild the preamble in 
> that case.
> 
> We should probably call `RemapFile` before calling `ParseAST` instead and 
> make sure it's properly resolved.
> ```
>   AddFile(MainName, ...);
>   // We don't call AddFile for the header at this point, so that it does not 
> exist on the filesystem.
>   RemapFile(Header1, "#define ZERO 0\n");
> 
>   std::unique_ptr AST(ParseAST(MainName));
>   // Also assert that there were no compiler errors? (I.e. file was resolved 
> properly)
>   //  
> 
>   // Now the file is on the filesystem, call reparse and check that we reused 
> the preamble.
>   AddFile(Header1, "#define ZERO 0\n");
>   ASSERT_TRUE(ReparseAST(AST));
>   ASSERT_EQ(AST->getPreambleCounter(), 1U);
> ```
> Are we testing the right thing here?
Huch, indeed, the test is wrong. 

I'll incorporate your suggested test (real file is added after providing 
unsaved file) and add also another one: real file is removed after providing an 
unsaved file.

> That is actually a bug, we should rebuild the preamble in that case.
Agree, the preamble should be rebuild in such a case, but that's something for 
a separate change.



Repository:
  rC Clang

https://reviews.llvm.org/D41005



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

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2018-11-12 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping.


Repository:
  rC Clang

https://reviews.llvm.org/D41005



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

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2018-11-14 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

In https://reviews.llvm.org/D41005#1295632, @ilya-biryukov wrote:

> > Before this change, this was not a problem because OverriddenFiles were 
> > keyed on Status.getUniqueID(). Starting with this change, the key is the 
> > file path.
>
> I suggest keeping two maps for overridden files: one for existing files (key 
> is UniqueID), another one for the ones that don't exist (key is the file 
> path).


Done.

>> Is there a nice way to map different file paths of the same file to the same 
>> id without touching the real file system? Would it be sufficient to 
>> normalize the file paths? If yes, is there a utility function for this 
>> (can't find it right now).
> 
> I don't think there is one, the unique ids are closely tied to the 
> filesystem. IIUC the unique ids are the same whenever two different paths are 
> symlinks (or hardlinks?) pointing to the same physical file and there's no 
> way to tell if they're the same without resolving the symlink.

OK, so if the unsaved file is not being backed up by a real file on disk and 
symlinks are used, we can't do much about it.
I've normalized the file paths to handle at least that case where they might 
differ.


Repository:
  rC Clang

https://reviews.llvm.org/D41005



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

[PATCH] D54523: [Preamble] Reuse preamble even if an unsaved file does not exist

2018-11-14 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik created this revision.
Herald added a subscriber: cfe-commits.

When a preamble is created an unsaved file not existing on disk is
already part of PrecompiledPreamble::FilesInPreamble. However, when
checking whether the preamble can be re-used, a failed stat of such an
unsaved file invalidated the preamble, which led to pointless and time
consuming preamble regenerations on subsequent reparses.

Do not require anymore that unsaved files should exist on disk.

This avoids costly preamble invalidations depending on timing issues for
the cases where the file on disk might be removed just to be regenerated
a bit later.

It also allows an IDE to provide in-memory files that might not exist on
disk, e.g. because the build system hasn't generated those yet.


Repository:
  rC Clang

https://reviews.llvm.org/D54523

Files:
  include/clang/Frontend/ASTUnit.h
  lib/Frontend/ASTUnit.cpp
  lib/Frontend/PrecompiledPreamble.cpp
  unittests/Frontend/PCHPreambleTest.cpp

Index: unittests/Frontend/PCHPreambleTest.cpp
===
--- unittests/Frontend/PCHPreambleTest.cpp
+++ unittests/Frontend/PCHPreambleTest.cpp
@@ -53,7 +53,10 @@
   FileSystemOptions FSOpts;
 
 public:
-  void SetUp() override {
+  void SetUp() override { ResetVFS(); }
+  void TearDown() override {}
+
+  void ResetVFS() {
 VFS = new ReadCountingInMemoryFileSystem();
 // We need the working directory to be set to something absolute,
 // otherwise it ends up being inadvertently set to the current
@@ -64,9 +67,6 @@
 VFS->setCurrentWorkingDirectory("//./");
   }
 
-  void TearDown() override {
-  }
-
   void AddFile(const std::string &Filename, const std::string &Contents) {
 ::time_t now;
 ::time(&now);
@@ -124,6 +124,72 @@
   }
 };
 
+TEST_F(PCHPreambleTest, ReparseReusesPreambleWithUnsavedFileNotExistingOnDisk) {
+  std::string Header1 = "//./header1.h";
+  std::string MainName = "//./main.cpp";
+  AddFile(MainName, R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which does not exist on
+  // disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  // Reparse and check that the preamble was reused.
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounter(), 1U);
+}
+
+TEST_F(PCHPreambleTest, ReparseReusesPreambleAfterUnsavedFileWasCreatedOnDisk) {
+  std::string Header1 = "//./header1.h";
+  std::string MainName = "//./main.cpp";
+  AddFile(MainName, R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which does not exist on
+  // disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  // Create the unsaved file also on disk and check that preamble was reused.
+  AddFile(Header1, "#define ZERO 0\n");
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounter(), 1U);
+}
+
+TEST_F(PCHPreambleTest,
+   ReparseReusesPreambleAfterUnsavedFileWasRemovedFromDisk) {
+  std::string Header1 = "//./foo/../header1.h";
+  std::string MainName = "//./main.cpp";
+  std::string MainFileContent = R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp";
+  AddFile(MainName, MainFileContent);
+  AddFile(Header1, "#define ZERO 0\n");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which exists on disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+  ASSERT_EQ(AST->getPreambleCounter(), 1U);
+
+  // Remove the unsaved file from disk and check that the preamble was reused.
+  ResetVFS();
+  AddFile(MainName, MainFileContent);
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounter(), 1U);
+}
+
 TEST_F(PCHPreambleTest, ReparseWithOverriddenFileDoesNotInvalidatePreamble) {
   std::string Header1 = "//./header1.h";
   std::string Header2 = "//./header2.h";
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -28,6 +28,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Mutex.h"
 #include "llvm/Support/MutexGuard.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include 
@@ -221,6 +222,14 @@
   return true;
 }
 
+template 
+llvm::SmallString<256> PathNormalized(IteratorType Start, IteratorType End) {
+  llvm::SmallString<256> Path{Start, End};
+  llvm::sys::path::remove_dots(Path, /*remove_dot_dots=*/true);
+  Path = llvm::sys::path::convert_to_slash(Path);
+  return Path;
+}
+
 } // na

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2018-11-14 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 174022.
nik added a comment.

Addressed comments.


Repository:
  rC Clang

https://reviews.llvm.org/D41005

Files:
  include/clang/Frontend/ASTUnit.h
  lib/Frontend/ASTUnit.cpp
  lib/Frontend/PrecompiledPreamble.cpp
  unittests/Frontend/PCHPreambleTest.cpp

Index: unittests/Frontend/PCHPreambleTest.cpp
===
--- unittests/Frontend/PCHPreambleTest.cpp
+++ unittests/Frontend/PCHPreambleTest.cpp
@@ -53,7 +53,10 @@
   FileSystemOptions FSOpts;
 
 public:
-  void SetUp() override {
+  void SetUp() override { ResetVFS(); }
+  void TearDown() override {}
+
+  void ResetVFS() {
 VFS = new ReadCountingInMemoryFileSystem();
 // We need the working directory to be set to something absolute,
 // otherwise it ends up being inadvertently set to the current
@@ -64,9 +67,6 @@
 VFS->setCurrentWorkingDirectory("//./");
   }
 
-  void TearDown() override {
-  }
-
   void AddFile(const std::string &Filename, const std::string &Contents) {
 ::time_t now;
 ::time(&now);
@@ -124,6 +124,72 @@
   }
 };
 
+TEST_F(PCHPreambleTest, ReparseReusesPreambleWithUnsavedFileNotExistingOnDisk) {
+  std::string Header1 = "//./header1.h";
+  std::string MainName = "//./main.cpp";
+  AddFile(MainName, R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which does not exist on
+  // disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  // Reparse and check that the preamble was reused.
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounter(), 1U);
+}
+
+TEST_F(PCHPreambleTest, ReparseReusesPreambleAfterUnsavedFileWasCreatedOnDisk) {
+  std::string Header1 = "//./header1.h";
+  std::string MainName = "//./main.cpp";
+  AddFile(MainName, R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which does not exist on
+  // disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  // Create the unsaved file also on disk and check that preamble was reused.
+  AddFile(Header1, "#define ZERO 0\n");
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounter(), 1U);
+}
+
+TEST_F(PCHPreambleTest,
+   ReparseReusesPreambleAfterUnsavedFileWasRemovedFromDisk) {
+  std::string Header1 = "//./foo/../header1.h";
+  std::string MainName = "//./main.cpp";
+  std::string MainFileContent = R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp";
+  AddFile(MainName, MainFileContent);
+  AddFile(Header1, "#define ZERO 0\n");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which exists on disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+  ASSERT_EQ(AST->getPreambleCounter(), 1U);
+
+  // Remove the unsaved file from disk and check that the preamble was reused.
+  ResetVFS();
+  AddFile(MainName, MainFileContent);
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounter(), 1U);
+}
+
 TEST_F(PCHPreambleTest, ReparseWithOverriddenFileDoesNotInvalidatePreamble) {
   std::string Header1 = "//./header1.h";
   std::string Header2 = "//./header2.h";
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -28,6 +28,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Mutex.h"
 #include "llvm/Support/MutexGuard.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include 
@@ -221,6 +222,14 @@
   return true;
 }
 
+template 
+llvm::SmallString<256> PathNormalized(IteratorType Start, IteratorType End) {
+  llvm::SmallString<256> Path{Start, End};
+  llvm::sys::path::remove_dots(Path, /*remove_dot_dots=*/true);
+  Path = llvm::sys::path::convert_to_slash(Path);
+  return Path;
+}
+
 } // namespace
 
 PreambleBounds clang::ComputePreambleBounds(const LangOptions &LangOpts,
@@ -367,13 +376,15 @@
 const FileEntry *File = Clang->getFileManager().getFile(Filename);
 if (!File || File == SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()))
   continue;
+auto FilePath =
+PathNormalized(File->getName().begin(), File->getName().end());
 if (time_t ModTime = File->getModificationTime()) {
-  FilesInPreamble[File->getName()] =
+  FilesInPreamble[FilePath] =
   PrecompiledPreamble::PreambleFileHash::createForFile(File->getSize(),
ModTime);
 } else {

[PATCH] D53866: [Preamble] Fix preamble for circular #includes

2018-11-16 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

I still don't have feedback for a real world case except "unintentional 
#include". Unfortunately, in real world cases the cyclic include might be not 
obvious at all.

@ilya: As far as I understand you prefer to make the preamble generation rather 
fail as long as we don't have more information. How do you suggest to implement 
this? I see that Clang->getPreprocessor().addPPCallbacks() is called in 
PrecompiledPreamble::Build(). Adding a custom PPCallbacks there that overrides 
"void InclusionDirective()" might be enough to detect the cyclic #include and 
to set a flag that is checked before PrecompiledPreamble::Build() returns - 
BuildPreambleError could get a new enumerator. Is there a better way to do 
this? For example, it would be desirable to not only observe this case, but 
then to also stop/abort/skip all the further parsing that is done for the 
preamble only as it's pointless then.


Repository:
  rC Clang

https://reviews.llvm.org/D53866



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

[PATCH] D53866: [Preamble] Fix preamble for circular #includes

2018-11-26 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 175210.
nik added a comment.

> Maybe produce a **fatal** error in the preprocessor? That seems to be the 
> simplest option: the preprocessor is aware it's building the preamble and 
> there's definitely some logic to produce fatal errors in other cases (include 
> not found).
>  A fatal error currently aborts other stages of the compilation pipeline and 
> producing one would make the run of the compiler that tries to produce the 
> preamble fail, giving the empty preamble as a result.

Done. I'm using diag::err_pp_error_opening_file as introducing an new 
artificial diagnostic error that the user will never see feels wrong.

Note that a preamble is generated for fatal errors like an unresolved #include 
and I think that's fine. As such, I need a way to propagate the error up to 
PrecompilePreambleConsumer to avoid writing the preamble. I've done that with 
an extra flag in Preprocessor. An alternative might be to put it into 
PreambleConditionalStackStore (as state? and rename that class to something 
more general?) - what do you think?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866

Files:
  include/clang/Lex/Preprocessor.h
  include/clang/Serialization/ASTWriter.h
  lib/Frontend/PrecompiledPreamble.cpp
  lib/Lex/PPDirectives.cpp
  test/Index/preamble-cyclic-include.cpp


Index: test/Index/preamble-cyclic-include.cpp
===
--- /dev/null
+++ test/Index/preamble-cyclic-include.cpp
@@ -0,0 +1,9 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-annotate-tokens=%s:4:1:8:1 
%s 2>&1 | FileCheck %s
+// CHECK-NOT: error: unterminated conditional directive
+// CHECK-NOT: Skipping: [4:1 - 8:7]
+#ifndef A_H
+#define A_H
+#  include "preamble-cyclic-include.cpp"
+int bar();
+#endif
+
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1933,6 +1933,20 @@
 return;
   }
 
+  // Check whether it makes sense to continue preamble generation. We can't
+  // generate a consistent preamble with regard to the conditional stack if the
+  // main file is included again as due to the preamble bounds some directives
+  // (e.g. #endif of a header guard) will never be seen. Since this will lead 
to
+  // confusing errors, abort the preamble generation.
+  if (File && PreambleConditionalStack.isRecording() &&
+  SourceMgr.translateFile(File) == SourceMgr.getMainFileID()) {
+PreambleGenerationFailed = true;
+// Generate a fatal error to skip further processing.
+Diag(FilenameTok.getLocation(), diag::err_pp_error_opening_file) << ""
+ << "";
+return;
+  }
+
   // Should we enter the source file? Set to false if either the source file is
   // known to have no effect beyond its effect on module visibility -- that is,
   // if it's got an include guard that is already defined or is a modular 
header
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -170,6 +170,9 @@
   }
 
   void HandleTranslationUnit(ASTContext &Ctx) override {
+if (getPreprocessor().preambleGenerationFailed())
+  return;
+
 PCHGenerator::HandleTranslationUnit(Ctx);
 if (!hasEmittedPCH())
   return;
Index: include/clang/Serialization/ASTWriter.h
===
--- include/clang/Serialization/ASTWriter.h
+++ include/clang/Serialization/ASTWriter.h
@@ -979,6 +979,7 @@
   ASTWriter &getWriter() { return Writer; }
   const ASTWriter &getWriter() const { return Writer; }
   SmallVectorImpl &getPCH() const { return Buffer->Data; }
+  const Preprocessor &getPreprocessor() const { return PP; }
 
 public:
   PCHGenerator(const Preprocessor &PP, StringRef OutputFile, StringRef 
isysroot,
Index: include/clang/Lex/Preprocessor.h
===
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -388,6 +388,7 @@
 SmallVector ConditionalStack;
 State ConditionalStackState = Off;
   } PreambleConditionalStack;
+  bool PreambleGenerationFailed = false;
 
   /// The current top of the stack that we're lexing from if
   /// not expanding a macro and we are lexing directly from source code.
@@ -2159,6 +2160,10 @@
   Module *M,
   SourceLocation MLoc);
 
+  bool preambleGenerationFailed() const {
+return PreambleGenerationFailed;
+  }
+
   bool isRecordingPreamble() const {
 return PreambleConditionalStack.isRecording();
   }


Index: test/Index/preamble-cyclic-include.cpp
==

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2018-11-26 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41005/new/

https://reviews.llvm.org/D41005



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

[PATCH] D55139: [clangd] Avoid memory-mapping files on Windows

2018-12-03 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Please fix this also for libclang clients. I think it's safe to assume that 
files might be edited once CXTranslationUnit_PrecompiledPreamble or 
CXTranslationUnit_CacheCompletionResults is set as flag - that's what 
clang_defaultEditingTranslationUnitOptions() returns.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55139/new/

https://reviews.llvm.org/D55139



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

[PATCH] D53866: [Preamble] Fix preamble for circular #includes

2018-12-03 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866



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

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2018-12-03 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41005/new/

https://reviews.llvm.org/D41005



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

[PATCH] D53866: [Preamble] Fix preamble for circular #includes

2018-12-05 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 176826.
nik added a comment.

Addressed comments.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866

Files:
  include/clang/Lex/Preprocessor.h
  include/clang/Serialization/ASTWriter.h
  lib/Frontend/PrecompiledPreamble.cpp
  lib/Lex/PPDirectives.cpp
  test/Index/preamble-cyclic-include.cpp


Index: test/Index/preamble-cyclic-include.cpp
===
--- /dev/null
+++ test/Index/preamble-cyclic-include.cpp
@@ -0,0 +1,9 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-annotate-tokens=%s:4:1:8:1 
%s 2>&1 | FileCheck %s
+// CHECK-NOT: error: unterminated conditional directive
+// CHECK-NOT: Skipping: [4:1 - 8:7]
+#ifndef A_H
+#define A_H
+#  include "preamble-cyclic-include.cpp"
+int bar();
+#endif
+
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1933,6 +1933,20 @@
 return;
   }
 
+  // Check whether it makes sense to continue preamble generation. We can't
+  // generate a consistent preamble with regard to the conditional stack if the
+  // main file is included again as due to the preamble bounds some directives
+  // (e.g. #endif of a header guard) will never be seen. Since this will lead 
to
+  // confusing errors, abort the preamble generation.
+  if (File && PreambleConditionalStack.isRecording() &&
+  SourceMgr.translateFile(File) == SourceMgr.getMainFileID()) {
+PreambleGenerationFailed = true;
+// Generate a fatal error to skip further processing.
+Diag(FilenameTok.getLocation(), diag::err_pp_error_opening_file)
+<< Filename << "Main file can't include itself for preamble.";
+return;
+  }
+
   // Should we enter the source file? Set to false if either the source file is
   // known to have no effect beyond its effect on module visibility -- that is,
   // if it's got an include guard that is already defined or is a modular 
header
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -170,6 +170,9 @@
   }
 
   void HandleTranslationUnit(ASTContext &Ctx) override {
+if (getPreprocessor().preambleGenerationFailed())
+  return;
+
 PCHGenerator::HandleTranslationUnit(Ctx);
 if (!hasEmittedPCH())
   return;
Index: include/clang/Serialization/ASTWriter.h
===
--- include/clang/Serialization/ASTWriter.h
+++ include/clang/Serialization/ASTWriter.h
@@ -979,6 +979,7 @@
   ASTWriter &getWriter() { return Writer; }
   const ASTWriter &getWriter() const { return Writer; }
   SmallVectorImpl &getPCH() const { return Buffer->Data; }
+  const Preprocessor &getPreprocessor() const { return PP; }
 
 public:
   PCHGenerator(const Preprocessor &PP, StringRef OutputFile, StringRef 
isysroot,
Index: include/clang/Lex/Preprocessor.h
===
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -388,6 +388,7 @@
 SmallVector ConditionalStack;
 State ConditionalStackState = Off;
   } PreambleConditionalStack;
+  bool PreambleGenerationFailed = false;
 
   /// The current top of the stack that we're lexing from if
   /// not expanding a macro and we are lexing directly from source code.
@@ -2159,6 +2160,10 @@
   Module *M,
   SourceLocation MLoc);
 
+  bool preambleGenerationFailed() const {
+return PreambleGenerationFailed;
+  }
+
   bool isRecordingPreamble() const {
 return PreambleConditionalStack.isRecording();
   }


Index: test/Index/preamble-cyclic-include.cpp
===
--- /dev/null
+++ test/Index/preamble-cyclic-include.cpp
@@ -0,0 +1,9 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-annotate-tokens=%s:4:1:8:1 %s 2>&1 | FileCheck %s
+// CHECK-NOT: error: unterminated conditional directive
+// CHECK-NOT: Skipping: [4:1 - 8:7]
+#ifndef A_H
+#define A_H
+#  include "preamble-cyclic-include.cpp"
+int bar();
+#endif
+
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1933,6 +1933,20 @@
 return;
   }
 
+  // Check whether it makes sense to continue preamble generation. We can't
+  // generate a consistent preamble with regard to the conditional stack if the
+  // main file is included again as due to the preamble bounds some directives
+  // (e.g. #endif of a header guard) will never be seen. Since this will lead to
+  // confusing errors, abort the preamble generation.
+  if (File && PreambleConditionalS

[PATCH] D53866: [Preamble] Fix preamble for circular #includes

2018-12-05 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik marked 2 inline comments as done.
nik added inline comments.



Comment at: include/clang/Lex/Preprocessor.h:391
   } PreambleConditionalStack;
+  bool PreambleGenerationFailed = false;
 

ilya-biryukov wrote:
> There's a mechanism to handle preamble with errors, see 
> `PreprocessorOpts::AllowPCHWithCompilerErrors`.
> Maybe rely on it and avoid adding a different one?
I'm not sure how to make use of AllowPCHWithCompilerErrors. It's clearly meant 
as an input/readonly option - do you suggest setting that one to "false" in 
case we detect the cyclic include (and later checking for it...)? That feels a 
bit hacky. Have you meant something else?



Comment at: lib/Lex/PPDirectives.cpp:1945
+// Generate a fatal error to skip further processing.
+Diag(FilenameTok.getLocation(), diag::err_pp_error_opening_file) << ""
+ << "";

ilya-biryukov wrote:
> Maybe add a new error or find a more appropriate existing one to reuse?
> "Error opening file", especially without any arguments does not provide 
> enough context to figure out what went wrong.
> Maybe add a new error or find a more appropriate existing one to reuse?

As stated above, introducing a new diagnostic that the user will never face 
feels wrong, but that's just a preference. If you prefer a dedicated 
diagnostics, that's fine for me.

Checking the existing ones, there are not many fatal errors to choose from:

  err_pp_file_not_found
  err_pp_through_header_not_found
  err_pp_through_header_not_seen
  err_pp_pragma_hdrstop_not_seen
  err_pp_error_opening_file

The last one looks the best for me.

> "Error opening file", especially without any arguments does not provide 
> enough context to figure out what went wrong.

I've added some arguments which might be useful for debugging.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866



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

[PATCH] D53866: [Preamble] Stop generating preamble for circular #includes

2018-12-06 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik marked 2 inline comments as done and an inline comment as not done.
nik added inline comments.



Comment at: include/clang/Lex/Preprocessor.h:391
   } PreambleConditionalStack;
+  bool PreambleGenerationFailed = false;
 

ilya-biryukov wrote:
> nik wrote:
> > ilya-biryukov wrote:
> > > There's a mechanism to handle preamble with errors, see 
> > > `PreprocessorOpts::AllowPCHWithCompilerErrors`.
> > > Maybe rely on it and avoid adding a different one?
> > I'm not sure how to make use of AllowPCHWithCompilerErrors. It's clearly 
> > meant as an input/readonly option - do you suggest setting that one to 
> > "false" in case we detect the cyclic include (and later checking for 
> > it...)? That feels a bit hacky. Have you meant something else?
> We emit an error, so the preamble will **not** be emitted. 
> Unless the users specify `AllowPCHWithCompilerErrors`, in which case they've 
> signed up for this anyway.
> 
> I propose to **not** add the new flag at all. Would that work?
As stated further above, no.

That's because for the libclang/c-index-test case, 
AllowPCHWithCompilerErrors=true - see clang_parseTranslationUnit_Impl. As such, 
the preamble will be generated/emitted as the second early return in 
PCHGenerator::HandleTranslationUnit is never hit.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866



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

[PATCH] D53866: [Preamble] Stop generating preamble for circular #includes

2018-12-06 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 176941.
nik added a comment.

Added a dedicated diagnostic.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  include/clang/Lex/Preprocessor.h
  include/clang/Serialization/ASTWriter.h
  lib/Frontend/PrecompiledPreamble.cpp
  lib/Lex/PPDirectives.cpp
  test/Index/preamble-cyclic-include.cpp

Index: test/Index/preamble-cyclic-include.cpp
===
--- /dev/null
+++ test/Index/preamble-cyclic-include.cpp
@@ -0,0 +1,9 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-annotate-tokens=%s:4:1:8:1 %s 2>&1 | FileCheck %s
+// CHECK-NOT: error: unterminated conditional directive
+// CHECK-NOT: Skipping: [4:1 - 8:7]
+#ifndef A_H
+#define A_H
+#  include "preamble-cyclic-include.cpp"
+int bar();
+#endif
+
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1933,6 +1933,20 @@
 return;
   }
 
+  // Check whether it makes sense to continue preamble generation. We can't
+  // generate a consistent preamble with regard to the conditional stack if the
+  // main file is included again as due to the preamble bounds some directives
+  // (e.g. #endif of a header guard) will never be seen. Since this will lead to
+  // confusing errors, abort the preamble generation.
+  if (File && PreambleConditionalStack.isRecording() &&
+  SourceMgr.translateFile(File) == SourceMgr.getMainFileID()) {
+PreambleGenerationFailed = true;
+// Generate a fatal error to skip further processing.
+Diag(FilenameTok.getLocation(),
+ diag::err_pp_including_mainfile_for_preamble);
+return;
+  }
+
   // Should we enter the source file? Set to false if either the source file is
   // known to have no effect beyond its effect on module visibility -- that is,
   // if it's got an include guard that is already defined or is a modular header
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -170,6 +170,9 @@
   }
 
   void HandleTranslationUnit(ASTContext &Ctx) override {
+if (getPreprocessor().preambleGenerationFailed())
+  return;
+
 PCHGenerator::HandleTranslationUnit(Ctx);
 if (!hasEmittedPCH())
   return;
Index: include/clang/Serialization/ASTWriter.h
===
--- include/clang/Serialization/ASTWriter.h
+++ include/clang/Serialization/ASTWriter.h
@@ -979,6 +979,7 @@
   ASTWriter &getWriter() { return Writer; }
   const ASTWriter &getWriter() const { return Writer; }
   SmallVectorImpl &getPCH() const { return Buffer->Data; }
+  const Preprocessor &getPreprocessor() const { return PP; }
 
 public:
   PCHGenerator(const Preprocessor &PP, StringRef OutputFile, StringRef isysroot,
Index: include/clang/Lex/Preprocessor.h
===
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -388,6 +388,7 @@
 SmallVector ConditionalStack;
 State ConditionalStackState = Off;
   } PreambleConditionalStack;
+  bool PreambleGenerationFailed = false;
 
   /// The current top of the stack that we're lexing from if
   /// not expanding a macro and we are lexing directly from source code.
@@ -2159,6 +2160,10 @@
   Module *M,
   SourceLocation MLoc);
 
+  bool preambleGenerationFailed() const {
+return PreambleGenerationFailed;
+  }
+
   bool isRecordingPreamble() const {
 return PreambleConditionalStack.isRecording();
   }
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -428,6 +428,8 @@
 : Error<"'%0' file not found, did you mean '%1'?">;
 def err_pp_error_opening_file : Error<
   "error opening file '%0': %1">, DefaultFatal;
+def err_pp_including_mainfile_for_preamble : Error<
+  "main file cannot be included recursively for preamble">, DefaultFatal;
 def err_pp_empty_filename : Error<"empty filename">;
 def err_pp_include_too_deep : Error<"#include nested too deeply">;
 def err_pp_expects_filename : Error<"expected \"FILENAME\" or ">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2018-12-06 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 176962.
nik marked an inline comment as done.
nik added a comment.

Addressed comments.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41005/new/

https://reviews.llvm.org/D41005

Files:
  include/clang/Frontend/ASTUnit.h
  lib/Frontend/ASTUnit.cpp
  lib/Frontend/PrecompiledPreamble.cpp
  unittests/Frontend/PCHPreambleTest.cpp

Index: unittests/Frontend/PCHPreambleTest.cpp
===
--- unittests/Frontend/PCHPreambleTest.cpp
+++ unittests/Frontend/PCHPreambleTest.cpp
@@ -53,7 +53,10 @@
   FileSystemOptions FSOpts;
 
 public:
-  void SetUp() override {
+  void SetUp() override { ResetVFS(); }
+  void TearDown() override {}
+
+  void ResetVFS() {
 VFS = new ReadCountingInMemoryFileSystem();
 // We need the working directory to be set to something absolute,
 // otherwise it ends up being inadvertently set to the current
@@ -64,9 +67,6 @@
 VFS->setCurrentWorkingDirectory("//./");
   }
 
-  void TearDown() override {
-  }
-
   void AddFile(const std::string &Filename, const std::string &Contents) {
 ::time_t now;
 ::time(&now);
@@ -124,6 +124,72 @@
   }
 };
 
+TEST_F(PCHPreambleTest, ReparseReusesPreambleWithUnsavedFileNotExistingOnDisk) {
+  std::string Header1 = "//./header1.h";
+  std::string MainName = "//./main.cpp";
+  AddFile(MainName, R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which does not exist on
+  // disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  // Reparse and check that the preamble was reused.
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+}
+
+TEST_F(PCHPreambleTest, ReparseReusesPreambleAfterUnsavedFileWasCreatedOnDisk) {
+  std::string Header1 = "//./header1.h";
+  std::string MainName = "//./main.cpp";
+  AddFile(MainName, R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which does not exist on
+  // disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  // Create the unsaved file also on disk and check that preamble was reused.
+  AddFile(Header1, "#define ZERO 0\n");
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+}
+
+TEST_F(PCHPreambleTest,
+   ReparseReusesPreambleAfterUnsavedFileWasRemovedFromDisk) {
+  std::string Header1 = "//./foo/header1.h";
+  std::string MainName = "//./main.cpp";
+  std::string MainFileContent = R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp";
+  AddFile(MainName, MainFileContent);
+  AddFile(Header1, "#define ZERO 0\n");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which exists on disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+
+  // Remove the unsaved file from disk and check that the preamble was reused.
+  ResetVFS();
+  AddFile(MainName, MainFileContent);
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+}
+
 TEST_F(PCHPreambleTest, ReparseWithOverriddenFileDoesNotInvalidatePreamble) {
   std::string Header1 = "//./header1.h";
   std::string Header2 = "//./header2.h";
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -28,6 +28,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Mutex.h"
 #include "llvm/Support/MutexGuard.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include 
@@ -449,20 +450,33 @@
 Status.getSize(), llvm::sys::toTimeT(Status.getLastModificationTime()));
   }
 
+  llvm::StringMap OverridenFileBuffers;
   for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
-llvm::vfs::Status Status;
-if (!moveOnNoError(VFS->status(RB.first), Status))
-  return false;
-
-OverriddenFiles[Status.getUniqueID()] =
+const PrecompiledPreamble::PreambleFileHash PreambleHash =
 PreambleFileHash::createForMemoryBuffer(RB.second);
+llvm::vfs::Status Status;
+if (moveOnNoError(VFS->status(RB.first), Status)) {
+  OverriddenFiles[Status.getUniqueID()] = PreambleHash;
+} else {
+  OverridenFileBuffers[RB.first] = PreambleHash;
+}
   }
 
   // Check whether anything has changed.
   for (const auto &F : FilesInPreamble) {
+auto OverridenFileBuffer = OverridenFileBuffers.fin

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2018-12-06 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik marked 4 inline comments as done.
nik added inline comments.



Comment at: include/clang/Frontend/ASTUnit.h:581
 
+  unsigned getPreambleCounter() const { return PreambleCounter; }
+

ilya-biryukov wrote:
> NIT: `getPreambleCounterForTests()`? This is clearly an internal detail, 
> would try giving it a name that discourages using it outside the testing code.
Done.

Side note: I hardly see something like this used in clang, but I agree that 
it's good.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41005/new/

https://reviews.llvm.org/D41005



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

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

2018-12-06 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.
Herald added a subscriber: arphaman.

In D48116#1144732 , @ilya-biryukov 
wrote:

> Have you considered doing the same filtering in ASTUnit's 
> `StoredDiagnosticConsumer`? It should not be more difficult and allows to 
> avoid changing the clang's diagnostic interfaces. That's what we do in clangd.


Will check.

> I wonder if you want to handle notes and remarks in a special manner? They 
> can be seen as part of the original diagnostic, rather than the separate 
> diagnostic. E.g. showing a note in the main file, but not showing the 
> diagnostic from the headers file that this note comes from, might be 
> confusing to the users.

Looks like this case is handled fine.

WIthout the new option, a note is printed as expected/always (shortened output):

  $ bin/c-index-test -test-load-source function 
ignore-warnings-from-headers.cpp -Wunused-parameter
  ignore-warnings-from-headers.h:1:12: warning: unused parameter 
'unusedInHeader' [-Wunused-parameter]
  ignore-warnings-from-headers.cpp:1:10: note: in file included from 
ignore-warnings-from-headers.cpp:1:
  ignore-warnings-from-headers.cpp:3:12: warning: unused parameter 
'unusedInMainFile' [-Wunused-parameter]

With the new option, note is not printed and thus no confusion should arise:

  $ CINDEXTEST_IGNORE_NONERRORS_FROM_INCLUDED_FILES=1 bin/c-index-test 
-test-load-source function ignore-warnings-from-headers.cpp -Wunused-parameter
  ignore-warnings-from-headers.cpp:3:12: warning: unused parameter 
'unusedInMainFile' [-Wunused-parameter]

I will add "// CHECK-NOT: note: in file included from" to the test.

> Maybe also add tests for diagnostics in the main file with notes/remarks in 
> the header files and vice versa?

"Vice versa" is covered as shown above. If the diagnostic is in main file and a 
note of that one refers to the header, then the note should be shown/included. 
This case seems also fine - I've added a test for this.

In D48116#1144838 , @yvvan wrote:

> But this one misses a way to set this flag for everything except libclang. We 
> can probably change that (Nikolai is in vacation till the end of summer so 
> it's probably my part now) and add some tests outside of Index for it 
> (probably Frontend)


What's the use case of this?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D48116/new/

https://reviews.llvm.org/D48116



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

[PATCH] D55415: Revert removal of tidy plugin support from libclang

2018-12-07 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

+1 as this seems to fix the regression.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55415/new/

https://reviews.llvm.org/D55415



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

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

2018-12-07 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

In D48116#1144732 , @ilya-biryukov 
wrote:

> Have you considered doing the same filtering in ASTUnit's 
> `StoredDiagnosticConsumer`? It should not be more difficult and allows to 
> avoid changing the clang's diagnostic interfaces. That's what we do in clangd.


Hmm, it's a bit strange that StoredDiagnosticConsumer should also start to 
filter things.

For filtering in StoredDiagnosticConsumer one needs to pass the new bool 
everywhere along where "bool CaptureDiagnostics" is already passed on (to end 
up in the StoredDiagnosticConsumer constructor) . Also, 
ASTUnit::CaptureDiagnostics is then not enough anymore since the new bool is 
also needed in getMainBufferWithPrecompiledPreamble(). One could also (2) 
convert  "bool CaptureDiagnostics" to an enum with enumerators like 
CaptureNothing, CaptureAll, CaptureAllWithoutNonErrorsFromIncludes to make this 
a bit less invasive.

If changing clang's diagnostic interface should be avoided, I tend to go with 
(2). Ilya?

  $ git grep "bool CaptureDiagnostics"
  include/clang/Frontend/ASTUnit.h:  bool CaptureDiagnostics = false;
  include/clang/Frontend/ASTUnit.h: ASTUnit &AST, 
bool CaptureDiagnostics,
  include/clang/Frontend/ASTUnit.h: 
IntrusiveRefCntPtr Diags, bool CaptureDiagnostics,
  include/clang/Frontend/ASTUnit.h:  bool CaptureDiagnostics = false, bool 
AllowPCHWithCompilerErrors = false,
  include/clang/Frontend/ASTUnit.h:  bool OnlyLocalDecls = false, bool 
CaptureDiagnostics = false,
  include/clang/Frontend/ASTUnit.h:  bool OnlyLocalDecls = false, bool 
CaptureDiagnostics = false,
  include/clang/Frontend/ASTUnit.h:  bool OnlyLocalDecls = false, bool 
CaptureDiagnostics = false,
  lib/Frontend/ASTUnit.cpp: ASTUnit &AST, bool 
CaptureDiagnostics,
  lib/Frontend/ASTUnit.cpp:bool CaptureDiagnostics, bool 
AllowPCHWithCompilerErrors,
  lib/Frontend/ASTUnit.cpp:bool CaptureDiagnostics, bool 
UserFilesAreVolatile) {
  lib/Frontend/ASTUnit.cpp:bool OnlyLocalDecls, bool CaptureDiagnostics,
  lib/Frontend/ASTUnit.cpp:bool OnlyLocalDecls, bool CaptureDiagnostics,
  lib/Frontend/ASTUnit.cpp:bool OnlyLocalDecls, bool CaptureDiagnostics,
  tools/libclang/Indexing.cpp:  bool CaptureDiagnostics = 
!Logger::isLoggingEnabled();


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D48116/new/

https://reviews.llvm.org/D48116



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

[PATCH] D53866: [Preamble] Stop circular inclusion of main file when building preamble

2019-04-18 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping. Ilya?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866



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

[PATCH] D61486: [Basic] Introduce active dummy DiagnosticBuilder

2019-05-03 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

...which does not emit anything.

This dummy is useful for filtering. Code expecting a DiagnosticBuilder
object can get a dummy and report further details, without
knowing/checking whether this is needed at all.


Repository:
  rC Clang

https://reviews.llvm.org/D61486

Files:
  include/clang/Basic/Diagnostic.h


Index: include/clang/Basic/Diagnostic.h
===
--- include/clang/Basic/Diagnostic.h
+++ include/clang/Basic/Diagnostic.h
@@ -1055,6 +1055,9 @@
   // Emit() would end up with if we used that as our status variable.
   mutable bool IsActive = false;
 
+  /// Flag indicating an active dummy builder that will not emit anything.
+  mutable bool IsDummy = false;
+
   /// Flag indicating that this diagnostic is being emitted via a
   /// call to ForceEmit.
   mutable bool IsForceEmit = false;
@@ -1077,6 +1080,7 @@
   void Clear() const {
 DiagObj = nullptr;
 IsActive = false;
+IsDummy = false;
 IsForceEmit = false;
   }
 
@@ -1095,6 +1099,12 @@
 // (or by a subclass, as in SemaDiagnosticBuilder).
 if (!isActive()) return false;
 
+if (IsDummy) {
+  DiagObj->Clear();
+  Clear();
+  return false;
+}
+
 // When emitting diagnostics, we set the final argument count into
 // the DiagnosticsEngine object.
 FlushCounts();
@@ -1114,6 +1124,7 @@
   DiagnosticBuilder(const DiagnosticBuilder &D) {
 DiagObj = D.DiagObj;
 IsActive = D.IsActive;
+IsDummy = D.IsDummy;
 IsForceEmit = D.IsForceEmit;
 D.Clear();
 NumArgs = D.NumArgs;
@@ -1131,6 +1142,13 @@
 return {};
   }
 
+  /// Retrieve an active diagnostic builder that will not emit anything.
+  static DiagnosticBuilder getDummy(DiagnosticsEngine *diagObj) {
+DiagnosticBuilder D(diagObj);
+D.IsDummy = true;
+return D;
+  }
+
   /// Forces the diagnostic to be emitted.
   const DiagnosticBuilder &setForceEmit() const {
 IsForceEmit = true;


Index: include/clang/Basic/Diagnostic.h
===
--- include/clang/Basic/Diagnostic.h
+++ include/clang/Basic/Diagnostic.h
@@ -1055,6 +1055,9 @@
   // Emit() would end up with if we used that as our status variable.
   mutable bool IsActive = false;
 
+  /// Flag indicating an active dummy builder that will not emit anything.
+  mutable bool IsDummy = false;
+
   /// Flag indicating that this diagnostic is being emitted via a
   /// call to ForceEmit.
   mutable bool IsForceEmit = false;
@@ -1077,6 +1080,7 @@
   void Clear() const {
 DiagObj = nullptr;
 IsActive = false;
+IsDummy = false;
 IsForceEmit = false;
   }
 
@@ -1095,6 +1099,12 @@
 // (or by a subclass, as in SemaDiagnosticBuilder).
 if (!isActive()) return false;
 
+if (IsDummy) {
+  DiagObj->Clear();
+  Clear();
+  return false;
+}
+
 // When emitting diagnostics, we set the final argument count into
 // the DiagnosticsEngine object.
 FlushCounts();
@@ -1114,6 +1124,7 @@
   DiagnosticBuilder(const DiagnosticBuilder &D) {
 DiagObj = D.DiagObj;
 IsActive = D.IsActive;
+IsDummy = D.IsDummy;
 IsForceEmit = D.IsForceEmit;
 D.Clear();
 NumArgs = D.NumArgs;
@@ -1131,6 +1142,13 @@
 return {};
   }
 
+  /// Retrieve an active diagnostic builder that will not emit anything.
+  static DiagnosticBuilder getDummy(DiagnosticsEngine *diagObj) {
+DiagnosticBuilder D(diagObj);
+D.IsDummy = true;
+return D;
+  }
+
   /// Forces the diagnostic to be emitted.
   const DiagnosticBuilder &setForceEmit() const {
 IsForceEmit = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

2019-05-03 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik created this revision.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.

The clang-tidy standalone tool implements the NOLINT filtering in
ClangTidyDiagnosticConsumer::HandleDiagnostic. For the plugin case no
ClangTidyDiagnosticConsumer is set up as it would have side effects with
the already set up diagnostic consumer.

This change introduces filtering in ClangTidyContext::diag() and returns
an active dummy DiagnosticBuilder in case the check was NOLINT-ed, so
that no check needs to be adapted. The filtering in HandleDiagnostic
needs to stay there as it is still relevant for non-tidy diagnostics.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61487

Files:
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.h
  test/clang-tidy/nolint-plugin.cpp

Index: test/clang-tidy/nolint-plugin.cpp
===
--- /dev/null
+++ test/clang-tidy/nolint-plugin.cpp
@@ -0,0 +1,49 @@
+// REQUIRES: static-analyzer
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,google-explicit-constructor,clang-diagnostic-unused-variable,clang-analyzer-core.UndefinedBinaryOperatorResult' -Wunused-variable -I%S/Inputs/nolint 2>&1 | FileCheck %s
+
+#include "trigger_warning.h"
+void I(int& Out) {
+  int In;
+  A1(In, Out);
+}
+// CHECK-NOT: trigger_warning.h:{{.*}} warning
+// CHECK-NOT: :[[@LINE-4]]:{{.*}} note
+
+class A { A(int i); };
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class B { B(int i); }; // NOLINT
+
+class C { C(int i); }; // NOLINT(for-some-other-check)
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class C1 { C1(int i); }; // NOLINT(*)
+
+class C2 { C2(int i); }; // NOLINT(not-closed-bracket-is-treated-as-skip-all
+
+class C3 { C3(int i); }; // NOLINT(google-explicit-constructor)
+
+class C4 { C4(int i); }; // NOLINT(some-check, google-explicit-constructor)
+
+class C5 { C5(int i); }; // NOLINT without-brackets-skip-all, another-check
+
+void f() {
+  int i;
+// CHECK-DAG: :[[@LINE-1]]:7: warning: unused variable 'i'
+  int j; // NOLINT
+  int k; // NOLINT(clang-diagnostic-unused-variable)
+}
+
+#define MACRO(X) class X { X(int i); };
+MACRO(D)
+// CHECK-DAG: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+MACRO(E) // NOLINT
+
+#define MACRO_NOARG class F { F(int i); };
+MACRO_NOARG // NOLINT
+
+#define MACRO_NOLINT class G { G(int i); }; // NOLINT
+MACRO_NOLINT
+
+#define DOUBLE_MACRO MACRO(H) // NOLINT
+DOUBLE_MACRO
Index: clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -215,6 +215,8 @@
   std::string ProfilePrefix;
 
   bool AllowEnablingAnalyzerAlphaCheckers;
+
+  bool LastErrorWasIgnored;
 };
 
 /// \brief A diagnostic consumer that turns each \c Diagnostic into a
@@ -255,7 +257,6 @@
   std::unique_ptr HeaderFilter;
   bool LastErrorRelatesToUserCode;
   bool LastErrorPassesLineFilter;
-  bool LastErrorWasIgnored;
 };
 
 } // end namespace tidy
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -186,7 +186,8 @@
 bool AllowEnablingAnalyzerAlphaCheckers)
 : DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)),
   Profile(false),
-  AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers) {
+  AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers),
+  LastErrorWasIgnored(false) {
   // Before the first translation unit we can get errors related to command-line
   // parsing, use empty string for the file name in this case.
   setCurrentFile("");
@@ -194,12 +195,112 @@
 
 ClangTidyContext::~ClangTidyContext() = default;
 
+static bool IsNOLINTFound(StringRef NolintDirectiveText, StringRef Line,
+  unsigned DiagID, const ClangTidyContext &Context) {
+  const size_t NolintIndex = Line.find(NolintDirectiveText);
+  if (NolintIndex == StringRef::npos)
+return false;
+
+  size_t BracketIndex = NolintIndex + NolintDirectiveText.size();
+  // Check if the specific checks are specified in brackets.
+  if (BracketIndex < Line.size() && Line[BracketIndex] == '(') {
+++BracketIndex;
+const size_t BracketEndIndex = Line.find(')', BracketIndex);
+if (BracketEndIndex != StringRef::npos) {
+  StringRef ChecksStr =
+  Line.substr(BracketIndex, BracketEndIndex - BracketIndex);
+  // Allow disabling all the checks with "*".
+  if (ChecksStr != "*") {
+std::string CheckName = Context.getCheckName(DiagID);
+// Allow specif

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

2019-05-03 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

This one depends on https://reviews.llvm.org/D61486


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61487/new/

https://reviews.llvm.org/D61487



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

[PATCH] D60678: [libclang] Forward isInline for NamespaceDecl to libclang

2019-05-03 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Did you forget to push the change to c-index-test.c?

Otherwise fine with me.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60678/new/

https://reviews.llvm.org/D60678



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

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

2019-05-03 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Thanks for the fast comments!

In D61487#1489308 , @alexfh wrote:

> I suspect that we're missing proper test coverage here.

True, ideally all the test scripts would also trigger the plugin case code path.

I've started with a modified copy of nolint.cpp as I wasn't able to have the 
two invocations (clang-tidy, c-index-test plugin) work with the same file. For 
example, the order of the diagnostics is different (seems solvable by appending 
using -DAG) and the c-index-test plugin case does not output "Suppressed 13 
warnings (13 NOLINT)" - is there a way to exclude the check for this output for 
the c-index-test case and still having all in the same file?

I haven't tested the plugin case with nolintnextline.cpp yet, but at least this 
one does not seem to contain the unmute case as far as I see.

> Another issue is that compiler diagnostics don't pass ClangTidyContext::diag 
> in the non-plugin use case.

Right, but how is this an issue?

> Do all the existing tests pass with your patch?

Yes.

> A better way to implement diagnostic filtering in the plugin would be to make 
> ClangTidyDiagnosticConsumer able to forward diagnostics to the external 
> diagnostics engine. It will still need some sort of a buffering though to 
> handle diagnostics and notes attached to them together.

Ahh, you suggest that the plugin should have a separate diagnostic engine and 
instantiate also a ClangTidyDiagnosticConsumer object, that forwards to the 
external one? Will check how this could work.

Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61487/new/

https://reviews.llvm.org/D61487

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

[PATCH] D60678: [libclang] Forward isInline for NamespaceDecl to libclang

2019-05-07 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

...and increased  CINDEX_VERSION_MINOR.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60678/new/

https://reviews.llvm.org/D60678



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

[PATCH] D60678: [libclang] Forward isInline for NamespaceDecl to libclang

2019-05-07 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 198403.
nik added a comment.
Herald added a project: clang.

Adapted c-index-test.c and added function to libclang.exports.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60678/new/

https://reviews.llvm.org/D60678

Files:
  include/clang-c/Index.h
  test/Index/print-type.cpp
  tools/c-index-test/c-index-test.c
  tools/libclang/CXType.cpp
  tools/libclang/libclang.exports


Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -43,6 +43,7 @@
 clang_Cursor_isBitField
 clang_Cursor_isDynamicCall
 clang_Cursor_isExternalSymbol
+clang_Cursor_isInlineNamespace
 clang_Cursor_isNull
 clang_Cursor_isObjCOptional
 clang_Cursor_isVariadic
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -1263,6 +1263,14 @@
   return 0;
 }
 
+unsigned clang_Cursor_isInlineNamespace(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const NamespaceDecl *ND = dyn_cast_or_null(D);
+  return ND ? ND->isInline() : 0;
+}
+
 CXType clang_Type_getNamedType(CXType CT){
   QualType T = GetQualType(CT);
   const Type *TP = T.getTypePtrOrNull();
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -1671,6 +1671,13 @@
   printf(" [isAnonRecDecl=%d]", isAnonRecDecl);
 }
 
+/* Print if it is an inline namespace decl */
+{
+  unsigned isInlineNamespace = clang_Cursor_isInlineNamespace(cursor);
+  if (isInlineNamespace != 0)
+printf(" [isInlineNamespace=%d]", isInlineNamespace);
+}
+
 printf("\n");
   }
   return CXChildVisit_Recurse;
Index: test/Index/print-type.cpp
===
--- test/Index/print-type.cpp
+++ test/Index/print-type.cpp
@@ -90,6 +90,8 @@
 namespace {
   int a;
 }
+
+inline namespace InlineNS {}
 // RUN: c-index-test -test-print-type %s -std=c++14 | FileCheck %s
 // CHECK: Namespace=outer:1:11 (Definition) [type=] [typekind=Invalid] 
[isPOD=0]
 // CHECK: ClassTemplate=Foo:4:8 (Definition) [type=] [typekind=Invalid] 
[isPOD=0]
@@ -204,3 +206,4 @@
 // CHECK: UnionDecl=:86:3 (Definition) [type=X::(anonymous union at 
{{.*}}print-type.cpp:86:3)] [typekind=Record] [isPOD=1] [nbFields=2] [isAnon=1]
 // CHECK: EnumDecl=:87:3 (Definition) [type=X::(anonymous enum at 
{{.*}}print-type.cpp:87:3)] [typekind=Enum] [isPOD=1] [isAnon=1]
 // CHECK: Namespace=:90:11 (Definition) [type=] [typekind=Invalid] [isPOD=0] 
[isAnon=1]
+// CHECK: Namespace=InlineNS:94:18 (Definition) [type=] [typekind=Invalid] 
[isPOD=0] [isAnonRecDecl=0] [isInlineNamespace=1]
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -32,7 +32,7 @@
  * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
  */
 #define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 56
+#define CINDEX_VERSION_MINOR 57
 
 #define CINDEX_VERSION_ENCODE(major, minor) ( \
   ((major) * 1)   \
@@ -3932,6 +3932,12 @@
  */
 CINDEX_LINKAGE unsigned clang_Cursor_isAnonymousRecordDecl(CXCursor C);
 
+/**
+ * Determine whether the given cursor represents an inline namespace
+ * declaration.
+ */
+CINDEX_LINKAGE unsigned clang_Cursor_isInlineNamespace(CXCursor C);
+
 enum CXRefQualifierKind {
   /** No ref-qualifier was provided. */
   CXRefQualifier_None = 0,


Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -43,6 +43,7 @@
 clang_Cursor_isBitField
 clang_Cursor_isDynamicCall
 clang_Cursor_isExternalSymbol
+clang_Cursor_isInlineNamespace
 clang_Cursor_isNull
 clang_Cursor_isObjCOptional
 clang_Cursor_isVariadic
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -1263,6 +1263,14 @@
   return 0;
 }
 
+unsigned clang_Cursor_isInlineNamespace(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const NamespaceDecl *ND = dyn_cast_or_null(D);
+  return ND ? ND->isInline() : 0;
+}
+
 CXType clang_Type_getNamedType(CXType CT){
   QualType T = GetQualType(CT);
   const Type *TP = T.getTypePtrOrNull();
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -1671,6 +1671,13 @@
   printf(" [isAnonRecDecl=%d]", isAnonRecDecl);
 }
 
+

[PATCH] D60678: [libclang] Forward isInline for NamespaceDecl to libclang

2019-05-07 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik accepted this revision.
nik added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60678/new/

https://reviews.llvm.org/D60678



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

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

2019-05-08 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 198637.
nik added a comment.

The plugin makes use of ClangTidyDiagnosticConsumer and forwards diagnostics to
the external diagnostic engine.

@alexfh: What do you think? If that looks roughly OK for you, I'll finish it.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61487/new/

https://reviews.llvm.org/D61487

Files:
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tidy/plugin/ClangTidyPlugin.cpp
  test/clang-tidy/nolint-plugin.cpp
  test/clang-tidy/nolintnextline-plugin.cpp

Index: test/clang-tidy/nolintnextline-plugin.cpp
===
--- /dev/null
+++ test/clang-tidy/nolintnextline-plugin.cpp
@@ -0,0 +1,47 @@
+class A { A(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE
+class B { B(int i); };
+
+// NOLINTNEXTLINE(for-some-other-check)
+class C { C(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE(*)
+class C1 { C1(int i); };
+
+// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all
+class C2 { C2(int i); };
+
+// NOLINTNEXTLINE(google-explicit-constructor)
+class C3 { C3(int i); };
+
+// NOLINTNEXTLINE(some-check, google-explicit-constructor)
+class C4 { C4(int i); };
+
+// NOLINTNEXTLINE without-brackets-skip-all, another-check
+class C5 { C5(int i); };
+
+
+// NOLINTNEXTLINE
+
+class D { D(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE
+//
+class E { E(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+#define MACRO(X) class X { X(int i); };
+MACRO(F)
+// CHECK: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+// NOLINTNEXTLINE
+MACRO(G)
+
+#define MACRO_NOARG class H { H(int i); };
+// NOLINTNEXTLINE
+MACRO_NOARG
+
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s
Index: test/clang-tidy/nolint-plugin.cpp
===
--- /dev/null
+++ test/clang-tidy/nolint-plugin.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: static-analyzer
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,google-explicit-constructor,clang-diagnostic-unused-variable,clang-analyzer-core.UndefinedBinaryOperatorResult' -Wunused-variable -I%S/Inputs/nolint 2>&1 | FileCheck %s
+
+#include "trigger_warning.h"
+void I(int& Out) {
+  int In;
+  A1(In, Out);
+}
+// CHECK-NOT: trigger_warning.h:{{.*}} warning
+// CHECK-NOT: :[[@LINE-4]]:{{.*}} note
+
+class A { A(int i); };
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class B { B(int i); }; // NOLINT
+
+class C { C(int i); }; // NOLINT(for-some-other-check)
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class C1 { C1(int i); }; // NOLINT(*)
+
+class C2 { C2(int i); }; // NOLINT(not-closed-bracket-is-treated-as-skip-all
+
+class C3 { C3(int i); }; // NOLINT(google-explicit-constructor)
+
+class C4 { C4(int i); }; // NOLINT(some-check, google-explicit-constructor)
+
+class C5 { C5(int i); }; // NOLINT without-brackets-skip-all, another-check
+
+void f() {
+  int i;
+// CHECK-DAG: :[[@LINE-1]]:7: warning: unused variable 'i' [-Wunused-variable]
+//  31:7: warning: unused variable 'i' [-Wunused-variable]
+//  int j; // NOLINT
+//  int k; // NOLINT(clang-diagnostic-unused-variable)
+}
+
+#define MACRO(X) class X { X(int i); };
+MACRO(D)
+// CHECK-DAG: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+MACRO(E) // NOLINT
+
+#define MACRO_NOARG class F { F(int i); };
+MACRO_NOARG // NOLINT
+
+#define MACRO_NOLINT class G { G(int i); }; // NOLINT
+MACRO_NOLINT
+
+#define DOUBLE_MACRO MACRO(H) // NOLINT
+DOUBLE_MACRO
Index: clang-tidy/plugin/ClangTidyPlugin.cpp
===
--- clang-tidy/plugin/ClangTidyPlugin.cpp
+++ clang-tidy/plugin/ClangTidyPlugin.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "../ClangTidy.h"
+#include "../ClangTidyDiagnosticConsumer.h"
 #include "../ClangTidyForceLinker.h"
 #include "../ClangTidyModule.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -33,8 +34,13 @@
 public:
   std::unique_ptr CreateASTConsumer(CompilerInstance &Compiler,
  StringRef File) override {
-// Insert the current diagnostics engine.
-Context->setDiagnosticsEngine(&Compiler.getDiagnostics());
+// Create a

[PATCH] D60672: [libclang] visit c++14 lambda capture init expressions

2019-05-08 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Tests do not pass on current trunk:

  /d2/llvm/trunk/builds/DebugShared % bin/llvm-lit -a 
/d2/llvm/trunk/source/tools/clang/test/Index/cxx14-lambdas.cpp
  llvm-lit: /d2/llvm/trunk/source/utils/lit/lit/llvm/config.py:341: note: using 
clang: /d2/llvm/trunk/builds/DebugShared/bin/clang
  -- Testing: 1 tests, single process --
  FAIL: Clang :: Index/cxx14-lambdas.cpp (1 of 1)
   TEST 'Clang :: Index/cxx14-lambdas.cpp' FAILED 

  Script:
  --
  : 'RUN: at line 13';   /d2/llvm/trunk/builds/DebugShared/bin/c-index-test 
-test-load-source all -std=c++14 
/d2/llvm/trunk/source/tools/clang/test/Index/cxx14-lambdas.cpp | 
/d2/llvm/trunk/builds/DebugShared/bin/FileCheck -check-prefix=CHECK-LOAD 
/d2/llvm/trunk/source/tools/clang/test/Index/cxx14-lambdas.cpp
  : 'RUN: at line 30';   env CINDEXTEST_INDEXLOCALSYMBOLS=1 
/d2/llvm/trunk/builds/DebugShared/bin/c-index-test -index-file -std=c++14 
/d2/llvm/trunk/source/tools/clang/test/Index/cxx14-lambdas.cpp | 
/d2/llvm/trunk/builds/DebugShared/bin/FileCheck -check-prefix=CHECK-INDEX 
/d2/llvm/trunk/source/tools/clang/test/Index/cxx14-lambdas.cpp
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  /d2/llvm/trunk/source/tools/clang/test/Index/cxx14-lambdas.cpp:22:16: error: 
CHECK-LOAD: expected string not found in input
  // CHECK-LOAD: cxx14-lambdas.cpp:7:27: DeclRefExpr=localA:6:9 Extent=[7:27 - 
7:33]
 ^
  :390:1: note: scanning from here
  // CHECK: cxx14-lambdas.cpp:7:59: ParmDecl=x:7:59 (Definition) Extent=[7:51 - 
7:60]
  ^
  :402:11: note: possible intended match here
  // CHECK: cxx14-lambdas.cpp:8:21: DeclRefExpr=copy:7:35 Extent=[8:21 - 8:25]
^
  
  --
  
  
  Testing Time: 0.13s
  
  Failing Tests (1):
  Clang :: Index/cxx14-lambdas.cpp
  
Unexpected Failures: 1
  zsh: exit 1 bin/llvm-lit -a 
/d2/llvm/trunk/source/tools/clang/test/Index/cxx14-lambdas.cp


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60672/new/

https://reviews.llvm.org/D60672



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

[PATCH] D53866: [Preamble] Stop circular inclusion of main file when building preamble

2019-05-08 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 198654.
nik edited the summary of this revision.
nik added a comment.

Rebased for current trunk.

If I miss something obvious, please tell me. Otherwise I'm waiting.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Basic/SourceManager.cpp
  lib/Lex/PPDirectives.cpp
  test/Index/preamble-cyclic-include.cpp


Index: test/Index/preamble-cyclic-include.cpp
===
--- /dev/null
+++ test/Index/preamble-cyclic-include.cpp
@@ -0,0 +1,9 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test 
-test-annotate-tokens=%s:5:1:10:1 %s 2>&1 | FileCheck %s
+// CHECK-NOT: error: unterminated conditional directive
+// CHECK-NOT: Skipping: [4:1 - 8:7]
+// CHECK: error: main file cannot be included recursively when building a 
preamble
+#ifndef A_H
+#define A_H
+#  include "preamble-cyclic-include.cpp"
+int bar();
+#endif
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1871,6 +1871,18 @@
 return {ImportAction::None};
   }
 
+  // Check for circular inclusion of the main file.
+  // We can't generate a consistent preamble with regard to the conditional
+  // stack if the main file is included again as due to the preamble bounds
+  // some directives (e.g. #endif of a header guard) will never be seen.
+  // Since this will lead to confusing errors, avoid the inclusion.
+  if (File && PreambleConditionalStack.isRecording() &&
+  SourceMgr.translateFile(File) == SourceMgr.getMainFileID()) {
+Diag(FilenameTok.getLocation(),
+ diag::err_pp_including_mainfile_for_preamble);
+return {ImportAction::None};
+  }
+
   // Should we enter the source file? Set to Skip if either the source file is
   // known to have no effect beyond its effect on module visibility -- that is,
   // if it's got an include guard that is already defined, set to Import if it
Index: lib/Basic/SourceManager.cpp
===
--- lib/Basic/SourceManager.cpp
+++ lib/Basic/SourceManager.cpp
@@ -1591,7 +1591,7 @@
 // as the main file.
 const FileEntry *MainFile = MainContentCache->OrigEntry;
 SourceFileName = llvm::sys::path::filename(SourceFile->getName());
-if (*SourceFileName == llvm::sys::path::filename(MainFile->getName())) 
{
+if (MainFile && *SourceFileName == 
llvm::sys::path::filename(MainFile->getName())) {
   SourceFileUID = getActualFileUID(SourceFile);
   if (SourceFileUID) {
 if (Optional MainFileUID =
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -426,6 +426,8 @@
   "did not find header '%0' in framework '%1' (loaded from '%2')">;
 def err_pp_error_opening_file : Error<
   "error opening file '%0': %1">, DefaultFatal;
+def err_pp_including_mainfile_for_preamble : Error<
+  "main file cannot be included recursively when building a preamble">;
 def err_pp_empty_filename : Error<"empty filename">;
 def err_pp_include_too_deep : Error<"#include nested too deeply">;
 def err_pp_expects_filename : Error<"expected \"FILENAME\" or ">;


Index: test/Index/preamble-cyclic-include.cpp
===
--- /dev/null
+++ test/Index/preamble-cyclic-include.cpp
@@ -0,0 +1,9 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-annotate-tokens=%s:5:1:10:1 %s 2>&1 | FileCheck %s
+// CHECK-NOT: error: unterminated conditional directive
+// CHECK-NOT: Skipping: [4:1 - 8:7]
+// CHECK: error: main file cannot be included recursively when building a preamble
+#ifndef A_H
+#define A_H
+#  include "preamble-cyclic-include.cpp"
+int bar();
+#endif
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1871,6 +1871,18 @@
 return {ImportAction::None};
   }
 
+  // Check for circular inclusion of the main file.
+  // We can't generate a consistent preamble with regard to the conditional
+  // stack if the main file is included again as due to the preamble bounds
+  // some directives (e.g. #endif of a header guard) will never be seen.
+  // Since this will lead to confusing errors, avoid the inclusion.
+  if (File && PreambleConditionalStack.isRecording() &&
+  SourceMgr.translateFile(File) == SourceMgr.getMainFileID()) {
+Diag(FilenameTok.getLocation(),
+ diag::err_pp_including_mainfile_for_preamble);
+return {ImportAction::None};
+  }
+
   // Should we enter the source file? Set to Skip if either the source file is
   // known to have no effect beyond its effect

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2019-05-08 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 198656.
nik added a comment.
Herald added a subscriber: dexonsmith.

Minor diff update fixing indentation and removing not needed include.

Ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41005/new/

https://reviews.llvm.org/D41005

Files:
  include/clang/Frontend/ASTUnit.h
  lib/Frontend/ASTUnit.cpp
  lib/Frontend/PrecompiledPreamble.cpp
  unittests/Frontend/PCHPreambleTest.cpp

Index: unittests/Frontend/PCHPreambleTest.cpp
===
--- unittests/Frontend/PCHPreambleTest.cpp
+++ unittests/Frontend/PCHPreambleTest.cpp
@@ -52,7 +52,10 @@
   FileSystemOptions FSOpts;
 
 public:
-  void SetUp() override {
+  void SetUp() override { ResetVFS(); }
+  void TearDown() override {}
+
+  void ResetVFS() {
 VFS = new ReadCountingInMemoryFileSystem();
 // We need the working directory to be set to something absolute,
 // otherwise it ends up being inadvertently set to the current
@@ -63,9 +66,6 @@
 VFS->setCurrentWorkingDirectory("//./");
   }
 
-  void TearDown() override {
-  }
-
   void AddFile(const std::string &Filename, const std::string &Contents) {
 ::time_t now;
 ::time(&now);
@@ -123,6 +123,72 @@
   }
 };
 
+TEST_F(PCHPreambleTest, ReparseReusesPreambleWithUnsavedFileNotExistingOnDisk) {
+  std::string Header1 = "//./header1.h";
+  std::string MainName = "//./main.cpp";
+  AddFile(MainName, R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which does not exist on
+  // disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  // Reparse and check that the preamble was reused.
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+}
+
+TEST_F(PCHPreambleTest, ReparseReusesPreambleAfterUnsavedFileWasCreatedOnDisk) {
+  std::string Header1 = "//./header1.h";
+  std::string MainName = "//./main.cpp";
+  AddFile(MainName, R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which does not exist on
+  // disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  // Create the unsaved file also on disk and check that preamble was reused.
+  AddFile(Header1, "#define ZERO 0\n");
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+}
+
+TEST_F(PCHPreambleTest,
+   ReparseReusesPreambleAfterUnsavedFileWasRemovedFromDisk) {
+  std::string Header1 = "//./foo/header1.h";
+  std::string MainName = "//./main.cpp";
+  std::string MainFileContent = R"cpp(
+#include "//./foo/header1.h"
+int main() { return ZERO; }
+)cpp";
+  AddFile(MainName, MainFileContent);
+  AddFile(Header1, "#define ZERO 0\n");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which exists on disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+
+  // Remove the unsaved file from disk and check that the preamble was reused.
+  ResetVFS();
+  AddFile(MainName, MainFileContent);
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+}
+
 TEST_F(PCHPreambleTest, ReparseWithOverriddenFileDoesNotInvalidatePreamble) {
   std::string Header1 = "//./header1.h";
   std::string Header2 = "//./header2.h";
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -454,20 +454,32 @@
 Status.getSize(), llvm::sys::toTimeT(Status.getLastModificationTime()));
   }
 
+  llvm::StringMap OverridenFileBuffers;
   for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
-llvm::vfs::Status Status;
-if (!moveOnNoError(VFS->status(RB.first), Status))
-  return false;
-
-OverriddenFiles[Status.getUniqueID()] =
+const PrecompiledPreamble::PreambleFileHash PreambleHash =
 PreambleFileHash::createForMemoryBuffer(RB.second);
+llvm::vfs::Status Status;
+if (moveOnNoError(VFS->status(RB.first), Status))
+  OverriddenFiles[Status.getUniqueID()] = PreambleHash;
+else
+  OverridenFileBuffers[RB.first] = PreambleHash;
   }
 
   // Check whether anything has changed.
   for (const auto &F : FilesInPreamble) {
+auto OverridenFileBuffer = OverridenFileBuffers.find(F.first());
+if (OverridenFileBuffer != OverridenFileBuffers.end()) {
+  // The file's buffer was remapped; check whether it matches up
+  // with the previous mapping.
+  if (Overrid

[PATCH] D53866: [Preamble] Stop circular inclusion of main file when building preamble

2019-05-09 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik marked an inline comment as done.
nik added inline comments.



Comment at: lib/Basic/SourceManager.cpp:1594
 SourceFileName = llvm::sys::path::filename(SourceFile->getName());
-if (*SourceFileName == llvm::sys::path::filename(MainFile->getName())) 
{
+if (MainFile && *SourceFileName == 
llvm::sys::path::filename(MainFile->getName())) {
   SourceFileUID = getActualFileUID(SourceFile);

ilya-biryukov wrote:
> Can this actually be`null`? 
The comment for OrigEntry states that it might be null:

/// Reference to the file entry representing this ContentCache.
///
/// This reference does not own the FileEntry object.
///
/// It is possible for this to be NULL if the ContentCache encapsulates
/// an imaginary text buffer.
const FileEntry *OrigEntry;

Note also that further down in this function, a null check for 
MainContentCache->OrigEntry is done, so I believe that this was just forgotten 
here (since there is also no assert) and we are the first one running into this 
with the introduced call to SourceMgr.translateFile(File).

I've tried to understand why we end up with a nullptr here, but this goes 
beyond me in the time I've taken for this. What I've observed is that module 
code path (LibclangReparseTest.ReparseWithModule vs 
LibclangReparseTest.Reparse) creates at least a MemoryBuffer more (in 
ModuleMap::parseModuleMapFile; possibly the one referred with "imaginary text 
buffer" from the comment) and I suspect this to be somehow related with the 
OrigEntry nullptr.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866



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

[PATCH] D53866: [Preamble] Stop circular inclusion of main file when building preamble

2019-05-09 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 198799.
nik added a comment.

Moved the MainFile / MainContentCache->OrigEntry check a bit further up, for
consistency with the same test further down in SourceManager::translateFile().


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Basic/SourceManager.cpp
  lib/Lex/PPDirectives.cpp
  test/Index/preamble-cyclic-include.cpp


Index: test/Index/preamble-cyclic-include.cpp
===
--- /dev/null
+++ test/Index/preamble-cyclic-include.cpp
@@ -0,0 +1,9 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test 
-test-annotate-tokens=%s:5:1:10:1 %s 2>&1 | FileCheck %s
+// CHECK-NOT: error: unterminated conditional directive
+// CHECK-NOT: Skipping: [4:1 - 8:7]
+// CHECK: error: main file cannot be included recursively when building a 
preamble
+#ifndef A_H
+#define A_H
+#  include "preamble-cyclic-include.cpp"
+int bar();
+#endif
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1871,6 +1871,18 @@
 return {ImportAction::None};
   }
 
+  // Check for circular inclusion of the main file.
+  // We can't generate a consistent preamble with regard to the conditional
+  // stack if the main file is included again as due to the preamble bounds
+  // some directives (e.g. #endif of a header guard) will never be seen.
+  // Since this will lead to confusing errors, avoid the inclusion.
+  if (File && PreambleConditionalStack.isRecording() &&
+  SourceMgr.translateFile(File) == SourceMgr.getMainFileID()) {
+Diag(FilenameTok.getLocation(),
+ diag::err_pp_including_mainfile_for_preamble);
+return {ImportAction::None};
+  }
+
   // Should we enter the source file? Set to Skip if either the source file is
   // known to have no effect beyond its effect on module visibility -- that is,
   // if it's got an include guard that is already defined, set to Import if it
Index: lib/Basic/SourceManager.cpp
===
--- lib/Basic/SourceManager.cpp
+++ lib/Basic/SourceManager.cpp
@@ -1582,7 +1582,7 @@
 if (MainSLoc.isFile()) {
   const ContentCache *MainContentCache
 = MainSLoc.getFile().getContentCache();
-  if (!MainContentCache) {
+  if (!MainContentCache || !MainContentCache->OrigEntry) {
 // Can't do anything
   } else if (MainContentCache->OrigEntry == SourceFile) {
 FirstFID = MainFileID;
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -426,6 +426,8 @@
   "did not find header '%0' in framework '%1' (loaded from '%2')">;
 def err_pp_error_opening_file : Error<
   "error opening file '%0': %1">, DefaultFatal;
+def err_pp_including_mainfile_for_preamble : Error<
+  "main file cannot be included recursively when building a preamble">;
 def err_pp_empty_filename : Error<"empty filename">;
 def err_pp_include_too_deep : Error<"#include nested too deeply">;
 def err_pp_expects_filename : Error<"expected \"FILENAME\" or ">;


Index: test/Index/preamble-cyclic-include.cpp
===
--- /dev/null
+++ test/Index/preamble-cyclic-include.cpp
@@ -0,0 +1,9 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-annotate-tokens=%s:5:1:10:1 %s 2>&1 | FileCheck %s
+// CHECK-NOT: error: unterminated conditional directive
+// CHECK-NOT: Skipping: [4:1 - 8:7]
+// CHECK: error: main file cannot be included recursively when building a preamble
+#ifndef A_H
+#define A_H
+#  include "preamble-cyclic-include.cpp"
+int bar();
+#endif
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1871,6 +1871,18 @@
 return {ImportAction::None};
   }
 
+  // Check for circular inclusion of the main file.
+  // We can't generate a consistent preamble with regard to the conditional
+  // stack if the main file is included again as due to the preamble bounds
+  // some directives (e.g. #endif of a header guard) will never be seen.
+  // Since this will lead to confusing errors, avoid the inclusion.
+  if (File && PreambleConditionalStack.isRecording() &&
+  SourceMgr.translateFile(File) == SourceMgr.getMainFileID()) {
+Diag(FilenameTok.getLocation(),
+ diag::err_pp_including_mainfile_for_preamble);
+return {ImportAction::None};
+  }
+
   // Should we enter the source file? Set to Skip if either the source file is
   // known to have no effect beyond its effect on module visibility -- that is,
   // if it's got an include guard that is already defined, set to Import if it
In

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2019-05-09 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 198809.
nik added a comment.

Addressed inline comments.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41005/new/

https://reviews.llvm.org/D41005

Files:
  include/clang/Frontend/ASTUnit.h
  lib/Frontend/ASTUnit.cpp
  lib/Frontend/PrecompiledPreamble.cpp
  unittests/Frontend/PCHPreambleTest.cpp

Index: unittests/Frontend/PCHPreambleTest.cpp
===
--- unittests/Frontend/PCHPreambleTest.cpp
+++ unittests/Frontend/PCHPreambleTest.cpp
@@ -52,7 +52,10 @@
   FileSystemOptions FSOpts;
 
 public:
-  void SetUp() override {
+  void SetUp() override { ResetVFS(); }
+  void TearDown() override {}
+
+  void ResetVFS() {
 VFS = new ReadCountingInMemoryFileSystem();
 // We need the working directory to be set to something absolute,
 // otherwise it ends up being inadvertently set to the current
@@ -63,9 +66,6 @@
 VFS->setCurrentWorkingDirectory("//./");
   }
 
-  void TearDown() override {
-  }
-
   void AddFile(const std::string &Filename, const std::string &Contents) {
 ::time_t now;
 ::time(&now);
@@ -123,6 +123,72 @@
   }
 };
 
+TEST_F(PCHPreambleTest, ReparseReusesPreambleWithUnsavedFileNotExistingOnDisk) {
+  std::string Header1 = "//./header1.h";
+  std::string MainName = "//./main.cpp";
+  AddFile(MainName, R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which does not exist on
+  // disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  // Reparse and check that the preamble was reused.
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+}
+
+TEST_F(PCHPreambleTest, ReparseReusesPreambleAfterUnsavedFileWasCreatedOnDisk) {
+  std::string Header1 = "//./header1.h";
+  std::string MainName = "//./main.cpp";
+  AddFile(MainName, R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which does not exist on
+  // disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  // Create the unsaved file also on disk and check that preamble was reused.
+  AddFile(Header1, "#define ZERO 0\n");
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+}
+
+TEST_F(PCHPreambleTest,
+   ReparseReusesPreambleAfterUnsavedFileWasRemovedFromDisk) {
+  std::string Header1 = "//./foo/header1.h";
+  std::string MainName = "//./main.cpp";
+  std::string MainFileContent = R"cpp(
+#include "//./foo/header1.h"
+int main() { return ZERO; }
+)cpp";
+  AddFile(MainName, MainFileContent);
+  AddFile(Header1, "#define ZERO 0\n");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which exists on disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+
+  // Remove the unsaved file from disk and check that the preamble was reused.
+  ResetVFS();
+  AddFile(MainName, MainFileContent);
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+}
+
 TEST_F(PCHPreambleTest, ReparseWithOverriddenFileDoesNotInvalidatePreamble) {
   std::string Header1 = "//./header1.h";
   std::string Header2 = "//./header2.h";
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -454,20 +454,33 @@
 Status.getSize(), llvm::sys::toTimeT(Status.getLastModificationTime()));
   }
 
+  // OverridenFileBuffers tracks only the files not found in VFS.
+  llvm::StringMap OverridenFileBuffers;
   for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
-llvm::vfs::Status Status;
-if (!moveOnNoError(VFS->status(RB.first), Status))
-  return false;
-
-OverriddenFiles[Status.getUniqueID()] =
+const PrecompiledPreamble::PreambleFileHash PreambleHash =
 PreambleFileHash::createForMemoryBuffer(RB.second);
+llvm::vfs::Status Status;
+if (moveOnNoError(VFS->status(RB.first), Status))
+  OverriddenFiles[Status.getUniqueID()] = PreambleHash;
+else
+  OverridenFileBuffers[RB.first] = PreambleHash;
   }
 
   // Check whether anything has changed.
   for (const auto &F : FilesInPreamble) {
+auto OverridenFileBuffer = OverridenFileBuffers.find(F.first());
+if (OverridenFileBuffer != OverridenFileBuffers.end()) {
+  // The file's buffer was remapped and the file was not found in VFS.
+  // Check whether it matches up with the previous mapping.
+

[PATCH] D53866: [Preamble] Stop circular inclusion of main file when building preamble

2019-05-09 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik marked an inline comment as done.
nik added inline comments.



Comment at: lib/Basic/SourceManager.cpp:1594
 SourceFileName = llvm::sys::path::filename(SourceFile->getName());
-if (*SourceFileName == llvm::sys::path::filename(MainFile->getName())) 
{
+if (MainFile && *SourceFileName == 
llvm::sys::path::filename(MainFile->getName())) {
   SourceFileUID = getActualFileUID(SourceFile);

ilya-biryukov wrote:
> nik wrote:
> > ilya-biryukov wrote:
> > > Can this actually be`null`? 
> > The comment for OrigEntry states that it might be null:
> > 
> > /// Reference to the file entry representing this ContentCache.
> > ///
> > /// This reference does not own the FileEntry object.
> > ///
> > /// It is possible for this to be NULL if the ContentCache encapsulates
> > /// an imaginary text buffer.
> > const FileEntry *OrigEntry;
> > 
> > Note also that further down in this function, a null check for 
> > MainContentCache->OrigEntry is done, so I believe that this was just 
> > forgotten here (since there is also no assert) and we are the first one 
> > running into this with the introduced call to SourceMgr.translateFile(File).
> > 
> > I've tried to understand why we end up with a nullptr here, but this goes 
> > beyond me in the time I've taken for this. What I've observed is that 
> > module code path (LibclangReparseTest.ReparseWithModule vs 
> > LibclangReparseTest.Reparse) creates at least a MemoryBuffer more (in 
> > ModuleMap::parseModuleMapFile; possibly the one referred with "imaginary 
> > text buffer" from the comment) and I suspect this to be somehow related 
> > with the OrigEntry nullptr.
> Should we check for equality of `MainContentCache->ContentsEntry` in that 
> case? I guess this is what should be set for "imaginary" files.
Does not help in this particular case as ContentsEntry is also a nullptr. All 
the members of the ContentsCache seem to be either nullptr or 0 as that 
particular point of execution.

How to continue?

a) Accept as is.
b) Ask someone knowledgeable for whom this could be a no-brainer that could 
respond hopefully soon. Any candidate?
c) Or should I dig deeper? This can take quite some time as this area is new 
for me.



Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866



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

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2019-02-12 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 186429.
nik marked an inline comment as done.
nik added a comment.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.

Addressed comment.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41005/new/

https://reviews.llvm.org/D41005

Files:
  include/clang/Frontend/ASTUnit.h
  lib/Frontend/ASTUnit.cpp
  lib/Frontend/PrecompiledPreamble.cpp
  unittests/Frontend/PCHPreambleTest.cpp

Index: unittests/Frontend/PCHPreambleTest.cpp
===
--- unittests/Frontend/PCHPreambleTest.cpp
+++ unittests/Frontend/PCHPreambleTest.cpp
@@ -52,7 +52,10 @@
   FileSystemOptions FSOpts;
 
 public:
-  void SetUp() override {
+  void SetUp() override { ResetVFS(); }
+  void TearDown() override {}
+
+  void ResetVFS() {
 VFS = new ReadCountingInMemoryFileSystem();
 // We need the working directory to be set to something absolute,
 // otherwise it ends up being inadvertently set to the current
@@ -63,9 +66,6 @@
 VFS->setCurrentWorkingDirectory("//./");
   }
 
-  void TearDown() override {
-  }
-
   void AddFile(const std::string &Filename, const std::string &Contents) {
 ::time_t now;
 ::time(&now);
@@ -123,6 +123,72 @@
   }
 };
 
+TEST_F(PCHPreambleTest, ReparseReusesPreambleWithUnsavedFileNotExistingOnDisk) {
+  std::string Header1 = "//./header1.h";
+  std::string MainName = "//./main.cpp";
+  AddFile(MainName, R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which does not exist on
+  // disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  // Reparse and check that the preamble was reused.
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+}
+
+TEST_F(PCHPreambleTest, ReparseReusesPreambleAfterUnsavedFileWasCreatedOnDisk) {
+  std::string Header1 = "//./header1.h";
+  std::string MainName = "//./main.cpp";
+  AddFile(MainName, R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which does not exist on
+  // disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  // Create the unsaved file also on disk and check that preamble was reused.
+  AddFile(Header1, "#define ZERO 0\n");
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+}
+
+TEST_F(PCHPreambleTest,
+   ReparseReusesPreambleAfterUnsavedFileWasRemovedFromDisk) {
+  std::string Header1 = "//./foo/header1.h";
+  std::string MainName = "//./main.cpp";
+  std::string MainFileContent = R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp";
+  AddFile(MainName, MainFileContent);
+  AddFile(Header1, "#define ZERO 0\n");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which exists on disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+
+  // Remove the unsaved file from disk and check that the preamble was reused.
+  ResetVFS();
+  AddFile(MainName, MainFileContent);
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+}
+
 TEST_F(PCHPreambleTest, ReparseWithOverriddenFileDoesNotInvalidatePreamble) {
   std::string Header1 = "//./header1.h";
   std::string Header2 = "//./header2.h";
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -28,6 +28,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Mutex.h"
 #include "llvm/Support/MutexGuard.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include 
@@ -451,20 +452,32 @@
 Status.getSize(), llvm::sys::toTimeT(Status.getLastModificationTime()));
   }
 
+  llvm::StringMap OverridenFileBuffers;
   for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
-llvm::vfs::Status Status;
-if (!moveOnNoError(VFS->status(RB.first), Status))
-  return false;
-
-OverriddenFiles[Status.getUniqueID()] =
+const PrecompiledPreamble::PreambleFileHash PreambleHash =
 PreambleFileHash::createForMemoryBuffer(RB.second);
+llvm::vfs::Status Status;
+if (moveOnNoError(VFS->status(RB.first), Status))
+  OverriddenFiles[Status.getUniqueID()] = PreambleHash;
+else
+  OverridenFileBuffers[RB.first] = PreambleHash;
   }
 
   // Check whether anything has changed.
   for (const auto &F : FilesInPreamble) {
+

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2019-02-12 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Meh, something changed in the meanwhile. 
ReparseReusesPreambleAfterUnsavedFileWasRemovedFromDisk fails now. Looking into 
it.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41005/new/

https://reviews.llvm.org/D41005



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

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2019-02-12 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 186436.
nik added a comment.

> Meh, something changed in the meanwhile. 
> ReparseReusesPreambleAfterUnsavedFileWasRemovedFromDisk fails now. Looking 
> into it.

No, it's just me ;) I've referenced the header file wrong.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41005/new/

https://reviews.llvm.org/D41005

Files:
  include/clang/Frontend/ASTUnit.h
  lib/Frontend/ASTUnit.cpp
  lib/Frontend/PrecompiledPreamble.cpp
  unittests/Frontend/PCHPreambleTest.cpp

Index: unittests/Frontend/PCHPreambleTest.cpp
===
--- unittests/Frontend/PCHPreambleTest.cpp
+++ unittests/Frontend/PCHPreambleTest.cpp
@@ -52,7 +52,10 @@
   FileSystemOptions FSOpts;
 
 public:
-  void SetUp() override {
+  void SetUp() override { ResetVFS(); }
+  void TearDown() override {}
+
+  void ResetVFS() {
 VFS = new ReadCountingInMemoryFileSystem();
 // We need the working directory to be set to something absolute,
 // otherwise it ends up being inadvertently set to the current
@@ -63,9 +66,6 @@
 VFS->setCurrentWorkingDirectory("//./");
   }
 
-  void TearDown() override {
-  }
-
   void AddFile(const std::string &Filename, const std::string &Contents) {
 ::time_t now;
 ::time(&now);
@@ -123,6 +123,72 @@
   }
 };
 
+TEST_F(PCHPreambleTest, ReparseReusesPreambleWithUnsavedFileNotExistingOnDisk) {
+  std::string Header1 = "//./header1.h";
+  std::string MainName = "//./main.cpp";
+  AddFile(MainName, R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which does not exist on
+  // disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  // Reparse and check that the preamble was reused.
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+}
+
+TEST_F(PCHPreambleTest, ReparseReusesPreambleAfterUnsavedFileWasCreatedOnDisk) {
+  std::string Header1 = "//./header1.h";
+  std::string MainName = "//./main.cpp";
+  AddFile(MainName, R"cpp(
+#include "//./header1.h"
+int main() { return ZERO; }
+)cpp");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which does not exist on
+  // disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  // Create the unsaved file also on disk and check that preamble was reused.
+  AddFile(Header1, "#define ZERO 0\n");
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+}
+
+TEST_F(PCHPreambleTest,
+   ReparseReusesPreambleAfterUnsavedFileWasRemovedFromDisk) {
+  std::string Header1 = "//./foo/header1.h";
+  std::string MainName = "//./main.cpp";
+  std::string MainFileContent = R"cpp(
+#include "//./foo/header1.h"
+int main() { return ZERO; }
+)cpp";
+  AddFile(MainName, MainFileContent);
+  AddFile(Header1, "#define ZERO 0\n");
+  RemapFile(Header1, "#define ZERO 0\n");
+
+  // Parse with header file provided as unsaved file, which exists on disk.
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+
+  // Remove the unsaved file from disk and check that the preamble was reused.
+  ResetVFS();
+  AddFile(MainName, MainFileContent);
+  ASSERT_TRUE(ReparseAST(AST));
+  ASSERT_EQ(AST->getPreambleCounterForTests(), 1U);
+}
+
 TEST_F(PCHPreambleTest, ReparseWithOverriddenFileDoesNotInvalidatePreamble) {
   std::string Header1 = "//./header1.h";
   std::string Header2 = "//./header2.h";
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -28,6 +28,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Mutex.h"
 #include "llvm/Support/MutexGuard.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include 
@@ -451,20 +452,32 @@
 Status.getSize(), llvm::sys::toTimeT(Status.getLastModificationTime()));
   }
 
+  llvm::StringMap OverridenFileBuffers;
   for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
-llvm::vfs::Status Status;
-if (!moveOnNoError(VFS->status(RB.first), Status))
-  return false;
-
-OverriddenFiles[Status.getUniqueID()] =
+const PrecompiledPreamble::PreambleFileHash PreambleHash =
 PreambleFileHash::createForMemoryBuffer(RB.second);
+llvm::vfs::Status Status;
+if (moveOnNoError(VFS->status(RB.first), Status))
+  OverriddenFiles[Status.getUniqueID()] = PreambleHash;
+else
+  OverridenFileBuffers[RB.first] = PreambleHash;
   }
 
   // Check whe

[PATCH] D53866: [Preamble] Stop generating preamble for circular #includes

2019-02-14 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik marked 2 inline comments as done.
nik added inline comments.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.



Comment at: include/clang/Lex/Preprocessor.h:391
   } PreambleConditionalStack;
+  bool PreambleGenerationFailed = false;
 

ilya-biryukov wrote:
> nik wrote:
> > ilya-biryukov wrote:
> > > nik wrote:
> > > > ilya-biryukov wrote:
> > > > > There's a mechanism to handle preamble with errors, see 
> > > > > `PreprocessorOpts::AllowPCHWithCompilerErrors`.
> > > > > Maybe rely on it and avoid adding a different one?
> > > > I'm not sure how to make use of AllowPCHWithCompilerErrors. It's 
> > > > clearly meant as an input/readonly option - do you suggest setting that 
> > > > one to "false" in case we detect the cyclic include (and later checking 
> > > > for it...)? That feels a bit hacky. Have you meant something else?
> > > We emit an error, so the preamble will **not** be emitted. 
> > > Unless the users specify `AllowPCHWithCompilerErrors`, in which case 
> > > they've signed up for this anyway.
> > > 
> > > I propose to **not** add the new flag at all. Would that work?
> > As stated further above, no.
> > 
> > That's because for the libclang/c-index-test case, 
> > AllowPCHWithCompilerErrors=true - see clang_parseTranslationUnit_Impl. As 
> > such, the preamble will be generated/emitted as the second early return in 
> > PCHGenerator::HandleTranslationUnit is never hit.
> if `AllowPCHWithCompilerErrors=true` is set to true, why not simply pretend 
> the include was not found? That would still generate the preamble, but users 
> have the error message to see that something went wrong.
> 
> `c-index-test` produces the errors, so we can check the error is present in 
> the output.
> if AllowPCHWithCompilerErrors=true is set to true, why not simply pretend the 
> include was not found?

Sounds good, especially that the preamble is still generated. 

> That would still generate the preamble, but users have the error message to 
> see that something went wrong.

The vanilla diagnostic in this case might be a bit confusing, I kept the 
introduced diagnostics as it carries more information.



Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866



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

[PATCH] D53866: [Preamble] Stop generating preamble for circular #includes

2019-02-14 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 186813.
nik added a comment.

Addressed comments.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Basic/SourceManager.cpp
  lib/Lex/PPDirectives.cpp
  test/Index/preamble-cyclic-include.cpp


Index: test/Index/preamble-cyclic-include.cpp
===
--- /dev/null
+++ test/Index/preamble-cyclic-include.cpp
@@ -0,0 +1,9 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test 
-test-annotate-tokens=%s:5:1:10:1 %s 2>&1 | FileCheck %s
+// CHECK-NOT: error: unterminated conditional directive
+// CHECK-NOT: Skipping: [4:1 - 8:7]
+// CHECK: error: main file cannot be included recursively when building a 
preamble
+#ifndef A_H
+#define A_H
+#  include "preamble-cyclic-include.cpp"
+int bar();
+#endif
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1890,6 +1890,18 @@
 return;
   }
 
+  // Check for circular inclusion of the main file.
+  // We can't generate a consistent preamble with regard to the conditional
+  // stack if the main file is included again as due to the preamble bounds
+  // some directives (e.g. #endif of a header guard) will never be seen.
+  // Since this will lead to confusing errors, avoid the inclusion.
+  if (File && PreambleConditionalStack.isRecording() &&
+  SourceMgr.translateFile(File) == SourceMgr.getMainFileID()) {
+Diag(FilenameTok.getLocation(),
+ diag::err_pp_including_mainfile_for_preamble);
+return;
+  }
+
   // Should we enter the source file? Set to false if either the source file is
   // known to have no effect beyond its effect on module visibility -- that is,
   // if it's got an include guard that is already defined or is a modular 
header
Index: lib/Basic/SourceManager.cpp
===
--- lib/Basic/SourceManager.cpp
+++ lib/Basic/SourceManager.cpp
@@ -1589,7 +1589,7 @@
 // as the main file.
 const FileEntry *MainFile = MainContentCache->OrigEntry;
 SourceFileName = llvm::sys::path::filename(SourceFile->getName());
-if (*SourceFileName == llvm::sys::path::filename(MainFile->getName())) 
{
+if (MainFile && *SourceFileName == 
llvm::sys::path::filename(MainFile->getName())) {
   SourceFileUID = getActualFileUID(SourceFile);
   if (SourceFileUID) {
 if (Optional MainFileUID =
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -422,6 +422,8 @@
   "did not find header '%0' in framework '%1' (loaded from '%2')">;
 def err_pp_error_opening_file : Error<
   "error opening file '%0': %1">, DefaultFatal;
+def err_pp_including_mainfile_for_preamble : Error<
+  "main file cannot be included recursively when building a preamble">;
 def err_pp_empty_filename : Error<"empty filename">;
 def err_pp_include_too_deep : Error<"#include nested too deeply">;
 def err_pp_expects_filename : Error<"expected \"FILENAME\" or ">;


Index: test/Index/preamble-cyclic-include.cpp
===
--- /dev/null
+++ test/Index/preamble-cyclic-include.cpp
@@ -0,0 +1,9 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-annotate-tokens=%s:5:1:10:1 %s 2>&1 | FileCheck %s
+// CHECK-NOT: error: unterminated conditional directive
+// CHECK-NOT: Skipping: [4:1 - 8:7]
+// CHECK: error: main file cannot be included recursively when building a preamble
+#ifndef A_H
+#define A_H
+#  include "preamble-cyclic-include.cpp"
+int bar();
+#endif
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1890,6 +1890,18 @@
 return;
   }
 
+  // Check for circular inclusion of the main file.
+  // We can't generate a consistent preamble with regard to the conditional
+  // stack if the main file is included again as due to the preamble bounds
+  // some directives (e.g. #endif of a header guard) will never be seen.
+  // Since this will lead to confusing errors, avoid the inclusion.
+  if (File && PreambleConditionalStack.isRecording() &&
+  SourceMgr.translateFile(File) == SourceMgr.getMainFileID()) {
+Diag(FilenameTok.getLocation(),
+ diag::err_pp_including_mainfile_for_preamble);
+return;
+  }
+
   // Should we enter the source file? Set to false if either the source file is
   // known to have no effect beyond its effect on module visibility -- that is,
   // if it's got an include guard that is already defined or is a modular header
Index: lib/Basic/SourceManager.cpp
=

[PATCH] D53866: [Preamble] Stop generating preamble for circular #includes

2019-02-14 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik marked an inline comment as done.
nik added inline comments.



Comment at: lib/Basic/SourceManager.cpp:1592
 SourceFileName = llvm::sys::path::filename(SourceFile->getName());
-if (*SourceFileName == llvm::sys::path::filename(MainFile->getName())) 
{
+if (MainFile && *SourceFileName == 
llvm::sys::path::filename(MainFile->getName())) {
   SourceFileUID = getActualFileUID(SourceFile);

I've added the nullptr check here as LibclangReparseTest.ReparseWithModule 
fails/crashes otherwise.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866



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

[PATCH] D53866: [Preamble] Stop circular inclusion of main file when building preamble

2019-02-14 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866



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

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2019-02-14 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41005/new/

https://reviews.llvm.org/D41005



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

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

2019-02-19 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.
Herald added a project: clang.

>> For filtering in StoredDiagnosticConsumer one needs to pass the new bool 
>> everywhere along where "bool CaptureDiagnostics" is already passed on (to 
>> end up in the StoredDiagnosticConsumer constructor) . Also, 
>> ASTUnit::CaptureDiagnostics is then not enough anymore since the new bool is 
>> also needed in getMainBufferWithPrecompiledPreamble(). One could also (2) 
>> convert  "bool CaptureDiagnostics" to an enum with enumerators like 
>> CaptureNothing, CaptureAll, CaptureAllWithoutNonErrorsFromIncludes to make 
>> this a bit less invasive.
>>  If changing clang's diagnostic interface should be avoided, I tend to go 
>> with (2). Ilya?
> 
> Yeah, LG. The changes in the `ASTUnit` look strictly better than changes in 
> clang - the latter seems to already provide enough to do the filtering.
>  If you avoid changing the `StoredDiagnosticConsumer` (or writing a filtering 
> wrapper for `DiagnosticConsumer`), you'll end up having some diagnostics 
> inside headers generated **after** preamble was built, right?

If there is some #include after the first declaration, possibly. Why is that 
relevant?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D48116/new/

https://reviews.llvm.org/D48116



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

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

2019-02-19 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 187318.
nik added a comment.
Herald added a subscriber: jdoerfert.

OK, filtering happens now in FilterAndStoreDiagnosticConsumer, the former
StoredDiagnosticConsumer.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D48116/new/

https://reviews.llvm.org/D48116

Files:
  include/clang-c/Index.h
  include/clang/Frontend/ASTUnit.h
  lib/Frontend/ASTUnit.cpp
  test/Index/ignore-warnings-from-headers.cpp
  test/Index/ignore-warnings-from-headers.h
  tools/c-index-test/c-index-test.c
  tools/c-index-test/core_main.cpp
  tools/libclang/CIndex.cpp
  tools/libclang/Indexing.cpp
  unittests/Frontend/ASTUnitTest.cpp
  unittests/Frontend/PCHPreambleTest.cpp

Index: unittests/Frontend/PCHPreambleTest.cpp
===
--- unittests/Frontend/PCHPreambleTest.cpp
+++ unittests/Frontend/PCHPreambleTest.cpp
@@ -96,8 +96,8 @@
 FileManager *FileMgr = new FileManager(FSOpts, VFS);
 
 std::unique_ptr AST = ASTUnit::LoadFromCompilerInvocation(
-  CI, PCHContainerOpts, Diags, FileMgr, false, false,
-  /*PrecompilePreambleAfterNParses=*/1);
+CI, PCHContainerOpts, Diags, FileMgr, false, CaptureDiagsKind::None,
+/*PrecompilePreambleAfterNParses=*/1);
 return AST;
   }
 
Index: unittests/Frontend/ASTUnitTest.cpp
===
--- unittests/Frontend/ASTUnitTest.cpp
+++ unittests/Frontend/ASTUnitTest.cpp
@@ -51,8 +51,8 @@
 PCHContainerOps = std::make_shared();
 
 return ASTUnit::LoadFromCompilerInvocation(
-CInvok, PCHContainerOps, Diags, FileMgr, false, false, 0, TU_Complete,
-false, false, isVolatile);
+CInvok, PCHContainerOps, Diags, FileMgr, false, CaptureDiagsKind::None,
+0, TU_Complete, false, false, isVolatile);
   }
 };
 
Index: tools/libclang/Indexing.cpp
===
--- tools/libclang/Indexing.cpp
+++ tools/libclang/Indexing.cpp
@@ -443,10 +443,14 @@
   if (CXXIdx->isOptEnabled(CXGlobalOpt_ThreadBackgroundPriorityForIndexing))
 setThreadBackgroundPriority();
 
-  bool CaptureDiagnostics = !Logger::isLoggingEnabled();
+  CaptureDiagsKind CaptureDiagnostics = CaptureDiagsKind::All;
+  if (TU_options & CXTranslationUnit_IgnoreNonErrorsFromIncludedFiles)
+CaptureDiagnostics = CaptureDiagsKind::AllWithoutNonErrorsFromIncludes;
+  if (Logger::isLoggingEnabled())
+CaptureDiagnostics = CaptureDiagsKind::None;
 
   CaptureDiagnosticConsumer *CaptureDiag = nullptr;
-  if (CaptureDiagnostics)
+  if (CaptureDiagnostics != CaptureDiagsKind::None)
 CaptureDiag = new CaptureDiagnosticConsumer();
 
   // Configure the diagnostics.
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3338,7 +3338,7 @@
   ASTUnit::LoadEverything, Diags,
   FileSystemOpts, /*UseDebugInfo=*/false,
   CXXIdx->getOnlyLocalDecls(), None,
-  /*CaptureDiagnostics=*/true,
+  CaptureDiagsKind::All,
   /*AllowPCHWithCompilerErrors=*/true,
   /*UserFilesAreVolatile=*/true);
   *out_TU = MakeCXTranslationUnit(CXXIdx, std::move(AU));
@@ -3411,6 +3411,10 @@
   if (options & CXTranslationUnit_KeepGoing)
 Diags->setSuppressAfterFatalError(false);
 
+  CaptureDiagsKind CaptureDiagnostics = CaptureDiagsKind::All;
+  if (options & CXTranslationUnit_IgnoreNonErrorsFromIncludedFiles)
+CaptureDiagnostics = CaptureDiagsKind::AllWithoutNonErrorsFromIncludes;
+
   // Recover resources if we crash before exiting this function.
   llvm::CrashRecoveryContextCleanupRegistrar >
@@ -3488,7 +3492,7 @@
   Args->data(), Args->data() + Args->size(),
   CXXIdx->getPCHContainerOperations(), Diags,
   CXXIdx->getClangResourcesPath(), CXXIdx->getOnlyLocalDecls(),
-  /*CaptureDiagnostics=*/true, *RemappedFiles.get(),
+  CaptureDiagnostics, *RemappedFiles.get(),
   /*RemappedFilesKeepOriginalName=*/true, PrecompilePreambleAfterNParses,
   TUKind, CacheCodeCompletionResults, IncludeBriefCommentsInCodeCompletion,
   /*AllowPCHWithCompilerErrors=*/true, SkipFunctionBodies, SingleFileParse,
Index: tools/c-index-test/core_main.cpp
===
--- tools/c-index-test/core_main.cpp
+++ tools/c-index-test/core_main.cpp
@@ -264,7 +264,7 @@
   modulePath, *pchRdr, ASTUnit::LoadASTOnly, Diags,
   FileSystemOpts, /*UseDebugInfo=*/false,
   /*OnlyLocalDecls=*/true, None,
-  /*CaptureDiagnostics=*/false,
+  CaptureDiagsKind::None,
   /*AllowPCHWithCompilerErrors=*/true,
   /*UserFilesAreVolatile=*/false);
   if (!AU) {
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -88,6 +88,8 @@
 options |= CXTrans

[PATCH] D58501: [libclang] Fix CXTranslationUnit_KeepGoing

2019-02-21 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik created this revision.
Herald added subscribers: cfe-commits, arphaman.
Herald added a project: clang.

Since

  commit 56f5487e4387a69708f70724d00e9e076153
  [modules] Round-trip -Werror flag through explicit module build.

the behavior of CXTranslationUnit_KeepGoing changed:
Unresolved #includes are fatal errors again. As a consequence, some
templates are not instantiated and lead to confusing errors.

Revert to the old behavior: With CXTranslationUnit_KeepGoing fatal
errors are mapped to errors.


Repository:
  rC Clang

https://reviews.llvm.org/D58501

Files:
  include/clang/Basic/Diagnostic.h
  lib/Basic/DiagnosticIDs.cpp
  test/Index/Inputs/keep-going-template-instantiations.h
  test/Index/keep-going-template-instantiations.cpp
  test/Index/keep-going.cpp
  tools/libclang/CIndex.cpp
  unittests/Basic/DiagnosticTest.cpp

Index: unittests/Basic/DiagnosticTest.cpp
===
--- unittests/Basic/DiagnosticTest.cpp
+++ unittests/Basic/DiagnosticTest.cpp
@@ -46,13 +46,13 @@
   EXPECT_FALSE(Diags.hasUnrecoverableErrorOccurred());
 }
 
-// Check that SuppressAfterFatalError works as intended
-TEST(DiagnosticTest, suppressAfterFatalError) {
-  for (unsigned Suppress = 0; Suppress != 2; ++Suppress) {
+// Check that FatalsAsError works as intended
+TEST(DiagnosticTest, fatalAsError) {
+  for (unsigned FatalAsError = 0; FatalAsError != 2; ++FatalAsError) {
 DiagnosticsEngine Diags(new DiagnosticIDs(),
 new DiagnosticOptions,
 new IgnoringDiagConsumer());
-Diags.setSuppressAfterFatalError(Suppress);
+Diags.setFatalsAsError(FatalAsError);
 
 // Diag that would set UnrecoverableErrorOccurred and ErrorOccurred.
 Diags.Report(diag::err_cannot_open_file) << "file" << "error";
@@ -62,13 +62,13 @@
 Diags.Report(diag::warn_mt_message) << "warning";
 
 EXPECT_TRUE(Diags.hasErrorOccurred());
-EXPECT_TRUE(Diags.hasFatalErrorOccurred());
+EXPECT_EQ(Diags.hasFatalErrorOccurred(), FatalAsError ? 0u : 1u);
 EXPECT_TRUE(Diags.hasUncompilableErrorOccurred());
 EXPECT_TRUE(Diags.hasUnrecoverableErrorOccurred());
 
 // The warning should be emitted and counted only if we're not suppressing
 // after fatal errors.
-EXPECT_EQ(Diags.getNumWarnings(), Suppress ? 0u : 1u);
+EXPECT_EQ(Diags.getNumWarnings(), FatalAsError);
   }
 }
 
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3409,7 +3409,7 @@
 Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions));
 
   if (options & CXTranslationUnit_KeepGoing)
-Diags->setSuppressAfterFatalError(false);
+Diags->setFatalsAsError(true);
 
   // Recover resources if we crash before exiting this function.
   llvm::CrashRecoveryContextCleanupRegistrar
+
+// RUN: env CINDEXTEST_KEEP_GOING=1 c-index-test -test-load-source none -I%S/Inputs %s 2>&1 | FileCheck %s
+// CHECK-NOT: error: expected class name
Index: test/Index/Inputs/keep-going-template-instantiations.h
===
--- /dev/null
+++ test/Index/Inputs/keep-going-template-instantiations.h
@@ -0,0 +1,3 @@
+template struct c {};
+using d = c;
+struct foo : public d {};
Index: lib/Basic/DiagnosticIDs.cpp
===
--- lib/Basic/DiagnosticIDs.cpp
+++ lib/Basic/DiagnosticIDs.cpp
@@ -481,6 +481,11 @@
   Result = diag::Severity::Fatal;
   }
 
+  // If explicitly requested, map fatal errors to errors.
+  if (Result == diag::Severity::Fatal &&
+  Diag.CurDiagID != diag::fatal_too_many_errors && Diag.FatalsAsError)
+Result = diag::Severity::Error;
+
   // Custom diagnostics always are emitted in system headers.
   bool ShowInSystemHeader =
   !GetDiagInfo(DiagID) || GetDiagInfo(DiagID)->WarnShowInSystemHeader;
@@ -660,7 +665,7 @@
 
   // If a fatal error has already been emitted, silence all subsequent
   // diagnostics.
-  if (Diag.FatalErrorOccurred && Diag.SuppressAfterFatalError) {
+  if (Diag.FatalErrorOccurred) {
 if (DiagLevel >= DiagnosticIDs::Error &&
 Diag.Client->IncludeInDiagnosticCounts()) {
   ++Diag.NumErrors;
Index: include/clang/Basic/Diagnostic.h
===
--- include/clang/Basic/Diagnostic.h
+++ include/clang/Basic/Diagnostic.h
@@ -209,8 +209,8 @@
   // Used by __extension__
   unsigned char AllExtensionsSilenced = 0;
 
-  // Suppress diagnostics after a fatal error?
-  bool SuppressAfterFatalError = true;
+  // Treat fatal errors like errors.
+  bool FatalsAsError = false;
 
   // Suppress all diagnostics.
   bool SuppressAllDiagnostics = false;
@@ -614,9 +614,11 @@
   void setErrorsAsFatal(bool Val) { GetCurDiagState()->ErrorsAsFatal = Val; }
   bool getErrorsAsFatal() const { return GetCurDiagState()->ErrorsAsFatal; }

[PATCH] D58501: [libclang] Fix CXTranslationUnit_KeepGoing

2019-02-21 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 187765.
nik added a comment.

Fixed minor typo.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58501/new/

https://reviews.llvm.org/D58501

Files:
  include/clang/Basic/Diagnostic.h
  lib/Basic/DiagnosticIDs.cpp
  test/Index/Inputs/keep-going-template-instantiations.h
  test/Index/keep-going-template-instantiations.cpp
  test/Index/keep-going.cpp
  tools/libclang/CIndex.cpp
  unittests/Basic/DiagnosticTest.cpp

Index: unittests/Basic/DiagnosticTest.cpp
===
--- unittests/Basic/DiagnosticTest.cpp
+++ unittests/Basic/DiagnosticTest.cpp
@@ -46,13 +46,13 @@
   EXPECT_FALSE(Diags.hasUnrecoverableErrorOccurred());
 }
 
-// Check that SuppressAfterFatalError works as intended
-TEST(DiagnosticTest, suppressAfterFatalError) {
-  for (unsigned Suppress = 0; Suppress != 2; ++Suppress) {
+// Check that FatalsAsError works as intended
+TEST(DiagnosticTest, fatalsAsError) {
+  for (unsigned FatalsAsError = 0; FatalsAsError != 2; ++FatalsAsError) {
 DiagnosticsEngine Diags(new DiagnosticIDs(),
 new DiagnosticOptions,
 new IgnoringDiagConsumer());
-Diags.setSuppressAfterFatalError(Suppress);
+Diags.setFatalsAsError(FatalsAsError);
 
 // Diag that would set UnrecoverableErrorOccurred and ErrorOccurred.
 Diags.Report(diag::err_cannot_open_file) << "file" << "error";
@@ -62,13 +62,13 @@
 Diags.Report(diag::warn_mt_message) << "warning";
 
 EXPECT_TRUE(Diags.hasErrorOccurred());
-EXPECT_TRUE(Diags.hasFatalErrorOccurred());
+EXPECT_EQ(Diags.hasFatalErrorOccurred(), FatalsAsError ? 0u : 1u);
 EXPECT_TRUE(Diags.hasUncompilableErrorOccurred());
 EXPECT_TRUE(Diags.hasUnrecoverableErrorOccurred());
 
 // The warning should be emitted and counted only if we're not suppressing
 // after fatal errors.
-EXPECT_EQ(Diags.getNumWarnings(), Suppress ? 0u : 1u);
+EXPECT_EQ(Diags.getNumWarnings(), FatalsAsError);
   }
 }
 
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3409,7 +3409,7 @@
 Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions));
 
   if (options & CXTranslationUnit_KeepGoing)
-Diags->setSuppressAfterFatalError(false);
+Diags->setFatalsAsError(true);
 
   // Recover resources if we crash before exiting this function.
   llvm::CrashRecoveryContextCleanupRegistrar
+
+// RUN: env CINDEXTEST_KEEP_GOING=1 c-index-test -test-load-source none -I%S/Inputs %s 2>&1 | FileCheck %s
+// CHECK-NOT: error: expected class name
Index: test/Index/Inputs/keep-going-template-instantiations.h
===
--- /dev/null
+++ test/Index/Inputs/keep-going-template-instantiations.h
@@ -0,0 +1,3 @@
+template struct c {};
+using d = c;
+struct foo : public d {};
Index: lib/Basic/DiagnosticIDs.cpp
===
--- lib/Basic/DiagnosticIDs.cpp
+++ lib/Basic/DiagnosticIDs.cpp
@@ -481,6 +481,11 @@
   Result = diag::Severity::Fatal;
   }
 
+  // If explicitly requested, map fatal errors to errors.
+  if (Result == diag::Severity::Fatal &&
+  Diag.CurDiagID != diag::fatal_too_many_errors && Diag.FatalsAsError)
+Result = diag::Severity::Error;
+
   // Custom diagnostics always are emitted in system headers.
   bool ShowInSystemHeader =
   !GetDiagInfo(DiagID) || GetDiagInfo(DiagID)->WarnShowInSystemHeader;
@@ -660,7 +665,7 @@
 
   // If a fatal error has already been emitted, silence all subsequent
   // diagnostics.
-  if (Diag.FatalErrorOccurred && Diag.SuppressAfterFatalError) {
+  if (Diag.FatalErrorOccurred) {
 if (DiagLevel >= DiagnosticIDs::Error &&
 Diag.Client->IncludeInDiagnosticCounts()) {
   ++Diag.NumErrors;
Index: include/clang/Basic/Diagnostic.h
===
--- include/clang/Basic/Diagnostic.h
+++ include/clang/Basic/Diagnostic.h
@@ -209,8 +209,8 @@
   // Used by __extension__
   unsigned char AllExtensionsSilenced = 0;
 
-  // Suppress diagnostics after a fatal error?
-  bool SuppressAfterFatalError = true;
+  // Treat fatal errors like errors.
+  bool FatalsAsError = false;
 
   // Suppress all diagnostics.
   bool SuppressAllDiagnostics = false;
@@ -614,9 +614,11 @@
   void setErrorsAsFatal(bool Val) { GetCurDiagState()->ErrorsAsFatal = Val; }
   bool getErrorsAsFatal() const { return GetCurDiagState()->ErrorsAsFatal; }
 
-  /// When set to true (the default), suppress further diagnostics after
-  /// a fatal error.
-  void setSuppressAfterFatalError(bool Val) { SuppressAfterFatalError = Val; }
+  /// \brief When set to true, any fatal error reported is made an error.
+  ///
+  /// This setting takes precedence over the setErrorsAsFatal setting above.
+  void setFatalsAsEr

[PATCH] D53866: [Preamble] Stop circular inclusion of main file when building preamble

2019-02-21 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53866/new/

https://reviews.llvm.org/D53866



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

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

2019-02-21 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41005/new/

https://reviews.llvm.org/D41005



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

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

2019-02-21 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D48116/new/

https://reviews.llvm.org/D48116



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

[PATCH] D37577: [libclang] add 'clang_getCursorTLSKind'

2017-09-12 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added inline comments.



Comment at: include/clang-c/Index.h:2840
+/**
+ * \brief Describe the "TLS kind" of the declaration referred to by a cursor.
+ */

I was wondering what "TLS" is and had to look it up. If not adapting the 
function and enum name, then maybe at least write it out once in the 
documentation.



https://reviews.llvm.org/D37577



___
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-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] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS

2017-09-13 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping


https://reviews.llvm.org/D37554



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

[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS

2017-09-20 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping 2


https://reviews.llvm.org/D37554



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

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

2019-06-04 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D48116/new/

https://reviews.llvm.org/D48116



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

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

2019-06-06 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 203306.
nik marked 2 inline comments as done.
nik added a comment.

Addressed inline comments.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61487/new/

https://reviews.llvm.org/D61487

Files:
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tidy/plugin/ClangTidyPlugin.cpp
  test/clang-tidy/basic.cpp
  test/clang-tidy/nolint-plugin.cpp
  test/clang-tidy/nolintnextline-plugin.cpp

Index: test/clang-tidy/nolintnextline-plugin.cpp
===
--- /dev/null
+++ test/clang-tidy/nolintnextline-plugin.cpp
@@ -0,0 +1,48 @@
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s
+
+class A { A(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE
+class B { B(int i); };
+
+// NOLINTNEXTLINE(for-some-other-check)
+class C { C(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE(*)
+class C1 { C1(int i); };
+
+// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all
+class C2 { C2(int i); };
+
+// NOLINTNEXTLINE(google-explicit-constructor)
+class C3 { C3(int i); };
+
+// NOLINTNEXTLINE(some-check, google-explicit-constructor)
+class C4 { C4(int i); };
+
+// NOLINTNEXTLINE without-brackets-skip-all, another-check
+class C5 { C5(int i); };
+
+
+// NOLINTNEXTLINE
+
+class D { D(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE
+//
+class E { E(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+#define MACRO(X) class X { X(int i); };
+MACRO(F)
+// CHECK: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+// NOLINTNEXTLINE
+MACRO(G)
+
+#define MACRO_NOARG class H { H(int i); };
+// NOLINTNEXTLINE
+MACRO_NOARG
+
Index: test/clang-tidy/nolint-plugin.cpp
===
--- /dev/null
+++ test/clang-tidy/nolint-plugin.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: static-analyzer
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,google-explicit-constructor,clang-diagnostic-unused-variable,clang-analyzer-core.UndefinedBinaryOperatorResult' -Wunused-variable -I%S/Inputs/nolint 2>&1 | FileCheck %s
+
+#include "trigger_warning.h"
+void I(int& Out) {
+  int In;
+  A1(In, Out);
+}
+// CHECK-NOT: trigger_warning.h:{{.*}} warning
+// CHECK-NOT: :[[@LINE-4]]:{{.*}} note
+
+class A { A(int i); };
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class B { B(int i); }; // NOLINT
+
+class C { C(int i); }; // NOLINT(for-some-other-check)
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class C1 { C1(int i); }; // NOLINT(*)
+
+class C2 { C2(int i); }; // NOLINT(not-closed-bracket-is-treated-as-skip-all
+
+class C3 { C3(int i); }; // NOLINT(google-explicit-constructor)
+
+class C4 { C4(int i); }; // NOLINT(some-check, google-explicit-constructor)
+
+class C5 { C5(int i); }; // NOLINT without-brackets-skip-all, another-check
+
+void f() {
+  int i;
+// CHECK-DAG: :[[@LINE-1]]:7: warning: unused variable 'i' [-Wunused-variable]
+//  31:7: warning: unused variable 'i' [-Wunused-variable]
+//  int j; // NOLINT
+//  int k; // NOLINT(clang-diagnostic-unused-variable)
+}
+
+#define MACRO(X) class X { X(int i); };
+MACRO(D)
+// CHECK-DAG: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+MACRO(E) // NOLINT
+
+#define MACRO_NOARG class F { F(int i); };
+MACRO_NOARG // NOLINT
+
+#define MACRO_NOLINT class G { G(int i); }; // NOLINT
+MACRO_NOLINT
+
+#define DOUBLE_MACRO MACRO(H) // NOLINT
+DOUBLE_MACRO
Index: test/clang-tidy/basic.cpp
===
--- test/clang-tidy/basic.cpp
+++ test/clang-tidy/basic.cpp
@@ -1,4 +1,5 @@
 // RUN: clang-tidy %s -checks='-*,llvm-namespace-comment' -- | FileCheck %s
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,llvm-namespace-comment' 2>&1 | FileCheck %s
 
 namespace i {
 }
Index: clang-tidy/plugin/ClangTidyPlugin.cpp
===
--- clang-tidy/plugin/ClangTidyPlugin.cpp
+++ clang-tidy/plugin/ClangTidyPlugin.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "../ClangTidy.h"
+#include "../ClangTidyDiagnosticConsumer.h"
 #include "../ClangTidyForceLinker.h"
 #inclu

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

2019-06-06 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik marked 4 inline comments as done.
nik added inline comments.



Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:207
   CheckNamesByDiagnosticID.try_emplace(ID, CheckName);
+  CustomDiagIdParamsByDiagnosticID.try_emplace(
+  ID, CustomDiagIdParams(Level, FormatString));

gribozavr wrote:
> Did you consider adding this query API to DiagnosticIDs instead? To me it 
> looks weird that creation of custom diag IDs is handled by one class 
> (DiagnosticIDs), and queries -- by another (ClangTidyContext).
Huch, that query API is already there. For some reason, I've missed it.

Good catch, thanks! :)



Comment at: test/clang-tidy/nolintnextline-plugin.cpp:47
+
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin 
-Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang 
-checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s

gribozavr wrote:
> Why not on the first line?
As this file is based on test/clang-tidy/nolint.cpp, I left it at is. But 
having it at top looks more common, so I've moved it there now.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61487/new/

https://reviews.llvm.org/D61487



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

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

2019-06-06 Thread Nikolai Kosjar via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362702: [clang-tidy] Make the plugin honor NOLINT (authored 
by nik, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61487/new/

https://reviews.llvm.org/D61487

Files:
  clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp
  clang-tools-extra/trunk/test/clang-tidy/basic.cpp
  clang-tools-extra/trunk/test/clang-tidy/nolint-plugin.cpp
  clang-tools-extra/trunk/test/clang-tidy/nolintnextline-plugin.cpp

Index: clang-tools-extra/trunk/test/clang-tidy/nolint-plugin.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/nolint-plugin.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/nolint-plugin.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: static-analyzer
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,google-explicit-constructor,clang-diagnostic-unused-variable,clang-analyzer-core.UndefinedBinaryOperatorResult' -Wunused-variable -I%S/Inputs/nolint 2>&1 | FileCheck %s
+
+#include "trigger_warning.h"
+void I(int& Out) {
+  int In;
+  A1(In, Out);
+}
+// CHECK-NOT: trigger_warning.h:{{.*}} warning
+// CHECK-NOT: :[[@LINE-4]]:{{.*}} note
+
+class A { A(int i); };
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class B { B(int i); }; // NOLINT
+
+class C { C(int i); }; // NOLINT(for-some-other-check)
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class C1 { C1(int i); }; // NOLINT(*)
+
+class C2 { C2(int i); }; // NOLINT(not-closed-bracket-is-treated-as-skip-all
+
+class C3 { C3(int i); }; // NOLINT(google-explicit-constructor)
+
+class C4 { C4(int i); }; // NOLINT(some-check, google-explicit-constructor)
+
+class C5 { C5(int i); }; // NOLINT without-brackets-skip-all, another-check
+
+void f() {
+  int i;
+// CHECK-DAG: :[[@LINE-1]]:7: warning: unused variable 'i' [-Wunused-variable]
+//  31:7: warning: unused variable 'i' [-Wunused-variable]
+//  int j; // NOLINT
+//  int k; // NOLINT(clang-diagnostic-unused-variable)
+}
+
+#define MACRO(X) class X { X(int i); };
+MACRO(D)
+// CHECK-DAG: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+MACRO(E) // NOLINT
+
+#define MACRO_NOARG class F { F(int i); };
+MACRO_NOARG // NOLINT
+
+#define MACRO_NOLINT class G { G(int i); }; // NOLINT
+MACRO_NOLINT
+
+#define DOUBLE_MACRO MACRO(H) // NOLINT
+DOUBLE_MACRO
Index: clang-tools-extra/trunk/test/clang-tidy/nolintnextline-plugin.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/nolintnextline-plugin.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/nolintnextline-plugin.cpp
@@ -0,0 +1,48 @@
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s
+
+class A { A(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE
+class B { B(int i); };
+
+// NOLINTNEXTLINE(for-some-other-check)
+class C { C(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE(*)
+class C1 { C1(int i); };
+
+// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all
+class C2 { C2(int i); };
+
+// NOLINTNEXTLINE(google-explicit-constructor)
+class C3 { C3(int i); };
+
+// NOLINTNEXTLINE(some-check, google-explicit-constructor)
+class C4 { C4(int i); };
+
+// NOLINTNEXTLINE without-brackets-skip-all, another-check
+class C5 { C5(int i); };
+
+
+// NOLINTNEXTLINE
+
+class D { D(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE
+//
+class E { E(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+#define MACRO(X) class X { X(int i); };
+MACRO(F)
+// CHECK: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+// NOLINTNEXTLINE
+MACRO(G)
+
+#define MACRO_NOARG class H { H(int i); };
+// NOLINTNEXTLINE
+MACRO_NOARG
+
Index: clang-tools-extra/trunk/test/clang-tidy/basic.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/basic.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/basic.cpp
@@ -1,4 +1,5 @@
 // RUN: clang-tidy %s -checks='-*,llvm-namespace-comment' -- | FileCheck %s
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -

[PATCH] D61486: [Basic] Introduce active dummy DiagnosticBuilder

2019-06-06 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

In D61486#1532272 , @gribozavr wrote:

> Is this patch still needed?


Nope, abandoning it now.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61486/new/

https://reviews.llvm.org/D61486



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

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

2019-06-06 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Thanks Dmitri!


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61487/new/

https://reviews.llvm.org/D61487



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

1 2 3 >

1 - 100 of 217 matches

Mail list logo