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

[PATCH] D80279: [libclang] Extend clang_Cursor_Evaluate().

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

nik added a comment.

@jkorous: Please submit. I can't as my svn account is still not ported to 
github... :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80279



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

[PATCH] D52079: [Sema] Do not load macros from preamble when LoadExternal is false.

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

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

I've bisected https://bugs.llvm.org/show_bug.cgi?id=42649 to this change. 
Reverting this change fixes the issue.

Please look into it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D52079



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

[PATCH] D53191: [clang] Introduce new completion context types

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

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

I've bisected https://bugs.llvm.org/show_bug.cgi?id=42646 to this change. 
Reverting it fixes the issue.

Please look into it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53191



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

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

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

nik added a comment.

In D63482#1567897 , @yvvan wrote:

> I have a commit access but I don't understand how am I supposed to commit 
> (haven't done that for a while). There's no clang svn repo anymore. Do you 
> know what's the current state of repositories and where to commit?


It's fairly simple with git nowadays: 
https://llvm.org/docs/GettingStarted.html#id15
(at least with mono repo)


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

https://reviews.llvm.org/D63482



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

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

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

nik added a comment.

In D63482#1567927 , @nik wrote:

> In D63482#1567897 , @yvvan wrote:
>
> > I have a commit access but I don't understand how am I supposed to commit 
> > (haven't done that for a while). There's no clang svn repo anymore. Do you 
> > know what's the current state of repositories and where to commit?
>
>
> It's fairly simple with git nowadays: 
> https://llvm.org/docs/GettingStarted.html#id15
>  (at least with mono repo)


Ops, correct I've meant 
https://llvm.org/docs/GettingStarted.html#for-developers-to-commit-changes-from-git


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

https://reviews.llvm.org/D63482



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

[PATCH] D63821: [clangd] Added C++ API code for semantic highlighting

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

nik added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.h:60
+  // Called by ClangdServer when some \p Highlightings for \p File are ready.
+  virtual void onHighlightingsReady(PathRef File,
+ std::vector Highlightings) 
= 0;

jvikstrom wrote:
> hokein wrote:
> > we may add this interface to the existing `DiagnosticsConsumer`.
> Probably want to rename `DiagnosticsConsumer` as well, can't come up with a 
> good name though. Any suggestions? 
One could summarize diagnostics and highlightings as annotations, so maybe 
FileAnnotationsConsumer or DocumentAnnotationsConsumer? Not sure how 
onFileUpdated() fits into this...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63821



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

[PATCH] D63763: [clang-tidy] Update documentation for Qt Creator integration.

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG181f252d5373: [clang-tidy] Update documentation for Qt 
Creator integration. (authored by nik).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63763

Files:
  clang-tools-extra/docs/clang-tidy/Integrations.rst


Index: clang-tools-extra/docs/clang-tidy/Integrations.rst
===
--- clang-tools-extra/docs/clang-tidy/Integrations.rst
+++ clang-tools-extra/docs/clang-tidy/Integrations.rst
@@ -32,7 +32,7 @@
 
+--++-+--+-+--+
 |KDevelop IDE  | \-\|  
 \+\   |   \+\| \+\ 
|   \+\|
 
+--++-+--+-+--+
-|Qt Creator IDE| \+\|  
 \+\   |   \-\| \-\ 
|   \+\|
+|Qt Creator IDE| \+\|  
 \+\   |   \-\| \+\ 
|   \+\|
 
+--++-+--+-+--+
 |ReSharper C++ for Visual Studio   | \+\|  
 \+\   |   \-\| \+\ 
|   \+\|
 
+--++-+--+-+--+
@@ -65,11 +65,13 @@
 
 .. _QtCreator: https://www.qt.io/
 .. _Clang Code Model: https://doc.qt.io/qtcreator/creator-clang-codemodel.html
+.. _Clang Tools: https://doc.qt.io/qtcreator/creator-clang-tools.html
 
 QtCreator_ 4.6 integrates :program:`clang-tidy` warnings into the editor
 diagnostics under the `Clang Code Model`_. To employ :program:`clang-tidy`
 inspection in QtCreator, you need to create a copy of one of the presets and
-choose the checks to be performed in the Clang Code Model Warnings menu.
+choose the checks to be performed. Since QtCreator 4.7 project-wide analysis is
+possible with the `Clang Tools`_ analyzer.
 
 .. _MS Visual Studio: https://visualstudio.microsoft.com/
 .. _ReSharper C++: 
https://www.jetbrains.com/help/resharper/Clang_Tidy_Integration.html


Index: clang-tools-extra/docs/clang-tidy/Integrations.rst
===
--- clang-tools-extra/docs/clang-tidy/Integrations.rst
+++ clang-tools-extra/docs/clang-tidy/Integrations.rst
@@ -32,7 +32,7 @@
 +--++-+--+-+--+
 |KDevelop IDE  | \-\|   \+\   |   \+\| \+\ |   \+\|
 +--++-+--+-+--+
-|Qt Creator IDE| \+\|   \+\   |   \-\| \-\ |   \+\|
+|Qt Creator IDE| \+\|   \+\   |   \-\| \+\ |   \+\|
 +--++-+--+-+--+
 |ReSharper C++ for Visual Studio   | \+\|   \+\   |   \-\| \+\ |   \+\|
 +--++-+--+-+--+
@@ -65,11 +65,13 @@
 
 .. _QtCreator: https://www.qt.io/
 .. _Clang Code Model: https://doc.qt.io/qtcreator/creator-clang-codemodel.html
+..

[PATCH] D63763: [clang-tidy] Update documentation for Qt Creator integration.

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

nik added a comment.

In D63763#1557398 , @gribozavr wrote:

> LGTM, assuming you know what's new in QtCreator.


Sure, I'm involved in Qt Creator development :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63763



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

[PATCH] D63763: [clang-tidy] Update documentation for Qt Creator integration.

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

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63763

Files:
  clang-tools-extra/docs/clang-tidy/Integrations.rst


Index: clang-tools-extra/docs/clang-tidy/Integrations.rst
===
--- clang-tools-extra/docs/clang-tidy/Integrations.rst
+++ clang-tools-extra/docs/clang-tidy/Integrations.rst
@@ -32,7 +32,7 @@
 
+--++-+--+-+--+
 |KDevelop IDE  | \-\|  
 \+\   |   \+\| \+\ 
|   \+\|
 
+--++-+--+-+--+
-|Qt Creator IDE| \+\|  
 \+\   |   \-\| \-\ 
|   \+\|
+|Qt Creator IDE| \+\|  
 \+\   |   \-\| \+\ 
|   \+\|
 
+--++-+--+-+--+
 |ReSharper C++ for Visual Studio   | \+\|  
 \+\   |   \-\| \+\ 
|   \+\|
 
+--++-+--+-+--+
@@ -65,11 +65,13 @@
 
 .. _QtCreator: https://www.qt.io/
 .. _Clang Code Model: https://doc.qt.io/qtcreator/creator-clang-codemodel.html
+.. _Clang Tools: https://doc.qt.io/qtcreator/creator-clang-tools.html
 
 QtCreator_ 4.6 integrates :program:`clang-tidy` warnings into the editor
 diagnostics under the `Clang Code Model`_. To employ :program:`clang-tidy`
 inspection in QtCreator, you need to create a copy of one of the presets and
-choose the checks to be performed in the Clang Code Model Warnings menu.
+choose the checks to be performed. Since QtCreator 4.7 project-wide analysis is
+possible with the `Clang Tools`_ analyzer.
 
 .. _MS Visual Studio: https://visualstudio.microsoft.com/
 .. _ReSharper C++: 
https://www.jetbrains.com/help/resharper/Clang_Tidy_Integration.html


Index: clang-tools-extra/docs/clang-tidy/Integrations.rst
===
--- clang-tools-extra/docs/clang-tidy/Integrations.rst
+++ clang-tools-extra/docs/clang-tidy/Integrations.rst
@@ -32,7 +32,7 @@
 +--++-+--+-+--+
 |KDevelop IDE  | \-\|   \+\   |   \+\| \+\ |   \+\|
 +--++-+--+-+--+
-|Qt Creator IDE| \+\|   \+\   |   \-\| \-\ |   \+\|
+|Qt Creator IDE| \+\|   \+\   |   \-\| \+\ |   \+\|
 +--++-+--+-+--+
 |ReSharper C++ for Visual Studio   | \+\|   \+\   |   \-\| \+\ |   \+\|
 +--++-+--+-+--+
@@ -65,11 +65,13 @@
 
 .. _QtCreator: https://www.qt.io/
 .. _Clang Code Model: https://doc.qt.io/qtcreator/creator-clang-codemodel.html
+.. _Clang Tools: https://doc.qt.io/qtcreator/creator-clang-tools.html
 
 QtCreator_ 4.6 integrates :program:`clang-tidy` warnings into the editor

[PATCH] D61842: [clangd] [WIP] [Not ready for review] Semantic highlighting

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

nik added a comment.

Some general notes regarding the current state of this patch as I was testing 
it with a language client:

- It still applies for current trunk, except for a *Tests* file.
- Note that a VersionedTextDocumentIdentifier has to be used, e.g. a "version" 
must be included in the response. This helped to get it working for now (not a 
proper fix):



  --- clang-tools-extra/clangd/Protocol.cpp
  +++ clang-tools-extra/clangd/Protocol.cpp
  @@ -79,11 +79,11 @@ llvm::json::Value toJSON(const URIForFile ) { return 
U.uri(); }
   llvm::raw_ostream <<(llvm::raw_ostream , const URIForFile ) {
 return OS << U.uri();
   }
   
   llvm::json::Value toJSON(const TextDocumentIdentifier ) {
  -  return llvm::json::Object{{"uri", R.uri}};
  +  return llvm::json::Object{{"uri", R.uri}, {"version", nullptr}};
   }
   
   bool fromJSON(const llvm::json::Value , TextDocumentIdentifier ) {
 llvm::json::ObjectMapper O(Params);
 return O && O.map("uri", R.uri);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61842



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

[PATCH] D63331: [clangd] WIP/RFC: Prototype for semantic highlighting proposal

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

nik added a comment.

In D63331#1543441 , @hokein wrote:

> Yeah, we definitely have interest in this feature, and our intern @jvikstrom 
> will work on this feature this summer.


"this summer" depends on the location, so can you concretize this? :)

> You probably missed the discussion 
>  on 
> clangd-dev mailing list, @nridge also has an unfinished prototype 
>  for semantic highlighting.

Yes, I wasn't aware that there is a clangd-dev mailing list and thus was not 
subscribed to it. Subscribed now.

I'm glad that I haven't put more work into this ;)

> To avoid multiple people working on the same feature, I think the plan is 
> that @jvikstrom will pick up the current stuff, and continue 
> working/improving it.

Great. The patch from @nridge is more advanced so I'll abandon this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63331



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

[PATCH] D63331: [clangd] WIP/RFC: Prototype for semantic highlighting proposal

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

nik added a comment.

Any interest in having the *proposal* implemented?

While there is a client implementation available in the theia ide 
(https://github.com/theia-ide/theia/pull/2332), I have only tested this against 
Qt Creator's client implementation (also work in progress).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63331



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

[PATCH] D63331: [clangd] WIP/RFC: Prototype for semantic highlighting proposal

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

nik created this revision.
nik added reviewers: ilya-biryukov, sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
javed.absar, mgorny.
Herald added a project: clang.

TODO: Is the visitor the right approach?
TODO: Add tests
TODO:   Test that only highlightings for main file are returned, but not 
headers.

...as specified at

  https://github.com/microsoft/vscode-languageserver-node/pull/367

See also

  https://github.com/microsoft/vscode-languageserver-node/issues/368

So far, this is an *incomplete* implementation providing highlighting
for identifiers of VarDecl/DeclRefExpr.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63331

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/ClangdUnit.cpp
  clang-tools-extra/clangd/ClangdUnit.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/thirdparty/base64/base64.cpp
  clang-tools-extra/clangd/thirdparty/base64/base64.h
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -537,8 +537,10 @@
   MockCompilationDatabase CDB(BuildDir, RelPathPrefix);
 
   IgnoreDiagnostics DiagConsumer;
+  HighlightingsConsumer HighlightConsumer;
   MockFSProvider FS;
-  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+  ClangdServer Server(CDB, FS, DiagConsumer, HighlightConsumer,
+  ClangdServer::optsForTest());
 
   // Fill the filesystem.
   auto FooCpp = testPath("src/foo.cpp");
@@ -1653,8 +1655,10 @@
 TEST(GoToInclude, All) {
   MockFSProvider FS;
   IgnoreDiagnostics DiagConsumer;
+  HighlightingsConsumer HighlightConsumer;
   MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+  ClangdServer Server(CDB, FS, DiagConsumer, HighlightConsumer,
+  ClangdServer::optsForTest());
 
   auto FooCpp = testPath("foo.cpp");
   const char *SourceContents = R"cpp(
@@ -1728,8 +1732,10 @@
   // good preamble.
   MockFSProvider FS;
   IgnoreDiagnostics DiagConsumer;
+  HighlightingsConsumer HighlightConsumer;
   MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+  ClangdServer Server(CDB, FS, DiagConsumer, HighlightConsumer,
+  ClangdServer::optsForTest());
 
   auto FooCpp = testPath("foo.cpp");
   // The trigger locations must be the same.
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -684,7 +684,9 @@
   } CaptureTUStatus;
   MockFSProvider FS;
   MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, FS, CaptureTUStatus, ClangdServer::optsForTest());
+  HighlightingsConsumer HighlightConsumer;
+  ClangdServer Server(CDB, FS, CaptureTUStatus, HighlightConsumer,
+  ClangdServer::optsForTest());
   Annotations Code("int m^ain () {}");
 
   // We schedule the following tasks in the queue:
Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -58,7 +58,8 @@
 class WorkspaceSymbolsTest : public ::testing::Test {
 public:
   WorkspaceSymbolsTest()
-  : Server(CDB, FSProvider, DiagConsumer, optsForTests()) {
+  : Server(CDB, FSProvider, DiagConsumer, HighlightConsumer,
+   optsForTests()) {
 // Make sure the test root directory is created.
 FSProvider.Files[testPath("unused")] = "";
 CDB.ExtraClangFlags = {"-xc++"};
@@ -68,6 +69,7 @@
   MockFSProvider FSProvider;
   MockCompilationDatabase CDB;
   IgnoreDiagnostics DiagConsumer;
+  HighlightingsConsumer HighlightConsumer;
   ClangdServer Server;
   int Limit = 0;
 
@@ -321,12 +323,14 @@
 class DocumentSymbolsTest : public ::testing::Test {
 public:
   DocumentSymbolsTest()
-  : Server(CDB,

[PATCH] D63193: [clangd] Fix typo in GUARDED_BY()

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363139: [clangd] Fix typo in GUARDED_BY() (authored by nik, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63193?vs=204250=204255#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63193

Files:
  clang-tools-extra/trunk/clangd/TUScheduler.cpp


Index: clang-tools-extra/trunk/clangd/TUScheduler.cpp
===
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp
@@ -273,7 +273,7 @@
   // The lifetime of the old/new ASTWorkers will overlap, but their handles
   // don't. When the old handle is destroyed, the old worker will stop 
reporting
   // diagnostics.
-  bool ReportDiagnostics = true; /* GUARDED_BY(DiagMu) */
+  bool ReportDiagnostics = true; /* GUARDED_BY(DiagsMu) */
 };
 
 /// A smart-pointer-like class that points to an active ASTWorker.


Index: clang-tools-extra/trunk/clangd/TUScheduler.cpp
===
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp
@@ -273,7 +273,7 @@
   // The lifetime of the old/new ASTWorkers will overlap, but their handles
   // don't. When the old handle is destroyed, the old worker will stop reporting
   // diagnostics.
-  bool ReportDiagnostics = true; /* GUARDED_BY(DiagMu) */
+  bool ReportDiagnostics = true; /* GUARDED_BY(DiagsMu) */
 };
 
 /// A smart-pointer-like class that points to an active ASTWorker.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63193: [clangd] Fix typo in GUARDED_BY()

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

nik created this revision.
nik added reviewers: ilya-biryukov, kadircet, sammccall.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, javed.absar.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63193

Files:
  clang-tools-extra/clangd/TUScheduler.cpp


Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -273,7 +273,7 @@
   // The lifetime of the old/new ASTWorkers will overlap, but their handles
   // don't. When the old handle is destroyed, the old worker will stop 
reporting
   // diagnostics.
-  bool ReportDiagnostics = true; /* GUARDED_BY(DiagMu) */
+  bool ReportDiagnostics = true; /* GUARDED_BY(DiagsMu) */
 };
 
 /// A smart-pointer-like class that points to an active ASTWorker.


Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -273,7 +273,7 @@
   // The lifetime of the old/new ASTWorkers will overlap, but their handles
   // don't. When the old handle is destroyed, the old worker will stop reporting
   // diagnostics.
-  bool ReportDiagnostics = true; /* GUARDED_BY(DiagMu) */
+  bool ReportDiagnostics = true; /* GUARDED_BY(DiagsMu) */
 };
 
 /// A smart-pointer-like class that points to an active ASTWorker.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63129: [clang-tidy] Fix invalid read on destruction

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363068: [clang-tidy] Fix invalid read on destruction 
(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/D63129/new/

https://reviews.llvm.org/D63129

Files:
  clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp


Index: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -44,18 +44,22 @@
 static const char DerefByRefResultName[] = "derefByRefResult";
 
 // shared matchers
-static const TypeMatcher AnyType = anything();
+static const TypeMatcher AnyType() { return anything(); }
 
-static const StatementMatcher IntegerComparisonMatcher =
-expr(ignoringParenImpCasts(
-
declRefExpr(to(varDecl(hasType(isInteger())).bind(ConditionVarName);
-
-static const DeclarationMatcher InitToZeroMatcher =
-varDecl(hasInitializer(ignoringParenImpCasts(integerLiteral(equals(0)
-.bind(InitVarName);
+static const StatementMatcher IntegerComparisonMatcher() {
+  return expr(ignoringParenImpCasts(
+  declRefExpr(to(varDecl(hasType(isInteger())).bind(ConditionVarName);
+}
+
+static const DeclarationMatcher InitToZeroMatcher() {
+  return varDecl(
+ hasInitializer(ignoringParenImpCasts(integerLiteral(equals(0)
+  .bind(InitVarName);
+}
 
-static const StatementMatcher IncrementVarMatcher =
-declRefExpr(to(varDecl(hasType(isInteger())).bind(IncrementVarName)));
+static const StatementMatcher IncrementVarMatcher() {
+  return declRefExpr(to(varDecl(hasType(isInteger())).bind(IncrementVarName)));
+}
 
 /// \brief The matcher for loops over arrays.
 ///
@@ -81,15 +85,15 @@
 
   return forStmt(
  unless(isInTemplateInstantiation()),
- hasLoopInit(declStmt(hasSingleDecl(InitToZeroMatcher))),
+ hasLoopInit(declStmt(hasSingleDecl(InitToZeroMatcher(,
  hasCondition(anyOf(
  binaryOperator(hasOperatorName("<"),
-hasLHS(IntegerComparisonMatcher),
+hasLHS(IntegerComparisonMatcher()),
 hasRHS(ArrayBoundMatcher)),
  binaryOperator(hasOperatorName(">"), 
hasLHS(ArrayBoundMatcher),
-hasRHS(IntegerComparisonMatcher,
+hasRHS(IntegerComparisonMatcher(),
  hasIncrement(unaryOperator(hasOperatorName("++"),
-hasUnaryOperand(IncrementVarMatcher
+
hasUnaryOperand(IncrementVarMatcher()
   .bind(LoopNameArray);
 }
 
@@ -190,7 +194,7 @@
  hasIncrement(anyOf(
  unaryOperator(hasOperatorName("++"),
hasUnaryOperand(declRefExpr(
-   to(varDecl(hasType(pointsTo(AnyType)))
+   to(varDecl(hasType(pointsTo(AnyType(
   .bind(IncrementVarName),
  cxxOperatorCallExpr(
  hasOverloadedOperatorName("++"),
@@ -278,17 +282,17 @@
  unless(isInTemplateInstantiation()),
  hasLoopInit(
  anyOf(declStmt(declCountIs(2),
-containsDeclaration(0, InitToZeroMatcher),
+containsDeclaration(0, InitToZeroMatcher()),
 containsDeclaration(1, EndDeclMatcher)),
-   declStmt(hasSingleDecl(InitToZeroMatcher,
+   declStmt(hasSingleDecl(InitToZeroMatcher(),
  hasCondition(anyOf(
  binaryOperator(hasOperatorName("<"),
-hasLHS(IntegerComparisonMatcher),
+hasLHS(IntegerComparisonMatcher()),
 hasRHS(IndexBoundMatcher)),
  binaryOperator(hasOperatorName(">"), 
hasLHS(IndexBoundMatcher),
-hasRHS(IntegerComparisonMatcher,
+hasRHS(IntegerComparisonMatcher(),
  hasIncrement(unaryOperator(hasOperatorName("++"),
-hasUnaryOperand(IncrementVarMatcher
+
hasUnaryOperand(IncrementVarMatcher()
   .bind(LoopNamePseudoArray);
 }
 


Index: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
+++

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

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363067: [libclang] Allow skipping warnings from all included 
files (authored by nik, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D48116?vs=199015=204060#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D48116

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

Index: cfe/trunk/lib/Frontend/ASTUnit.cpp
===
--- cfe/trunk/lib/Frontend/ASTUnit.cpp
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp
@@ -608,17 +608,20 @@
 };
 
 /// Diagnostic consumer that saves each diagnostic it is given.
-class StoredDiagnosticConsumer : public DiagnosticConsumer {
+class FilterAndStoreDiagnosticConsumer : public DiagnosticConsumer {
   SmallVectorImpl *StoredDiags;
   SmallVectorImpl *StandaloneDiags;
+  bool CaptureNonErrorsFromIncludes = true;
   const LangOptions *LangOpts = nullptr;
   SourceManager *SourceMgr = nullptr;
 
 public:
-  StoredDiagnosticConsumer(
+  FilterAndStoreDiagnosticConsumer(
   SmallVectorImpl *StoredDiags,
-  SmallVectorImpl *StandaloneDiags)
-  : StoredDiags(StoredDiags), StandaloneDiags(StandaloneDiags) {
+  SmallVectorImpl *StandaloneDiags,
+  bool CaptureNonErrorsFromIncludes)
+  : StoredDiags(StoredDiags), StandaloneDiags(StandaloneDiags),
+CaptureNonErrorsFromIncludes(CaptureNonErrorsFromIncludes) {
 assert((StoredDiags || StandaloneDiags) &&
"No output collections were passed to StoredDiagnosticConsumer.");
   }
@@ -634,21 +637,25 @@
 const Diagnostic ) override;
 };
 
-/// RAII object that optionally captures diagnostics, if
+/// RAII object that optionally captures and filters diagnostics, if
 /// there is no diagnostic client to capture them already.
 class CaptureDroppedDiagnostics {
   DiagnosticsEngine 
-  StoredDiagnosticConsumer Client;
+  FilterAndStoreDiagnosticConsumer Client;
   DiagnosticConsumer *PreviousClient = nullptr;
   std::unique_ptr OwningPreviousClient;
 
 public:
   CaptureDroppedDiagnostics(
-  bool RequestCapture, DiagnosticsEngine ,
+  CaptureDiagsKind CaptureDiagnostics, DiagnosticsEngine ,
   SmallVectorImpl *StoredDiags,
   SmallVectorImpl *StandaloneDiags)
-  : Diags(Diags), Client(StoredDiags, StandaloneDiags) {
-if (RequestCapture || Diags.getClient() == nullptr) {
+  : Diags(Diags),
+Client(StoredDiags, StandaloneDiags,
+   CaptureDiagnostics !=
+   CaptureDiagsKind::AllWithoutNonErrorsFromIncludes) {
+if (CaptureDiagnostics != CaptureDiagsKind::None ||
+Diags.getClient() == nullptr) {
   OwningPreviousClient = Diags.takeClient();
   PreviousClient = Diags.getClient();
   Diags.setClient(, false);
@@ -667,8 +674,16 @@
 makeStandaloneDiagnostic(const LangOptions ,
  const StoredDiagnostic );
 
-void StoredDiagnosticConsumer::HandleDiagnostic(DiagnosticsEngine::Level Level,
-const Diagnostic ) {
+static bool isInMainFile(const clang::Diagnostic ) {
+  if (!D.hasSourceManager() || !D.getLocation().isValid())
+return false;
+
+  auto  = D.getSourceManager();
+  return M.isWrittenInMainFile(M.getExpansionLoc(D.getLocation()));
+}
+
+void FilterAndStoreDiagnosticConsumer::HandleDiagnostic(
+DiagnosticsEngine::Level Level, const Diagnostic ) {
   // Default implementation (Warnings/errors count).
   DiagnosticConsumer::HandleDiagnostic(Level, Info);
 
@@ -676,6 +691,11 @@
   // about. This effectively drops diagnostics from modules we're building.
   // FIXME: In the long run, ee don't want to drop source managers from modules.
   if (!Info.hasSourceManager() || () == SourceMgr) {
+if (!CaptureNonErrorsFromIncludes && Level <= DiagnosticsEngine::Warning &&
+!isInMainFile(Info)) {
+  return;
+}
+
 StoredDiagnostic *ResultDiag = nullptr;
 if (StoredDiags) {
   StoredDiags->emplace_back(Level, Info);
@@ -723,10 +743,13 @@
 
 /// Configure the diagnostics object for use with ASTUnit.
 void ASTUnit::ConfigureDiags(IntrusiveRefCntPtr Diags,
- ASTUnit , bool CaptureDiagnostics) {
+ ASTUnit ,
+ CaptureDiagsKind CaptureDiagnostics) {
   assert(Diags.get() && "no

[PATCH] D63129: [clang-tidy] Fix invalid read on destruction

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

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

...in case the clang tidy plugin is linked into the clang binary.

Valgrind's memcheck reports:

8949== Invalid read ==8866== Invalid read of size 4
---

8866==at 0x164D248B: fetch_sub (atomic_base.h:524)
--

8866==by 0x164D248B: 
llvm::ThreadSafeRefCountedBase::Release()
 const (IntrusiveRefCntPtr.h:98)
--

8866==by 0x164CE16C: 
llvm::IntrusiveRefCntPtrInfo::release(clang::ast_matchers::internal::DynMatcherInterface*)
 (IntrusiveRefCntPtr.h:127)
--

8866==by 0x164C8D5C: 
llvm::IntrusiveRefCntPtr::release()
 (IntrusiveRefCntPtr.h:190)
---

8866==by 0x164C3B87: 
llvm::IntrusiveRefCntPtr::~IntrusiveRefCntPtr()
 (IntrusiveRefCntPtr.h:157)
---

8866==by 0x164BB4F1: 
clang::ast_matchers::internal::DynTypedMatcher::~DynTypedMatcher() 
(ASTMatchersInternal.h:341)
---

8866==by 0x164BB529: 
clang::ast_matchers::internal::Matcher::~Matcher() 
(ASTMatchersInternal.h:496)


8866==by 0xD7AE614: __cxa_finalize (cxa_finalize.c:83)
--

8866==by 0x164B3082: ??? (in 
/d2/llvm/8/qtc/builds/DebugShared/lib/libclangTidyModernizeModule.so.8)


8866==by 0x4010B72: _dl_fini (dl-fini.c:138)


8866==by 0xD7AE040: __run_exit_handlers (exit.c:108)


8866==by 0xD7AE139: exit (exit.c:139)
-

8866==by 0xD78CB9D: (below main) (libc-start.c:344)
---

8866==  Address 0x19dd9bc8 is 8 bytes inside a block of size 16 free'd
--

8866==at 0x4C3123B: operator delete(void*) (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
---

8866==by 0x1469BB99: clang::ast_matchers::internal::(anonymous 
namespace)::TrueMatcherImpl::~TrueMatcherImpl() (ASTMatchersInternal.cpp:126)


8866==by 0x1469BBC5: 
llvm::object_deleter::call(void*) (ManagedStatic.h:30)
--

8866==by 0x9ABFF26: llvm::ManagedStaticBase::destroy() const 
(ManagedStatic.cpp:72)
---

8866==by 0x9ABFF94: llvm::llvm_shutdown() (ManagedStatic.cpp:84)


8866==by 0x9A65232: llvm::InitLLVM::~InitLLVM() (InitLLVM.cpp:52)
-

8866==by 0x14B0C8: main (driver.cpp:323)


8866==  Block was alloc'd at


8866==at 0x4C3017F: operator new(unsigned long) (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)


8866==by 0x1469BB36: 
llvm::object_creator::call() (ManagedStatic.h:24)
-

8866==by 0x9ABFD99: llvm::ManagedStaticBase::RegisterManagedStatic(void* 
(*)(), void (*)(void*)) const (ManagedStatic.cpp:42)
-

8866==by 0x1469B5DF: 
llvm::ManagedStatic, 
llvm::object_deleter >::operator*()

[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

[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

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] 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"

[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] 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] D48116: [libclang] Allow skipping warnings from all included files

2019-05-27 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-05-27 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping :)


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-05-22 Thread Nikolai Kosjar via Phabricator via cfe-commits

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

As I've commented on, this change is not finished. However, I've addressed the 
inline comments nevertheless.

There is one TODO left for which I would like to have an opinion.

Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:472
   if (Info.hasSourceManager())
 checkFilters(Info.getLocation(), Info.getSourceManager());
 }

gribozavr wrote:
> It seems like the `checkFilters` call should not be skipped even if we have 
> another diagnostic engine.
checkFilters() sets some state so that later finalizeLastError() can remove 
diagnostics/errors from ClangTidyDiagnosticConsumer::Errors and also track some 
statistics.

Statistics are not relevant for the pluginc case as they should not be printed 
out for that case.
The filtering happening in finalizeLastError() does not look relevant as the 
plugin currently only supports the "-checks=..." argument but not the e.g. 
header and line filter. When checks are created in 
ClangTidyCheckFactories::createChecks, the "-checks=..." argument is honored, 
so that the !Context.isCheckEnabled(...) in finalizeLastError() is also not 
relevant.

I agree that checkFilters()/finalizeLastError() will be needed once the plugin 
case should support the other filtering options (header + line filter), but 
note that this goes beyond the scope of this change here, which is NOLINT 
filtering.

This is all new to me, so if I miss something regarding the 
checkFilters()/finalizeLastError() thingy, please tell me, preferably with an 
example if possible :)

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] D48116: [libclang] Allow skipping warnings from all included files

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

nik added a comment.

Jan?


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] D60672: [libclang] visit c++14 lambda capture init expressions

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

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC361234: [libclang] visit c++14 lambda capture init 
expressions (authored by nik, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60672?vs=195077=200431#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60672

Files:
  test/Index/cxx14-lambdas.cpp
  tools/libclang/CIndex.cpp


Index: test/Index/cxx14-lambdas.cpp
===
--- test/Index/cxx14-lambdas.cpp
+++ test/Index/cxx14-lambdas.cpp
@@ -0,0 +1,38 @@
+// Test is line- and column-sensitive; see below.
+
+typedef int Integer;
+struct X {
+  void f() {
+int localA, localB;
+auto lambda = [ptr = , copy = localB] (Integer x) -> Integer {
+  return *ptr + copy + x;
+};
+  }
+};
+
+// RUN: c-index-test -test-load-source all -std=c++14 %s | FileCheck 
-check-prefix=CHECK-LOAD %s
+// CHECK-LOAD: cxx14-lambdas.cpp:7:5: DeclStmt= Extent=[7:5 - 9:7]
+// CHECK-LOAD: cxx14-lambdas.cpp:7:10: VarDecl=lambda:7:10 (Definition) 
Extent=[7:5 - 9:6]
+// CHECK-LOAD: cxx14-lambdas.cpp:7:19: UnexposedExpr= Extent=[7:19 - 9:6]
+// CHECK-LOAD: cxx14-lambdas.cpp:7:19: CallExpr= Extent=[7:19 - 9:6]
+// CHECK-LOAD: cxx14-lambdas.cpp:7:19: UnexposedExpr= Extent=[7:19 - 9:6]
+// CHECK-LOAD: cxx14-lambdas.cpp:7:19: LambdaExpr= Extent=[7:19 - 9:6]
+// CHECK-LOAD: cxx14-lambdas.cpp:7:20: VariableRef=ptr:7:20 Extent=[7:20 - 
7:23]
+// CHECK-LOAD: cxx14-lambdas.cpp:7:35: VariableRef=copy:7:35 Extent=[7:35 - 
7:39]
+// CHECK-LOAD: cxx14-lambdas.cpp:7:27: DeclRefExpr=localA:6:9 Extent=[7:27 - 
7:33]
+// CHECK-LOAD: cxx14-lambdas.cpp:7:42: DeclRefExpr=localB:6:17 Extent=[7:42 - 
7:48]
+// CHECK-LOAD: cxx14-lambdas.cpp:7:59: ParmDecl=x:7:59 (Definition) 
Extent=[7:51 - 7:60]
+// CHECK-LOAD: cxx14-lambdas.cpp:7:51: TypeRef=Integer:3:13 Extent=[7:51 - 
7:58]
+// CHECK-LOAD: cxx14-lambdas.cpp:7:65: TypeRef=Integer:3:13 Extent=[7:65 - 
7:72]
+// CHECK-LOAD: cxx14-lambdas.cpp:7:73: CompoundStmt= Extent=[7:73 - 9:6]
+// CHECK-LOAD: cxx14-lambdas.cpp:8:7: ReturnStmt= Extent=[8:7 - 8:29]
+
+// RUN: env CINDEXTEST_INDEXLOCALSYMBOLS=1 c-index-test -index-file -std=c++14 
%s | FileCheck -check-prefix=CHECK-INDEX %s
+// CHECK-INDEX: [indexEntityReference]: kind: variable | name: ptr | USR: 
c:cxx14-lambdas.cpp@139@S@X@F@f#@Sa@F@operator()#I#1@ptr | lang: C | cursor: 
VariableRef=ptr:7:20 | loc: 7:20
+// CHECK-INDEX: [indexEntityReference]: kind: variable | name: copy | USR: 
c:cxx14-lambdas.cpp@154@S@X@F@f#@Sa@F@operator()#I#1@copy | lang: C | cursor: 
VariableRef=copy:7:35 | loc: 7:35
+// CHECK-INDEX: [indexDeclaration]: kind: variable | name: x | USR: 
c:cxx14-lambdas.cpp@170@S@X@F@f#@Sa@F@operator()#I#1@x | lang: C | cursor: 
ParmDecl=x:7:59 (Definition) | loc: 7:59
+// CHECK-INDEX: [indexEntityReference]: kind: typedef | name: Integer | USR: 
c:cxx14-lambdas.cpp@T@Integer | lang: C | cursor: TypeRef=Integer:3:13 | loc: 
7:51
+// CHECK-INDEX: [indexEntityReference]: kind: typedef | name: Integer | USR: 
c:cxx14-lambdas.cpp@T@Integer | lang: C | cursor: TypeRef=Integer:3:13 | loc: 
7:65
+// CHECK-INDEX: [indexEntityReference]: kind: variable | name: ptr | USR: 
c:cxx14-lambdas.cpp@139@S@X@F@f#@Sa@F@operator()#I#1@ptr | lang: C | cursor: 
DeclRefExpr=ptr:7:20 | loc: 8:15
+// CHECK-INDEX: [indexEntityReference]: kind: variable | name: copy | USR: 
c:cxx14-lambdas.cpp@154@S@X@F@f#@Sa@F@operator()#I#1@copy | lang: C | cursor: 
DeclRefExpr=copy:7:35 | loc: 8:21
+// CHECK-INDEX: [indexEntityReference]: kind: variable | name: x | USR: 
c:cxx14-lambdas.cpp@170@S@X@F@f#@Sa@F@operator()#I#1@x | lang: C | cursor: 
DeclRefExpr=x:7:59 | loc: 8:28
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3134,12 +3134,11 @@
   }
 
   case VisitorJob::LambdaExprPartsKind: {
-// Visit captures.
+// Visit non-init captures.
 const LambdaExpr *E = cast()->get();
 for (LambdaExpr::capture_iterator C = E->explicit_capture_begin(),
CEnd = E->explicit_capture_end();
  C != CEnd; ++C) {
-  // FIXME: Lambda init-captures.
   if (!C->capturesVariable())
 continue;
 
@@ -3148,6 +3147,11 @@
   TU)))
 return true;
 }
+// Visit init captures
+for (auto InitExpr : E->capture_inits()) {
+  if (Visit(InitExpr))
+return true;
+}
 
 TypeLoc TL = E->getCallOperator()->getTypeSourceInfo()->getTypeLoc();
 // Visit parameters and return type, if present.


Index: test/Index/cxx14-lambdas.cpp

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

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

nik added a comment.

> Are you sure you have compiled this patch? If I comment out the visit of the 
> InitExpr in CIndex.cpp again, then I get the same failure as you...

Huch, I've indeed somehow missed to compile. Sorry for that. Works fine after 
compilation :)

LGTM.


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] D61487: [clang-tidy] Make the plugin honor NOLINT

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

nik added a comment.

Ping.


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] D41005: Reuse preamble even if an unsaved file does not exist

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rC361226: [Preamble] Reuse preamble even if an unsaved file 
does not exist (authored by nik, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41005?vs=198809=200415#toc

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: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1304,22 +1304,22 @@
 PreambleInvocationIn.getDiagnosticOpts());
   getDiagnostics().setNumWarnings(NumWarningsInPreamble);
 
-  PreambleRebuildCounter = 1;
+  PreambleRebuildCountdown = 1;
   return MainFileBuffer;
 } else {
   Preamble.reset();
   PreambleDiagnostics.clear();
   TopLevelDeclsInPreamble.clear();
   PreambleSrcLocCache.clear();
-  PreambleRebuildCounter = 1;
+  PreambleRebuildCountdown = 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 (PreambleRebuildCountdown > 1) {
+--PreambleRebuildCountdown;
 return nullptr;
   }
 
@@ -1329,6 +1329,8 @@
   if (!AllowRebuild)
 return nullptr;
 
+  ++PreambleCounter;
+
   SmallVector NewPreambleDiagsStandalone;
   SmallVector NewPreambleDiags;
   ASTUnitPreambleCallbacks Callbacks;
@@ -1356,18 +1358,18 @@
 
 if (NewPreamble) {
   Preamble = std::move(*NewPreamble);
-  PreambleRebuildCounter = 1;
+  PreambleRebuildCountdown = 1;
 } else {
   switch (static_cast(NewPreamble.getError().value())) {
   case BuildPreambleError::CouldntCreateTempFile:
 // Try again next time.
-PreambleRebuildCounter = 1;
+PreambleRebuildCountdown = 1;
 return nullptr;
   case BuildPreambleError::CouldntCreateTargetInfo:
   case BuildPreambleError::BeginSourceFileFailed:
   case BuildPreambleError::CouldntEmitPCH:
 // These erros are more likely to repeat, retry after some period.
-PreambleRebuildCounter = DefaultPreambleRebuildInterval;
+PreambleRebuildCountdown = DefaultPreambleRebuildInterval;
 return nullptr;
   }
   llvm_unreachable("unexpected BuildPreambleError");
@@ -1507,7 +1509,7 @@
   AST->OnlyLocalDecls = OnlyLocalDecls;
   AST->CaptureDiagnostics = CaptureDiagnostics;
   if (PrecompilePreambleAfterNParses > 0)
-AST->PreambleRebuildCounter = PrecompilePreambleAfterNParses;
+AST->PreambleRebuildCountdown = PrecompilePreambleAfterNParses;
   AST->TUKind = Action ? Action->getTranslationUnitKind() : TU_Complete;
   AST->ShouldCacheCodeCompletionResults = CacheCodeCompletionResults;
   AST->IncludeBriefCommentsInCodeCompletion
@@ -1641,7 +1643,7 @@
 
   std::unique_ptr OverrideMainBuffer;
   if (PrecompilePreambleAfterNParses > 0) {
-PreambleRebuildCounter = PrecompilePreambleAfterNParses;
+PreambleRebuildCountdown = PrecompilePreambleAfterNParses;
 OverrideMainBuffer =
 getMainBufferWithPrecompiledPreamble(PCHContainerOps, *Invocation, VFS);
 getDiagnostics().Reset();
@@ -1819,7 +1821,7 @@
   // If we have a preamble file lying around, or if we might try to
   // build a precompiled preamble, do so now.
   std::unique_ptr OverrideMainBuffer;
-  if (Preamble || PreambleRebuildCounter > 0)
+  if (Preamble || PreambleRebuildCountdown > 0)
 OverrideMainBuffer =
 getMainBufferWithPrecompiledPreamble(PCHContainerOps, *Invocation, VFS);
 
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  : 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  : FilesInPreamble) {
+auto OverridenFileBuffer =

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

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

nik added a comment.

Ping. alexfh?


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] D48116: [libclang] Allow skipping warnings from all included files

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

nik added a comment.

Sorry for the pointless ping, haven't seen the inline comments. They are 
addressed now.

I've also increased CINDEX_VERSION_MINOR so clients can detect availability of 
this new flag.

> It's been a while since I've looked at the ASTUnit code, though, would be 
> good if someone else took an extra look at it.

Benjamin? Argyrios? Would you be so kind? :)


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-05-10 Thread Nikolai Kosjar via Phabricator via cfe-commits

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

Addressed inline comments.


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
@@ -3352,7 +3352,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));
@@ -3425,6 +3425,10 @@
   if (options & CXTranslationUnit_KeepGoing)
 Diags->setFatalsAsError(true);
 
+  CaptureDiagsKind CaptureDiagnostics = CaptureDiagsKind::All;
+  if (options & CXTranslationUnit_IgnoreNonErrorsFromIncludedFiles)
+CaptureDiagnostics = CaptureDiagsKind::AllWithoutNonErrorsFromIncludes;
+
   // Recover resources if we crash before exiting this function.
   llvm::CrashRecoveryContextCleanupRegistrar >
@@ -3502,7 +3506,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 |= CXTranslationUnit_IncludeAttributedTypes;
   if

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

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

nik marked 4 inline comments as done.
nik added inline comments.
Herald added a subscriber: dexonsmith.



Comment at: lib/Frontend/ASTUnit.cpp:682
+  auto  = D.getSourceManager();
+  return M.isInMainFile(M.getExpansionLoc(D.getLocation()));
+}

ilya-biryukov wrote:
> `isWrittenInMainFile` might be a better fit: it does not look at presumed 
> locations. That would be the expected behavior in the most common case, i.e. 
> showing errors in an IDE or a text editor.
Oh, good catch! Thanks! :)



Comment at: lib/Frontend/ASTUnit.cpp:694
+  if ((!Info.hasSourceManager() || () == SourceMgr) &&
+  (StoredDiags || StandaloneDiags)) {
+if (!CaptureNonErrorsFromIncludes

ilya-biryukov wrote:
> Why do we need this extra check? We checked for StoredDiags and Standalone 
> diags in the body of the statement later again.
Ops, looks like a left-over from a previous version, removed.


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] D53866: [Preamble] Stop circular inclusion of main file when building preamble

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

nik closed this revision.
nik added a comment.

Huch, I forgot to add "Differential Revision: " to the commit message, so 
I'll close this manually once I know how to add the svn revision number to 
this. https://llvm.org/docs/Phabricator.html states:

> In the web UI, under “Leap Into Action” put the SVN revision number in the 
> Comment, set the Action to “Close Revision” and click Submit.

Where is the "Lean Into Action" thingy? Can't find it.


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-10 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 198997.
nik marked an inline comment as done and an inline comment as not done.
nik added a comment.

Renamed to err_pp_including_mainfile_in_preamble.


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_in_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_in_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_in_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

[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-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 , const std::string ) {
 ::time_t now;
 ::time();
@@ -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  : 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  : 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.
+  if

[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

[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] 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 , const std::string ) {
 ::time_t now;
 ::time();
@@ -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  : 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  : 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 !=

[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

[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] 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 ,
  StringRef File) override {
-// Insert the current diagnostics engine.
-Context->setDiagnosticsEngine(());
+// Create and set diagnostics engine
+

[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] 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] 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-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.

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] 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 ) {
+  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 specifying a

[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 ) {
 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 () 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 ) {
 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 () const {
 IsForceEmit = true;
___
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-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] 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] 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] 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] 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

[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] 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 |=

[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] 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] 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] 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 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 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] 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 , const std::string ) {
 ::time_t now;
 ::time();
@@ -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  : 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

[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 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 , const std::string ) {
 ::time_t now;
 ::time();
@@ -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  : 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  : FilesInPreamble) {
+auto

[PATCH] D54996: [libclang] Fix clang_Cursor_isAnonymous

2019-01-09 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: test/Index/print-type.cpp:202
 // CHECK: CallExpr=Bar:17:3 [type=outer::inner::Bar] [typekind=Elaborated] 
[canonicaltype=outer::inner::Bar] [canonicaltypekind=Record] [args= 
[outer::Foo *] [Pointer]] [isPOD=0] [nbFields=3]
+// CHECK: StructDecl=:84:3 (Definition) [type=X::(anonymous struct at 
D:\code\qt_llvm\tools\clang\test\Index\print-type.cpp:84:3)] [typekind=Record] 
[isPOD=1] [nbFields=1] [isAnon=1]
+// CHECK: ClassDecl=:85:3 (Definition) [type=X::(anonymous class at 
D:\code\qt_llvm\tools\clang\test\Index\print-type.cpp:85:3)] [typekind=Record] 
[isPOD=1] [nbFields=1] [isAnon=1]

Hard coded paths will probably only work on your machine ;)


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

https://reviews.llvm.org/D54996



___
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 , 
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 , 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] 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-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] 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] 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 , const std::string ) {
 ::time_t now;
 ::time();
@@ -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  : 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  : FilesInPreamble) {
+auto OverridenFileBuffer = OverridenFileBuffers.find(F.first());
+if

[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 ) 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 () { return Writer; }
   const ASTWriter () const { return Writer; }
   SmallVectorImpl () const { return Buffer->Data; }
+  const Preprocessor () const { return PP; }
 
 public:
   PCHGenerator(const Preprocessor , 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] 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] 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] 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 ) 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 () { return Writer; }
   const ASTWriter () const { return Writer; }
   SmallVectorImpl () const { return Buffer->Data; }
+  const Preprocessor () const { return PP; }
 
 public:
   PCHGenerator(const Preprocessor , 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 && PreambleConditionalStack.isRecording() &&
+

[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-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] 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] 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] 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 ) 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 () { return Writer; }
   const ASTWriter () const { return Writer; }
   SmallVectorImpl () const { return Buffer->Data; }
+  const Preprocessor () const { return PP; }
 
 public:
   PCHGenerator(const Preprocessor , 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] 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] 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 , const std::string ) {
 ::time_t now;
 ::time();
@@ -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 ,
@@ -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 {
   llvm::MemoryBuffer *Buffer =

[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 , const std::string ) {
 ::time_t now;
 ::time();
@@ -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

[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] 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-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] 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] 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] 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] 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-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 ,
+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 ,
+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(
-

[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] 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] 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

[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

1 2 3 >

1 - 100 of 216 matches

Mail list logo