[PATCH] D37282: clangd: Tolerate additional headers

2017-08-31 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning updated this revision to Diff 113476.
puremourning added a comment.

Validate that the duplicate message is printed.


https://reviews.llvm.org/D37282

Files:
  clangd/JSONRPCDispatcher.cpp
  test/clangd/protocol.test

Index: test/clangd/protocol.test
===
--- /dev/null
+++ test/clangd/protocol.test
@@ -0,0 +1,82 @@
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# RUN: clangd -run-synchronously < %s 2>&1 | FileCheck -check-prefix=STDERR %s
+# vim: fileformat=dos
+# It is absolutely vital that this file has CRLF line endings.
+#
+# Test protocol parsing
+Content-Length: 125
+Content-Type: application/vscode-jsonrpc; charset-utf-8
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+# Test message with Content-Type after Content-Length
+#
+# CHECK: "jsonrpc":"2.0","id":0,"result":{"capabilities":{
+# CHECK-DAG: "textDocumentSync": 1,
+# CHECK-DAG: "documentFormattingProvider": true,
+# CHECK-DAG: "documentRangeFormattingProvider": true,
+# CHECK-DAG: "documentOnTypeFormattingProvider": {"firstTriggerCharacter":"}","moreTriggerCharacter":[]},
+# CHECK-DAG: "codeActionProvider": true,
+# CHECK-DAG: "completionProvider": {"resolveProvider": false, "triggerCharacters": [".",">",":"]},
+# CHECK-DAG: "definitionProvider": true
+# CHECK: }}
+
+Content-Length: 246
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"struct fake { int a, bb, ccc; int f(int i, const float f) const; };\nint main() {\n  fake f;\n  f.\n}\n"}}}
+
+Content-Type: application/vscode-jsonrpc; charset-utf-8
+Content-Length: 146
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:/main.cpp"},"position":{"line":3,"character":5}}}
+# Test message with Content-Type before Content-Length
+#
+# CHECK: {"jsonrpc":"2.0","id":1,"result":[
+# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a"}
+# CHECK: ]}
+
+X-Test: Testing
+Content-Type: application/vscode-jsonrpc; charset-utf-8
+Content-Length: 146
+Content-Type: application/vscode-jsonrpc; charset-utf-8
+X-Testing: Test
+
+{"jsonrpc":"2.0","id":2,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:/main.cpp"},"position":{"line":3,"character":5}}}
+
+Content-Type: application/vscode-jsonrpc; charset-utf-8
+Content-Length: 10
+Content-Length: 146
+
+{"jsonrpc":"2.0","id":3,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:/main.cpp"},"position":{"line":3,"character":5}}}
+# Test message with duplicate Content-Length headers
+#
+# CHECK: {"jsonrpc":"2.0","id":3,"result":[
+# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a"}
+# CHECK: ]}
+# STDERR: Warning: Duplicate Content-Length header received. The previous value for this message (10) was ignored.
+
+Content-Type: application/vscode-jsonrpc; charset-utf-8
+Content-Length: 10
+
+{"jsonrpc":"2.0","id":4,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:/main.cpp"},"position":{"line":3,"character":5}}}
+# Test message with malformed Content-Length
+#
+# STDERR: JSON dispatch failed!
+# Ensure we recover by sending another (valid) message
+
+Content-Length: 146
+
+{"jsonrpc":"2.0","id":5,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:/main.cpp"},"position":{"line":3,"character":5}}}
+# Test message with Content-Type before Content-Length
+#
+# CHECK: {"jsonrpc":"2.0","id":5,"result":[
+# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a"}
+# CHECK: ]}
+
+Content-Length: 1024
+
+{"jsonrpc":"2.0","id":5,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:/main.cpp"},"position":{"line":3,"character":5}}}
+# Test message which reads beyond the end of the stream.
+#
+# Ensure this is the last test in the file!
+# STDERR: Input was aborted. Read only {{[0-9]+}} bytes of expected {{[0-9]+}}.
+
Index: clangd/JSONRPCDispatcher.cpp
===
--- clangd/JSONRPCDispatcher.cpp
+++ clangd/JSONRPCDispatcher.cpp
@@ -136,46 +136,68 @@
JSONRPCDispatcher &Dispatcher,
bool &IsDone) {
   while (In.good()) {
-// A Language Server Protocol message starts with a HTTP header, delimited
-// by \r\n.
-std::string Line;
-std::getline(In, Line);
-if (!In.good() && errno == EINTR) {
-  In.clear();
-  continue;
+// A Language Server Protocol message starts with a set of HTTP headers,
+// delimited  by \r\n, and terminated by an empty line (\r\n).
+unsigned long long ContentLength = 0;
+while (In.good()) {
+  std::string Line;
+  std::getline(In, Lin

[PATCH] D37282: clangd: Tolerate additional headers

2017-09-02 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning added a comment.

So I think this is ready now. Anything more you need from me?




Comment at: clangd/JSONRPCDispatcher.cpp:138
bool &IsDone) {
+  unsigned long long ContentLength = 0;
   while (In.good()) {

puremourning wrote:
> ilya-biryukov wrote:
> > Maybe we could move that variable into loop body by splitting the loop body 
> > into multiple parts?
> > Having it outside a loop makes it harder to comprehend the parsing code.
> > Rewriting to something like this would arguably make it easier to read:
> > ```
> > while (In.good()) {
> >   // Read first line
> >   //
> >   llvm::StringRef LineRef = /*...*/;
> >   // Skip comments
> >   if (LineRef.startsWith("#"))
> > continue;
> >   
> >   // Read HTTP headers here, reading multiple lines right inside the loop.
> >   unsigned ContentLength = 0;
> >   // 
> > 
> > 
> >   // Read ContentLength bytes of a message here.
> >   std::vector JSON;
> >   //
> > }
> > ```
> I'll take a look, sure. Something like...
> 
> ```
> while ( the input stream is good )
> {
>   set len to 0
>   while ( the input stream is good )
>   {
> read a line into header
> if the line is a comment, read another line (continue the inner loop !)
> 
> if header is Content-Length, store the length in len
> otherwise, if header is empty, we're no longer reading headers, break out 
> of the inner loop
>   }
> 
>   if the input stream is not good, break out of the loop
> 
>   if len is 0, reset the outer loop
> 
>   read len bytes from the stream, 
>   if we read len bytes ok, parse JSON and dispatch to the handlers
> }
> ```
done.



Comment at: clangd/JSONRPCDispatcher.cpp:163
+if (LineRef.consume_front("Content-Length: ")) {
+  llvm::getAsUnsignedInteger(LineRef.trim(), 0, ContentLength);
+  continue;

ilya-biryukov wrote:
> Maybe log an error if `Content-Length` is specified twice?
OK, though I suspect this would only happen in the tests :)



Comment at: clangd/JSONRPCDispatcher.cpp:167
+  // It's another header, ignore it.
   continue;
+} else {

ilya-biryukov wrote:
> Maybe log the skipped header?
I think this would just be noise for conforming clients which send the 
(legitimate) Content-Type header. We could of course special case this, but I'm 
not sure the extra logging would be all that useful.


https://reviews.llvm.org/D37282



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

[PATCH] D37282: clangd: Tolerate additional headers

2017-09-04 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning added a comment.

In https://reviews.llvm.org/D37282#860200, @ilya-biryukov wrote:

> Looks good and ready to land. Thanks for this change.
>  Do you have commit rights to llvm repo?


Thanks for the review!

No I don't have commit rights. Can you land for me? Thanks!


https://reviews.llvm.org/D37282



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

[PATCH] D66031: clangd: use -j for background index pool

2019-08-09 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning created this revision.
puremourning added a reviewer: kadircet.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

clangd supports a -j option to limit the amount of threads to use for parsing
TUs. However, when using -background-index (the default in later versions of
clangd), the parallelism used by clangd defaults to the hardware_parallelisn,
i.e. number of physical cores.

On shared hardware environments, with large projects, this can significantly
affect performance with no way to tune it down.

This change makes the -j parameter apply equally to parsing and background
index. It's not perfect, because the total number of threads is 2x the -j value,
which may still be unexpected. But at least this change allows users to prevent
clangd using all CPU cores.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66031

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -308,7 +308,7 @@
 opt Sync{
 "sync",
 cat(Misc),
-desc("Parse on main thread. If set, -j is ignored"),
+desc("Parse on main thread. If set, -j is only applied to background 
index"),
 init(false),
 Hidden,
 };
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -43,6 +43,7 @@
 #include 
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -141,10 +142,19 @@
   if (Opts.StaticIndex)
 AddIndex(Opts.StaticIndex);
   if (Opts.BackgroundIndex) {
-BackgroundIdx = llvm::make_unique(
-Context::current().clone(), FSProvider, CDB,
-BackgroundIndexStorage::createDiskBackedStorageFactory(
-[&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); 
}));
+auto&& DBSF = BackgroundIndexStorage::createDiskBackedStorageFactory(
+[&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); });
+
+if (Opts.AsyncThreadsCount) {
+  BackgroundIdx = llvm::make_unique(
+  Context::current().clone(), FSProvider, CDB,
+  std::forward(DBSF),
+  Opts.AsyncThreadsCount );
+} else {
+  BackgroundIdx = llvm::make_unique(
+  Context::current().clone(), FSProvider, CDB,
+  std::forward( DBSF ) );
+}
 AddIndex(BackgroundIdx.get());
   }
   if (DynamicIdx)


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -308,7 +308,7 @@
 opt Sync{
 "sync",
 cat(Misc),
-desc("Parse on main thread. If set, -j is ignored"),
+desc("Parse on main thread. If set, -j is only applied to background index"),
 init(false),
 Hidden,
 };
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -43,6 +43,7 @@
 #include 
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -141,10 +142,19 @@
   if (Opts.StaticIndex)
 AddIndex(Opts.StaticIndex);
   if (Opts.BackgroundIndex) {
-BackgroundIdx = llvm::make_unique(
-Context::current().clone(), FSProvider, CDB,
-BackgroundIndexStorage::createDiskBackedStorageFactory(
-[&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }));
+auto&& DBSF = BackgroundIndexStorage::createDiskBackedStorageFactory(
+[&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); });
+
+if (Opts.AsyncThreadsCount) {
+  BackgroundIdx = llvm::make_unique(
+  Context::current().clone(), FSProvider, CDB,
+  std::forward(DBSF),
+  Opts.AsyncThreadsCount );
+} else {
+  BackgroundIdx = llvm::make_unique(
+  Context::current().clone(), FSProvider, CDB,
+  std::forward( DBSF ) );
+}
 AddIndex(BackgroundIdx.get());
   }
   if (DynamicIdx)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66031: clangd: use -j for background index pool

2019-08-09 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:152
+  std::forward(DBSF),
+  Opts.AsyncThreadsCount );
+} else {

sammccall wrote:
> can we use `std::max(Opts.AsyncThreadsCount, 1)` instead?
> 
> Having `-sync -background-index` use one thread seems less weird than having 
> it use all the cores.
> (Or at least not more weird, and simpler in the code here)
Hmm. What I was thinking is more that if you specify none of sync or -j, you 
should get physical cores as you do now.

But I realise that this change doesn't do that, because AsyncThreadsCount 
defaults slightly differently  to `llvm::heavyweight_hardware_concurrency()` 
(it uses std::thread::hardware_concurrency)

The difference is pretty small, so probably not material ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66031



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

[PATCH] D66031: clangd: use -j for background index pool

2019-08-09 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning updated this revision to Diff 214454.
puremourning marked 3 inline comments as done.
puremourning added a comment.

When -sync -background-index supplied, use 1 thread for background index.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66031

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -267,7 +267,8 @@
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
-desc("Number of async workers used by clangd"),
+desc("Number of async workers used by clangd. Background index also "
+ "uses this many workers."),
 init(getDefaultAsyncThreadsCount()),
 };
 
@@ -308,7 +309,8 @@
 opt Sync{
 "sync",
 cat(Misc),
-desc("Parse on main thread. If set, -j is ignored"),
+desc("Handle client requests on main thread. Background index still uses "
+ "its own thread."),
 init(false),
 Hidden,
 };
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -40,9 +40,11 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -117,8 +119,7 @@
  : nullptr),
   GetClangTidyOptions(Opts.GetClangTidyOptions),
   SuggestMissingIncludes(Opts.SuggestMissingIncludes),
-  TweakFilter(Opts.TweakFilter),
-  WorkspaceRoot(Opts.WorkspaceRoot),
+  TweakFilter(Opts.TweakFilter), WorkspaceRoot(Opts.WorkspaceRoot),
   // Pass a callback into `WorkScheduler` to extract symbols from a newly
   // parsed file and rebuild the file index synchronously each time an AST
   // is parsed.
@@ -144,7 +145,8 @@
 BackgroundIdx = llvm::make_unique(
 Context::current().clone(), FSProvider, CDB,
 BackgroundIndexStorage::createDiskBackedStorageFactory(
-[&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); 
}));
+[&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }),
+std::max(Opts.AsyncThreadsCount, static_cast(1)));
 AddIndex(BackgroundIdx.get());
   }
   if (DynamicIdx)


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -267,7 +267,8 @@
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
-desc("Number of async workers used by clangd"),
+desc("Number of async workers used by clangd. Background index also "
+ "uses this many workers."),
 init(getDefaultAsyncThreadsCount()),
 };
 
@@ -308,7 +309,8 @@
 opt Sync{
 "sync",
 cat(Misc),
-desc("Parse on main thread. If set, -j is ignored"),
+desc("Handle client requests on main thread. Background index still uses "
+ "its own thread."),
 init(false),
 Hidden,
 };
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -40,9 +40,11 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -117,8 +119,7 @@
  : nullptr),
   GetClangTidyOptions(Opts.GetClangTidyOptions),
   SuggestMissingIncludes(Opts.SuggestMissingIncludes),
-  TweakFilter(Opts.TweakFilter),
-  WorkspaceRoot(Opts.WorkspaceRoot),
+  TweakFilter(Opts.TweakFilter), WorkspaceRoot(Opts.WorkspaceRoot),
   // Pass a callback into `WorkScheduler` to extract symbols from a newly
   // parsed file and rebuild the file index synchronously each time an AST
   // is parsed.
@@ -144,7 +145,8 @@
 BackgroundIdx = llvm::make_unique(
 Context::current().clone(), FSProvider, CDB,
 BackgroundIndexStorage::createDiskBackedStorageFactory(
-[&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }));
+[&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }),
+std::max(Opts.AsyncThreadsCount, static_cast(1)));
 AddIndex(BackgroundIdx.get());
   }
   if (DynamicIdx)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66031: clangd: use -j for background index pool

2019-08-09 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning updated this revision to Diff 214460.
puremourning marked an inline comment as not done.
puremourning added a comment.
Herald added subscribers: jfb, javed.absar.

Always use physical cores rather than logical cores for best performance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66031

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -267,7 +267,8 @@
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
-desc("Number of async workers used by clangd"),
+desc("Number of async workers used by clangd. Background index also "
+ "uses this many workers."),
 init(getDefaultAsyncThreadsCount()),
 };
 
@@ -308,7 +309,8 @@
 opt Sync{
 "sync",
 cat(Misc),
-desc("Parse on main thread. If set, -j is ignored"),
+desc("Handle client requests on main thread. Background index still uses "
+ "its own thread."),
 init(false),
 Hidden,
 };
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -54,6 +54,7 @@
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Threading.h"
 #include 
 #include 
 #include 
@@ -801,10 +802,10 @@
 } // namespace
 
 unsigned getDefaultAsyncThreadsCount() {
-  unsigned HardwareConcurrency = std::thread::hardware_concurrency();
-  // C++ standard says that hardware_concurrency()
-  // may return 0, fallback to 1 worker thread in
-  // that case.
+  unsigned HardwareConcurrency = llvm::heavyweight_hardware_concurrency();
+  // heavyweight_hardware_concurrency may fall back to hardware_concurrency.
+  // C++ standard says that hardware_concurrency() may return 0; fallback to 1
+  // worker thread in that case.
   if (HardwareConcurrency == 0)
 return 1;
   return HardwareConcurrency;
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -40,9 +40,11 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -117,8 +119,7 @@
  : nullptr),
   GetClangTidyOptions(Opts.GetClangTidyOptions),
   SuggestMissingIncludes(Opts.SuggestMissingIncludes),
-  TweakFilter(Opts.TweakFilter),
-  WorkspaceRoot(Opts.WorkspaceRoot),
+  TweakFilter(Opts.TweakFilter), WorkspaceRoot(Opts.WorkspaceRoot),
   // Pass a callback into `WorkScheduler` to extract symbols from a newly
   // parsed file and rebuild the file index synchronously each time an AST
   // is parsed.
@@ -144,7 +145,8 @@
 BackgroundIdx = llvm::make_unique(
 Context::current().clone(), FSProvider, CDB,
 BackgroundIndexStorage::createDiskBackedStorageFactory(
-[&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); 
}));
+[&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }),
+std::max(Opts.AsyncThreadsCount, 1u));
 AddIndex(BackgroundIdx.get());
   }
   if (DynamicIdx)


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -267,7 +267,8 @@
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
-desc("Number of async workers used by clangd"),
+desc("Number of async workers used by clangd. Background index also "
+ "uses this many workers."),
 init(getDefaultAsyncThreadsCount()),
 };
 
@@ -308,7 +309,8 @@
 opt Sync{
 "sync",
 cat(Misc),
-desc("Parse on main thread. If set, -j is ignored"),
+desc("Handle client requests on main thread. Background index still uses "
+ "its own thread."),
 init(false),
 Hidden,
 };
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -54,6 +54,7 @@
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Threading.h"
 #include 
 #include 
 #include 
@@ -801,10 +802,10 @@
 } // namespace
 
 unsigned getDefaultAsyncThreadsCount() {
-  unsigned HardwareConcurrency = std::thread::hardware_concur

[PATCH] D66031: clangd: use -j for background index pool

2019-08-09 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning marked an inline comment as done.
puremourning added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:152
+  std::forward(DBSF),
+  Opts.AsyncThreadsCount );
+} else {

sammccall wrote:
> puremourning wrote:
> > sammccall wrote:
> > > can we use `std::max(Opts.AsyncThreadsCount, 1)` instead?
> > > 
> > > Having `-sync -background-index` use one thread seems less weird than 
> > > having it use all the cores.
> > > (Or at least not more weird, and simpler in the code here)
> > Hmm. What I was thinking is more that if you specify none of sync or -j, 
> > you should get physical cores as you do now.
> > 
> > But I realise that this change doesn't do that, because AsyncThreadsCount 
> > defaults slightly differently  to 
> > `llvm::heavyweight_hardware_concurrency()` (it uses 
> > std::thread::hardware_concurrency)
> > 
> > The difference is pretty small, so probably not material ?
> yikes, I forgot about that difference.
> 
> We observed *significantly* worse performance and responsiveness when 
> background threads was equal to the number of hardware threads rather than 
> number of cores.
> 
> If you don't mind, we should just use cores for everything: change 
> `getDefaultAsyncThreadCount()` in TUScheduler.cpp to call 
> llvm::heavyweight_hardware_concurrency() instead of 
> std::thread::hardware_concurrency().
Sure thing. That makes sense.

It occurs to me that we might want to change the default value used by 
`BackgroundIndex` constructor,  because it also can end up with `0` return from 
`heavyweight_hardware_concurrency`.

Worth changing that here ? I think the default is only used by the tests now 
though, so probably not a big issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66031



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

[PATCH] D66031: clangd: use -j for background index pool

2019-08-09 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning updated this revision to Diff 214463.
puremourning added a comment.

Rebase on master


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66031

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -267,7 +267,8 @@
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
-desc("Number of async workers used by clangd"),
+desc("Number of async workers used by clangd. Background index also "
+ "uses this many workers."),
 init(getDefaultAsyncThreadsCount()),
 };
 
@@ -308,7 +309,8 @@
 opt Sync{
 "sync",
 cat(Misc),
-desc("Parse on main thread. If set, -j is ignored"),
+desc("Handle client requests on main thread. Background index still uses "
+ "its own thread."),
 init(false),
 Hidden,
 };
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -54,6 +54,7 @@
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Threading.h"
 #include 
 #include 
 #include 
@@ -801,10 +802,10 @@
 } // namespace
 
 unsigned getDefaultAsyncThreadsCount() {
-  unsigned HardwareConcurrency = std::thread::hardware_concurrency();
-  // C++ standard says that hardware_concurrency()
-  // may return 0, fallback to 1 worker thread in
-  // that case.
+  unsigned HardwareConcurrency = llvm::heavyweight_hardware_concurrency();
+  // heavyweight_hardware_concurrency may fall back to hardware_concurrency.
+  // C++ standard says that hardware_concurrency() may return 0; fallback to 1
+  // worker thread in that case.
   if (HardwareConcurrency == 0)
 return 1;
   return HardwareConcurrency;
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -40,9 +40,11 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -117,8 +119,7 @@
  : nullptr),
   GetClangTidyOptions(Opts.GetClangTidyOptions),
   SuggestMissingIncludes(Opts.SuggestMissingIncludes),
-  TweakFilter(Opts.TweakFilter),
-  WorkspaceRoot(Opts.WorkspaceRoot),
+  TweakFilter(Opts.TweakFilter), WorkspaceRoot(Opts.WorkspaceRoot),
   // Pass a callback into `WorkScheduler` to extract symbols from a newly
   // parsed file and rebuild the file index synchronously each time an AST
   // is parsed.
@@ -144,7 +145,8 @@
 BackgroundIdx = llvm::make_unique(
 Context::current().clone(), FSProvider, CDB,
 BackgroundIndexStorage::createDiskBackedStorageFactory(
-[&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); 
}));
+[&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }),
+std::max(Opts.AsyncThreadsCount, 1u));
 AddIndex(BackgroundIdx.get());
   }
   if (DynamicIdx)


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -267,7 +267,8 @@
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
-desc("Number of async workers used by clangd"),
+desc("Number of async workers used by clangd. Background index also "
+ "uses this many workers."),
 init(getDefaultAsyncThreadsCount()),
 };
 
@@ -308,7 +309,8 @@
 opt Sync{
 "sync",
 cat(Misc),
-desc("Parse on main thread. If set, -j is ignored"),
+desc("Handle client requests on main thread. Background index still uses "
+ "its own thread."),
 init(false),
 Hidden,
 };
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -54,6 +54,7 @@
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Threading.h"
 #include 
 #include 
 #include 
@@ -801,10 +802,10 @@
 } // namespace
 
 unsigned getDefaultAsyncThreadsCount() {
-  unsigned HardwareConcurrency = std::thread::hardware_concurrency();
-  // C++ standard says that hardware_concurrency()
-  // may return 0, fallback to 1 worker thread in
-  // that case.
+  unsigned HardwareCon

[PATCH] D66031: clangd: use -j for background index pool

2019-08-09 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning added a comment.

In D66031#1623855 , @sammccall wrote:

> Thanks! Want me to land this for you?


yes please! I don't have commit rights.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66031



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

[PATCH] D66038: [clangd] Give absolute path to clang-tidy and include-fixer. HintPath should always be absolute, some URI schemes care.

2019-08-09 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning added a comment.

LGTM :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66038



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

[PATCH] D66056: Add 2 tests which cover the hover result for auto

2019-08-10 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning created this revision.
puremourning added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, 
ilya-biryukov.
Herald added a project: clang.

Clangd supports reporting the deduced type when hovering on 'auto', but
does not report it when hovering on a particular name/decl.

Add two tests which show this behaviour.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66056

Files:
  clang-tools-extra/clangd/test/hover.test


Index: clang-tools-extra/clangd/test/hover.test
===
--- clang-tools-extra/clangd/test/hover.test
+++ clang-tools-extra/clangd/test/hover.test
@@ -52,6 +52,50 @@
 # CHECK-NEXT:  }
 # CHECK-NEXT:}
 ---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main3.cpp","languageId":"cpp","version":1,"text":"int
 main() { auto x = 2; }\n"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"test:///main3.cpp"},"position":{"line":0,"character":18}}}
+#  CHECK:  "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
+# CHECK-NEXT:"contents": {
+# CHECK-NEXT:  "kind": "plaintext",
+# CHECK-NEXT:  "value": "Declared in main\n\nauto x = 2"
+# CHECK-NEXT:},
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 19,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 18,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"test:///main3.cpp"},"position":{"line":0,"character":15}}}
+#  CHECK:  "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
+# CHECK-NEXT:"contents": {
+# CHECK-NEXT:  "kind": "plaintext",
+# CHECK-NEXT:  "value": "int"
+# CHECK-NEXT:},
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 17,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 13,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+---
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}


Index: clang-tools-extra/clangd/test/hover.test
===
--- clang-tools-extra/clangd/test/hover.test
+++ clang-tools-extra/clangd/test/hover.test
@@ -52,6 +52,50 @@
 # CHECK-NEXT:  }
 # CHECK-NEXT:}
 ---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main3.cpp","languageId":"cpp","version":1,"text":"int main() { auto x = 2; }\n"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"test:///main3.cpp"},"position":{"line":0,"character":18}}}
+#  CHECK:  "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
+# CHECK-NEXT:"contents": {
+# CHECK-NEXT:  "kind": "plaintext",
+# CHECK-NEXT:  "value": "Declared in main\n\nauto x = 2"
+# CHECK-NEXT:},
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 19,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 18,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"test:///main3.cpp"},"position":{"line":0,"character":15}}}
+#  CHECK:  "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
+# CHECK-NEXT:"contents": {
+# CHECK-NEXT:  "kind": "plaintext",
+# CHECK-NEXT:  "value": "int"
+# CHECK-NEXT:},
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 17,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 13,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+---
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66254: Correct include suggestion when search path includes symlink

2019-08-14 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning created this revision.
puremourning added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, ilya-biryukov.
Herald added a project: clang.
puremourning added a comment.

FYI this fixes the issue in https://github.com/clangd/clangd/issues/124

I haven't added new tests to HeaderSearchTest.cc because the InMemoryFileSystem 
doesn't support symlinks and I didn't want to try and implement that for this 
patch. Let me know thoughts on alternative ways to regression test it.


When a header search path contained a symlink, the suggested include
spelling could be wrong, as the files to insert provided by clangd are
always canonical (i.e. with the symlinks removed), but the search paths
(which come from flags), might not be. As files from Sema also come from
the VFS, they are also canonical, I think.

When trying to suggest the spelling of the include to insert, we should
always compare canonical paths. This corrects the include suggestion in
this case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66254

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/lib/Lex/HeaderSearch.cpp


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1723,7 +1723,13 @@
 if (!WorkingDir.empty() && !path::is_absolute(Dir))
   fs::make_absolute(WorkingDir, DirPath);
 path::remove_dots(DirPath, /*remove_dot_dot=*/true);
-Dir = DirPath;
+// Find the canonical absolute path, as that's what we expect in File
+auto& FS = FileMgr.getVirtualFileSystem();
+SmallString<4096> CanonicalNameBuf;
+if (!FS.getRealPath(DirPath, CanonicalNameBuf))
+  Dir = CanonicalNameBuf;
+else
+  Dir = DirPath;
 for (auto NI = path::begin(File), NE = path::end(File),
   DI = path::begin(Dir), DE = path::end(Dir);
  /*termination condition in loop*/; ++NI, ++DI) {
Index: clang/include/clang/Lex/HeaderSearch.h
===
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -727,6 +727,7 @@
   /// MainFile location, if none of the include search directories were prefix
   /// of File.
   ///
+  /// \param File A canonical absolute path to the file to be included.
   /// \param WorkingDir If non-empty, this will be prepended to search 
directory
   /// paths that are relative.
   std::string suggestPathToFileForDiagnostics(llvm::StringRef File,


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1723,7 +1723,13 @@
 if (!WorkingDir.empty() && !path::is_absolute(Dir))
   fs::make_absolute(WorkingDir, DirPath);
 path::remove_dots(DirPath, /*remove_dot_dot=*/true);
-Dir = DirPath;
+// Find the canonical absolute path, as that's what we expect in File
+auto& FS = FileMgr.getVirtualFileSystem();
+SmallString<4096> CanonicalNameBuf;
+if (!FS.getRealPath(DirPath, CanonicalNameBuf))
+  Dir = CanonicalNameBuf;
+else
+  Dir = DirPath;
 for (auto NI = path::begin(File), NE = path::end(File),
   DI = path::begin(Dir), DE = path::end(Dir);
  /*termination condition in loop*/; ++NI, ++DI) {
Index: clang/include/clang/Lex/HeaderSearch.h
===
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -727,6 +727,7 @@
   /// MainFile location, if none of the include search directories were prefix
   /// of File.
   ///
+  /// \param File A canonical absolute path to the file to be included.
   /// \param WorkingDir If non-empty, this will be prepended to search directory
   /// paths that are relative.
   std::string suggestPathToFileForDiagnostics(llvm::StringRef File,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66254: Correct include suggestion when search path includes symlink

2019-08-14 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning added a comment.

FYI this fixes the issue in https://github.com/clangd/clangd/issues/124

I haven't added new tests to HeaderSearchTest.cc because the InMemoryFileSystem 
doesn't support symlinks and I didn't want to try and implement that for this 
patch. Let me know thoughts on alternative ways to regression test it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66254



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

[PATCH] D66254: Correct include suggestion when search path includes symlink

2019-08-14 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning added a comment.

In D66254#1630277 , @lebedev.ri wrote:

> Tests?


Per my comment: I haven't added new tests to HeaderSearchTest.cc because the 
InMemoryFileSystem doesn't support symlinks and I didn't want to try and 
implement that for this patch. Let me know thoughts on alternative ways to 
regression test it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66254



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

[PATCH] D66254: Correct include suggestion when search path includes symlink

2019-08-14 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning added a comment.

In D66254#1630283 , @puremourning 
wrote:

> In D66254#1630277 , @lebedev.ri 
> wrote:
>
> > Tests?
>
>
> Per my comment: I haven't added new tests to HeaderSearchTest.cc because the 
> InMemoryFileSystem doesn't support symlinks and I didn't want to try and 
> implement that for this patch. Let me know thoughts on alternative ways to 
> regression test it.


OK I found test/SemaCXX/missing-header

Might be able to use the _real_ filesystem and go via the diagnostic rather 
than clangd


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66254



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

[PATCH] D93565: scan-view: Remove Reporter.py and associated AppleScript files

2021-05-20 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning added a comment.

Unless i'm missing something, this change seems to have broken scan-view, as it 
now just says

"ModuleNotFOundError: No module named 'Reporter'"

Due to :

`import Reporter` in `ScanView.py`

Am i losing my mind?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93565

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

[PATCH] D93565: scan-view: Remove Reporter.py and associated AppleScript files

2021-05-20 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning added a comment.

In D93565#2770709 , @puremourning 
wrote:

> Unless i'm missing something, this change seems to have broken scan-view, as 
> it now just says
>
> "ModuleNotFOundError: No module named 'Reporter'"
>
> Due to :
>
> `import Reporter` in `ScanView.py`
>
> Am i losing my mind?

This seems to fix it:

  index 5a5d15e85b30..4eccb9958a05 100644
  --- a/clang/tools/scan-view/share/ScanView.py
  +++ b/clang/tools/scan-view/share/ScanView.py
  @@ -26,7 +26,6 @@ import time
   import socket
   import itertools
  
  -import Reporter
   try:
   import configparser
   except ImportError:
  Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]? y
  @@ -775,9 +774,7 @@ File Bug
  
  
   def create_server(address, options, root):
  -import Reporter
  -
  -reporters = Reporter.getReporters()
  +reporters = []
  
   return ScanViewServer(address, ScanViewRequestHandler,
 root,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93565

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

[PATCH] D93565: scan-view: Remove Reporter.py and associated AppleScript files

2021-05-21 Thread Ben Jackson via Phabricator via cfe-commits 
puremourning added a comment.

Erm, doh. I thought I was on main, but I was using clang 12.0.0 (tag 
llvmorg-10.0.0 from llvm-project). We tend to only use release tagged versions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93565

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