[PATCH] D29271: Revert r293455, which breaks v8 with a spurious error. Testcase added.

2017-01-30 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. Revert r293455, which breaks v8 with a spurious error. Testcase added. https://reviews.llvm.org/D29271 Files: include/clang/AST/DeclTemplate.h include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaDecl.cpp

[PATCH] D29271: Revert r293455, which breaks v8 with a spurious error. Testcase added.

2017-01-30 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In https://reviews.llvm.org/D29271#660169, @sberg wrote: > Btw, ran into that with the even simpler test case Thanks, I stole it :-) https://reviews.llvm.org/D29271 ___ cfe-commits mailing list

[PATCH] D29271: Revert r293455, which breaks v8 with a spurious error. Testcase added.

2017-01-30 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL293473: Revert r293455, which breaks v8 with a spurious error. Testcase added. (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D29271?vs=86257=86259#toc Repository: rL

[PATCH] D29303: In VirtualCallChecker, handle indirect calls

2017-01-30 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 86337. sammccall added a comment. Oops, reverted noise :( https://reviews.llvm.org/D29303 Files: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp Index: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp

[PATCH] D29303: In VirtualCallChecker, handle indirect calls

2017-01-30 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. I couldn't work out how to add a test for this, advice appreciated. Closest I could get was adding something like this to test/Analysis/virtualcall.cpp: class F { public: F(); void foo(); }; F::F() { void (F::* ptr) = ::foo; (this->*ptr)(); } which crashes, but

[PATCH] D29303: In VirtualCallChecker, handle indirect calls

2017-01-30 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 86342. sammccall added a comment. Add regression test. https://reviews.llvm.org/D29303 Files: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp test/Analysis/virtualcall.cpp Index: test/Analysis/virtualcall.cpp

[PATCH] D29303: In VirtualCallChecker, handle indirect calls

2017-01-30 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. In VirtualCallChecker, handle indirect calls. getDirectCallee() can be nullptr, and dyn_cast(nullptr) is UB https://reviews.llvm.org/D29303 Files: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp Index: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp

[PATCH] D29303: In VirtualCallChecker, handle indirect calls

2017-01-30 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL293604: In VirtualCallChecker, handle indirect calls (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D29303?vs=86342=86385#toc Repository: rL LLVM

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 89337. sammccall marked 7 inline comments as done. sammccall added a comment. Address review comments. https://reviews.llvm.org/D30210 Files: include-fixer/IncludeFixer.cpp include-fixer/SymbolIndexManager.cpp

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall planned changes to this revision. sammccall added a comment. OK, some changes to come here: - klimek thinks using a set as a pseudo-map with `mutable` is evil, which is a fair point - the current `SymbolReporter` interface doesn't provide enough information to deduplicate

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks for the review, still learning the style :) Comment at: include-fixer/find-all-symbols/FindAllSymbols.cpp:250 + } else { +assert(!"Must match a NamedDecl!"); + } hokein wrote: > Is the preceding `!` intended? This does

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 89360. sammccall added a comment. Changes based on offline discussion: - mutable SignalInfo data split into SignalInfo::Signals - yaml serialized data is SignalInfo::WithSignals, which is currently a pair -

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 89632. sammccall added a comment. Report symbols by reference. https://reviews.llvm.org/D30210 Files: include-fixer/InMemorySymbolIndex.cpp include-fixer/InMemorySymbolIndex.h include-fixer/IncludeFixer.cpp include-fixer/SymbolIndex.h

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 89636. sammccall added a comment. Remove redundant std::move https://reviews.llvm.org/D30210 Files: include-fixer/InMemorySymbolIndex.cpp include-fixer/InMemorySymbolIndex.h include-fixer/IncludeFixer.cpp include-fixer/SymbolIndex.h

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 89484. sammccall marked 4 inline comments as done. sammccall added a comment. Address review comments. https://reviews.llvm.org/D30210 Files: include-fixer/InMemorySymbolIndex.cpp include-fixer/InMemorySymbolIndex.h include-fixer/IncludeFixer.cpp

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: include-fixer/InMemorySymbolIndex.h:27 - std::vector + std::vector search(llvm::StringRef Identifier) override; hokein wrote: > There are many places using > `std::vector`. Maybe we can use a > type alias for

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 89510. sammccall added a comment. Update comment - oops! https://reviews.llvm.org/D30210 Files: include-fixer/InMemorySymbolIndex.cpp include-fixer/InMemorySymbolIndex.h include-fixer/IncludeFixer.cpp include-fixer/SymbolIndex.h

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks for bearing with me here :) Comment at: include-fixer/InMemorySymbolIndex.h:27 - std::vector + std::vector search(llvm::StringRef Identifier) override; hokein wrote: > sammccall wrote: > > hokein wrote: > > > There are

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 89506. sammccall marked 3 inline comments as done. sammccall added a comment. Address review comments; remove redundant namespace qualifiers; format. https://reviews.llvm.org/D30210 Files: include-fixer/InMemorySymbolIndex.cpp

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. Add usage count to find-all-symbols. FindAllSymbols now finds (most!) main-file usages of the discovered symbols. The per-TU map output has NumUses=0 or 1 (only one use per file is counted). The reducer aggregates these to find the number of files that use a

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 89236. sammccall added a comment. Remove obsolete comment. https://reviews.llvm.org/D30210 Files: include-fixer/IncludeFixer.cpp include-fixer/SymbolIndexManager.cpp include-fixer/find-all-symbols/FindAllSymbols.cpp

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 89365. sammccall added a comment. Switch from SymbolInfo::WithSignals as a typedef for std::pair, to a struct SymbolAndSignals, to have clearer semantics. https://reviews.llvm.org/D30210 Files: include-fixer/InMemorySymbolIndex.cpp

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 89366. sammccall added a comment. git-clang-format https://reviews.llvm.org/D30210 Files: include-fixer/InMemorySymbolIndex.cpp include-fixer/InMemorySymbolIndex.h include-fixer/IncludeFixer.cpp include-fixer/SymbolIndex.h

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-28 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL296446: [include-fixer] Add usage count to find-all-symbols. (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D30210?vs=89982=89983#toc Repository: rL LLVM

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

2017-02-28 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 89982. sammccall marked an inline comment as done. sammccall added a comment. Review comments https://reviews.llvm.org/D30210 Files: include-fixer/InMemorySymbolIndex.cpp include-fixer/InMemorySymbolIndex.h include-fixer/IncludeFixer.cpp

[PATCH] D27504: Compilation database test: don't try to output to CWD

2016-12-06 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. sammccall added a reviewer: bkramer. sammccall added subscribers: cfe-commits, joerg. Write output from compilation database test to %T rather than the working dir. Sometimes CWD isn't writable! Also specify no-canonical-prefixes so that clang has 'clang' in the

[PATCH] D27504: Compilation database test: don't try to output to CWD

2016-12-06 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. bkramer: summoning my lit guru for guidance, is there a better way? https://reviews.llvm.org/D27504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27504: Compilation database test: don't try to output to CWD

2016-12-07 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL288892: Compilation database test: don't try to output to CWD (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D27504?vs=80553=80554#toc Repository: rL LLVM

[PATCH] D27504: Compilation database test: don't try to output to CWD

2016-12-07 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 80553. sammccall added a comment. Hoist cd %T to avoid repeating it. https://reviews.llvm.org/D27504 Files: test/Driver/compilation_database.c Index: test/Driver/compilation_database.c

[PATCH] D27504: Compilation database test: don't try to output to CWD

2016-12-07 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked an inline comment as done. sammccall added a comment. Thanks! That makes sense. https://reviews.llvm.org/D27504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28246: clang-format: [JS] avoid indent after ambient function declarations.

2017-01-04 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a reviewer: sammccall. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: unittests/Format/FormatTestJS.cpp:382 + "let x = 1;\n"); + verifyFormat("declare function foo():

[PATCH] D30685: [include-fixer] Remove line number from Symbol identity

2017-03-09 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL297371: [include-fixer] Remove line number from Symbol identity (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D30685?vs=90990=91149#toc Repository: rL LLVM

[PATCH] D30720: [include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

2017-03-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 91042. sammccall marked 4 inline comments as done. sammccall added a comment. Address review comments. https://reviews.llvm.org/D30720 Files: include-fixer/CMakeLists.txt include-fixer/FuzzySymbolIndex.cpp include-fixer/FuzzySymbolIndex.h

[PATCH] D30720: [include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

2017-03-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks for the comments! Comment at: include-fixer/SymbolIndexManager.cpp:106 // Match the identifier name without qualifier. - if (Symbol.getName() == Names.back()) { -bool IsMatched = true; hokein wrote: > Just

[PATCH] D30720: [include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

2017-03-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. bkramer: ping https://reviews.llvm.org/D30720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30720: [include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

2017-03-13 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL297630: [include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly. (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D30720?vs=91042=91570#toc Repository:

[PATCH] D30685: [include-fixer] Remove line number from Symbol identity

2017-03-07 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. Remove line number from Symbol identity. For our purposes (include-fixer and clangd autocomplete), function overloads within the same header should mostly be treated as a single combined symbol. We may want to track individual occurrences (line number, full type

[PATCH] D30685: [include-fixer] Remove line number from Symbol identity

2017-03-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 90990. sammccall added a comment. Address review comments. Uncovered two bugs: - accidentally casting through bool: EXPECT_EQ(1, hasSymbol) was a lie - we were double counting uses of typedefs where the underlying type was built in, because

[PATCH] D30685: [include-fixer] Remove line number from Symbol identity

2017-03-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked an inline comment as done. sammccall added a comment. In https://reviews.llvm.org/D30685#695284, @hokein wrote: > > We may want to track individual occurrences (line number, full type info) > > and aggregate this during mapreduce, but that's not done here. > > Seems reasonable.

[PATCH] D30720: [include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

2017-03-07 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. Herald added a subscriber: mgorny. Add fuzzy SymbolIndex, where identifier needn't match exactly. The purpose for this is global autocomplete in clangd. The query will be a partial identifier up to the cursor, and the results will be suggestions. It's in

[PATCH] D38627: [clangd] Added move-only function helpers.

2017-10-09 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/Function.h:9 +//===--===// + +#ifndef

[PATCH] D38629: [clangd] Added a callback-based codeComplete in clangd.

2017-10-09 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. I'm missing some context for this one: - what's the goal? Make the code read more naturally, change the async model, or something else? - what's the end state for codeComplete specifically? will we switch to the new overload and delete the other, or is

[PATCH] D38939: [clangd] Handle exit notification (proper shutdown)

2017-10-16 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: test/clangd/authority-less-uri.test:33 {"jsonrpc":"2.0","id":3,"method":"shutdown"} +# CHECK: {"jsonrpc":"2.0","id":3,"result":null} +Content-Length:

[PATCH] D38464: [clangd] less boilerplate in RPC dispatch

2017-10-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks! All addressed (except removing ShutdownParams, which I'm not sure is worthwhile). Comment at: clangd/ClangdLSPServer.h:47 // Implement ProtocolCallbacks. - void onInitialize(StringRef ID, InitializeParams IP, -

[PATCH] D38464: [clangd] less boilerplate in RPC dispatch

2017-10-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 117835. sammccall marked 5 inline comments as done. sammccall added a comment. - ClangLSPServer.h should refer to ShutdownParams, not NoParams - Address review comments https://reviews.llvm.org/D38464 Files: clangd/ClangdLSPServer.cpp

[PATCH] D38939: [clangd] Handle exit notification (proper shutdown)

2017-10-17 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. So I'm not totally convinced that treating an abrupt exit or EOF as an error is worth the hassle (others might disagree), but it's certainly reasonable. Could you add a test for the new behavior? something like RUN: not clangd

[PATCH] D38629: [clangd] Added a callback-based codeComplete in clangd.

2017-10-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clangd/ClangdServer.cpp:51 +template +std::future makeFutureAPIFromCallback( I'm not sure we need a template here rather than just writing the code directly. Comment at:

[PATCH] D38720: [clangd] Report proper kinds for 'Keyword' and 'Snippet' completion items.

2017-10-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. Going to land this on Ilya's behalf as he's out on vacation. https://reviews.llvm.org/D38720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38464: [clangd] less boilerplate in RPC dispatch

2017-10-12 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 118782. sammccall added a comment. Rebase https://reviews.llvm.org/D38464 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/JSONRPCDispatcher.cpp clangd/JSONRPCDispatcher.h clangd/Protocol.cpp clangd/Protocol.h

[PATCH] D38464: [clangd] less boilerplate in RPC dispatch

2017-10-12 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL315577: [clangd] less boilerplate in RPC dispatch (authored by sammccall). Repository: rL LLVM https://reviews.llvm.org/D38464 Files: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp

[PATCH] D37620: [Sema] Put tautological comparison of unsigned and zero into it's own flag

2017-09-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks! Unless I'm missing something, the current patch just adds the category, but doesn't actually recategorize the warning, right? Repository: rL LLVM https://reviews.llvm.org/D37620 ___ cfe-commits mailing list

[PATCH] D37620: [Sema] Put tautological comparison of unsigned and zero into it's own flag

2017-09-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Right, of course. Thanks and LG, but clearly also get a review from someone more familiar with the code (I'm just a buildcop) Repository: rL LLVM https://reviews.llvm.org/D37620

[PATCH] D38414: [clangd] simplify ClangdLSPServer by private-inheriting callback interfaces. NFC

2017-09-30 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL314587: [clangd] simplify ClangdLSPServer by private-inheriting callback interfaces. NFC (authored by sammccall). Repository: rL LLVM https://reviews.llvm.org/D38414 Files:

[PATCH] D38414: [clangd] simplify ClangdLSPServer by private-inheriting callback interfaces. NFC

2017-09-30 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 117256. sammccall added a comment. - Address review comments https://reviews.llvm.org/D38414 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h Index: clangd/ClangdLSPServer.h

[PATCH] D37973: [clang-format] Fix regression about short functions after #else

2017-10-02 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. This test looks like it was intended to catch some case, maybe we're now mishandling some case like if (foo) { } else { // this can now be merged } But there's no

[PATCH] D38464: [clangd] less boilerplate in RPC dispatch

2017-10-02 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 117374. sammccall added a comment. - clang-format https://reviews.llvm.org/D38464 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/JSONRPCDispatcher.cpp clangd/JSONRPCDispatcher.h clangd/Protocol.cpp clangd/Protocol.h

[PATCH] D38464: [clangd] less boilerplate in RPC dispatch

2017-10-02 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. Make the ProtocolHandlers glue between JSONRPCDispatcher and ClangdLSPServer generic. Eliminate small differences between methods, de-emphasize the unimportant distinction between notifications and methods. ClangdLSPServer is no longer responsible for producing a

[PATCH] D38414: [clangd] simplify ClangdLSPServer by private-inheriting callback interfaces. NFC

2017-09-29 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. There doesn't seem to be any real separation between the current three objects. Feel free to reject this if you find the current style valuable, though. (Mostly I'm just looking around for cleanups to help me understand the code).

[PATCH] D39086: Performance tracing facility for clangd.

2017-10-19 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. Herald added a subscriber: mgorny. This lets you visualize clangd's activity on different threads over time, and understand critical paths of requests and object lifetimes. The data produced can be visualized in Chrome (at chrome://tracing), or in a standalone

[PATCH] D38985: [refactor] Add support for editor commands that connect IDEs/editors to the refactoring actions

2017-10-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Hi Alex! I'm working on clangd, but am pretty new to the project, so forgive some naive questions. I'm a bit unclear at a high level what the new abstractions in this patch add, in terms of wiring refactorings up to clangd and other tools. My understanding is: we

[PATCH] D40089: [clangd] Make completion scores use 0-1 floats internally.

2017-11-15 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. This scale is much easier to mix with other signals, such as fuzzy match strength. Mostly NFC, but it does reorder some low-priority items that get folded together at a score of 0 (see completion-qualifiers.test). Removed the exact sortText from the testcases,

[PATCH] D40060: [clangd] Fuzzy match scorer

2017-11-14 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. Herald added a subscriber: mgorny. This will be used for rescoring code completion results based on partial identifiers. Short-term use: - we want to limit the number of code completion results returned to improve performance of global completion. The scorer

[PATCH] D40060: [clangd] Fuzzy match scorer

2017-11-14 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 122956. sammccall added a comment. clang-format https://reviews.llvm.org/D40060 Files: clangd/CMakeLists.txt clangd/FuzzyMatch.cpp clangd/FuzzyMatch.h unittests/clangd/CMakeLists.txt unittests/clangd/FuzzyMatchTests.cpp Index:

[PATCH] D39852: [clangd] Support returning a limited number of completion results.

2017-11-15 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL318287: [clangd] Support returning a limited number of completion results. (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D39852?vs=122342=122987#toc Repository: rL LLVM

[PATCH] D40060: [clangd] Fuzzy match scorer

2017-11-15 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 122986. sammccall added a comment. Trim memory usage and add comments. https://reviews.llvm.org/D40060 Files: clangd/CMakeLists.txt clangd/FuzzyMatch.cpp clangd/FuzzyMatch.h unittests/clangd/CMakeLists.txt unittests/clangd/FuzzyMatchTests.cpp

[PATCH] D40406: [clangd] Switch from YAMLParser to JSONExpr

2017-11-28 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE319159: [clangd] Switch from YAMLParser to JSONExpr (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D40406?vs=124163=124529#toc Repository: rCTE Clang Tools Extra

[PATCH] D40564: [clangd] Simplify common JSON-parsing patterns in Protocol.

2017-11-28 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. Herald added subscribers: cfe-commits, klimek. This makes the parse() functions about as short as they can be given the current signature, and moves all array-traversal etc code to a central location. We keep the ability to distinguish between optional and

[PATCH] D40409: [Tooling] Acknowledge that many CompilationDatabases don't support enumeration.

2017-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. Herald added a subscriber: klimek. Provide default implementations so that only getCompileCommands() is mandatory. https://reviews.llvm.org/D40409 Files: include/clang/Tooling/CompilationDatabase.h lib/Tooling/CompilationDatabase.cpp Index:

[PATCH] D40409: [Tooling] Acknowledge that many CompilationDatabases don't support enumeration.

2017-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: include/clang/Tooling/CompilationDatabase.h:122 + /// can enumerate their source files. + virtual std::vector getAllFiles() const { return {}; } grandinj wrote: > I know very little about LLVM's standards, so

[PATCH] D40409: [Tooling] Acknowledge that many CompilationDatabases don't support enumeration.

2017-11-24 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL318943: [Tooling] Acknowledge that many CompilationDatabases don't support enumeration. (authored by sammccall). Repository: rL LLVM https://reviews.llvm.org/D40409 Files:

[PATCH] D40406: [clangd] Switch from YAMLParser to JSONExpr

2017-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 124163. sammccall marked an inline comment as done. sammccall added a comment. Herald added a subscriber: ilya-biryukov. Use llvm::toString https://reviews.llvm.org/D40406 Files: clangd/ClangdLSPServer.cpp clangd/JSONExpr.h

[PATCH] D40406: [clangd] Switch from YAMLParser to JSONExpr

2017-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clangd/JSONRPCDispatcher.h:47 + // Whether output should be pretty-printed. + const bool Pretty; + ioeric wrote: > It seems that we are piggybacking a state of clangd with `JSONOutput`. It > might make sense to

[PATCH] D40399: [clangd] Add missing (but documented!) JSONExpr typed accessors

2017-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clangd/JSONExpr.h:368 +// Typed accessors return None/nullptr if the element has the wrong type. +llvm::Optional null(size_t I) const { + return (*this)[I].null(); ioeric wrote: > Why is this needed?

[PATCH] D40399: [clangd] Add missing (but documented!) JSONExpr typed accessors

2017-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 124203. sammccall marked 2 inline comments as done. sammccall added a comment. Renames to address review comments https://reviews.llvm.org/D40399 Files: clangd/ClangdUnit.cpp clangd/JSONExpr.cpp clangd/JSONExpr.h clangd/JSONRPCDispatcher.cpp

[PATCH] D40399: [clangd] Add missing (but documented!) JSONExpr typed accessors

2017-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks for the review! I think you're right about new names. Comment at: clangd/JSONExpr.h:368 +// Typed accessors return None/nullptr if the element has the wrong type. +llvm::Optional null(size_t I) const { + return (*this)[I].null();

[PATCH] D40439: [Tooling] Remove file/command enumeration from CompilationDatabase.

2017-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. Herald added a subscriber: klimek. In practice, these methods aren't used, and several databases don't implement them. They remain in libclang as a stub for binary compatibility. c-index-tests are modified not to rely on them. https://reviews.llvm.org/D40439

[PATCH] D40399: [clangd] Add missing (but documented!) JSONExpr typed accessors

2017-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 124204. sammccall added a comment. rebase https://reviews.llvm.org/D40399 Files: clangd/JSONExpr.cpp clangd/JSONExpr.h unittests/clangd/JSONExprTests.cpp Index: unittests/clangd/JSONExprTests.cpp

[PATCH] D40450: [clangd] Refactoring of GlobalCompilationDatabase

2017-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. The interface used is now tooling::CompilationDatabase. The various behaviors of the existing CDB implementation is decomposed into several classes responsible for one aspect each. - The fallback commands are now a FixedCompilationDatabase. For now, this is done

[PATCH] D39836: [clangd] Drop impossible completions (unavailable or inaccessible)

2017-11-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clangd/tool/ClangdMain.cpp:169 + clangd::CodeCompleteOptions CCOpts; + CCOpts.EnableSnippets = EnableSnippets; + CCOpts.IncludeIneligibleResults = IncludeIneligibleResults; ilya-biryukov wrote: > We should also set

[PATCH] D39882: [clangd] Filter completion results by fuzzy-matching identifiers.

2017-11-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 124043. sammccall added a comment. Rebase https://reviews.llvm.org/D39882 Files: clangd/ClangdUnit.cpp unittests/clangd/ClangdTests.cpp Index: unittests/clangd/ClangdTests.cpp === ---

[PATCH] D40089: [clangd] Make completion scores use 0-1 floats internally.

2017-11-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 124044. sammccall marked 3 inline comments as done. sammccall added a comment. address review comments https://reviews.llvm.org/D40089 Files: clangd/ClangdUnit.cpp test/clangd/authority-less-uri.test test/clangd/completion-items-kinds.test

[PATCH] D40089: [clangd] Make completion scores use 0-1 floats internally.

2017-11-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked 4 inline comments as done. sammccall added a comment. Thanks for the quick review, took me a while to get back to this but I do care about it! Comment at: clangd/ClangdUnit.cpp:387 std::string sortText() const { std::string S, NameStorage;

[PATCH] D39882: [clangd] Filter completion results by fuzzy-matching identifiers.

2017-11-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In https://reviews.llvm.org/D39882#932858, @ilya-biryukov wrote: > We definitely need to: > > - Rebase this change on top of current head (to account for limits and > scoring) Done. There's very little interaction - for now the match doesn't affect scoring, we're

[PATCH] D40301: [clangd] Ensure preamble outlives the AST

2017-11-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/ClangdUnit.h:51 +struct PreambleData; + can you move the definition here to avoid the extra decl? (I tend to find this more

[PATCH] D39836: [clangd] Drop impossible completions (unavailable or inaccessible)

2017-11-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 124066. sammccall added a comment. Address review comments https://reviews.llvm.org/D39836 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h

[PATCH] D40132: [clangd] Tracing improvements

2017-11-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 124069. sammccall marked an inline comment as done. sammccall added a comment. Address review comment. https://reviews.llvm.org/D40132 Files: clangd/ClangdUnit.cpp clangd/JSONRPCDispatcher.cpp clangd/JSONRPCDispatcher.h clangd/Trace.cpp

[PATCH] D40132: [clangd] Tracing improvements

2017-11-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clangd/JSONRPCDispatcher.h:78 llvm::Optional ID; + std::unique_ptr Tracer; }; ilya-biryukov wrote: > Why do we need `unique_ptr`? Are `Span`s non-movable? Spans aren't movable, they have an explicitly declared

[PATCH] D40060: [clangd] Fuzzy match scorer

2017-11-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 124096. sammccall marked 15 inline comments as done. sammccall added a comment. Addressing review comments and generally improving comments. https://reviews.llvm.org/D40060 Files: clangd/CMakeLists.txt clangd/FuzzyMatch.cpp clangd/FuzzyMatch.h

[PATCH] D40060: [clangd] Fuzzy match scorer

2017-11-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks for the review, and sorry for the subtlety of the code and sparse comments. It should be a little better now, please let me know which parts aren't clear enough. Comment at: clangd/FuzzyMatch.cpp:69 +: NPat(std::min(MaxPat,

[PATCH] D40132: [clangd] Tracing improvements

2017-11-23 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL318928: [clangd] Tracing improvements (authored by sammccall). Repository: rL LLVM https://reviews.llvm.org/D40132 Files: clang-tools-extra/trunk/clangd/ClangdUnit.cpp

[PATCH] D39836: [clangd] Drop impossible completions (unavailable or inaccessible)

2017-11-23 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL318925: [clangd] Drop impossible completions (unavailable or inaccessible) (authored by sammccall). Repository: rL LLVM https://reviews.llvm.org/D39836 Files:

[PATCH] D40089: [clangd] Make completion scores use 0-1 floats internally.

2017-11-23 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL318927: [clangd] Make completion scores use 0-1 floats internally. (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D40089?vs=124044=124099#toc Repository: rL LLVM

[PATCH] D40564: [clangd] Simplify common JSON-parsing patterns in Protocol.

2017-11-29 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL319309: [clangd] Simplify common JSON-parsing patterns in Protocol. (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D40564?vs=124585=124713#toc Repository: rL LLVM

[PATCH] D40596: [clangd] New conventions for JSON-marshalling functions, centralize machinery

2017-11-29 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. Herald added subscribers: cfe-commits, klimek. - JSON<->Obj interface is now ADL functions, so they play nicely with enums - recursive vector/map parsing and ObjectMapper moved to JSONExpr and tested - renamed (un)parse to (de)serialize, since text -> JSON is

[PATCH] D40060: [clangd] Fuzzy match scorer

2017-11-30 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 124968. sammccall marked 5 inline comments as done. sammccall added a comment. - added more VSCode tests, and made test assert matched characters. This uncovered algorithm problems - cache now includes "did previous character match" in the key (scoring

[PATCH] D40060: [clangd] Fuzzy match scorer

2017-11-30 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks @ilya-biryukov, @inspirer, @klimek for the helpful comments! I've addressed hopefully the most important and added more rigorous testing. Sorry for the large delta, the most invasive change was of course adding the extra dimension to the scoring table. (Which

[PATCH] D40596: [clangd] New conventions for JSON-marshalling functions, centralize machinery

2017-11-30 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL319478: [clangd] New conventions for JSON-marshalling functions, centralize machinery (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D40596?vs=124715=125004#toc Repository:

[PATCH] D39882: [clangd] Filter completion results by fuzzy-matching identifiers.

2017-11-30 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. @ilya-biryukov Ping... this is the patch we wanted to land this week, so long result sets are correct for user testing next week. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D39882 ___ cfe-commits

[PATCH] D40596: [clangd] New conventions for JSON-marshalling functions, centralize machinery

2017-11-30 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rCTE319478: [clangd] New conventions for JSON-marshalling functions, centralize machinery (authored by sammccall). Changed prior to commit:

[PATCH] D39882: [clangd] Filter completion results by fuzzy-matching identifiers.

2017-11-30 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 125009. sammccall added a comment. Herald added a subscriber: klimek. Rebase and remove debug output. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D39882 Files: clangd/ClangdUnit.cpp unittests/clangd/ClangdTests.cpp Index:

  1   2   3   4   5   6   7   8   9   >