ilya-biryukov created this revision.
https://reviews.llvm.org/D34107
Files:
clangd/ClangdServer.cpp
clangd/ClangdServer.h
unittests/clangd/ClangdTests.cpp
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/Clangd
ilya-biryukov updated this revision to Diff 102300.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.
Fixed formatting, added a space (see krasimir's comment)
https://reviews.llvm.org/D34107
Files:
clangd/ClangdServer.cpp
clangd/ClangdServer.h
unittests/clangd
ilya-biryukov added inline comments.
Comment at: unittests/clangd/ClangdTests.cpp:452
+ StringRef(OverridenSourceContents))
+.Value;
+EXPECT_TRUE(ContainsItem(CodeCompletionResultsOverriden, "cbc"));
krasimir wrote:
> Is t
ilya-biryukov created this revision.
This is a reapplied r305280 with a fix to the crash found by build bots
(StringRef to an out-of-scope local std::string).
https://reviews.llvm.org/D34146
Files:
clangd/ClangdServer.cpp
clangd/ClangdServer.h
unittests/clangd/ClangdTests.cpp
Index: unit
ilya-biryukov added inline comments.
Comment at: clangd/ClangdServer.cpp:190
+ llvm::Optional OverridenContents) {
+ std::string DraftStorage;
+ if (!OverridenContents) {
This is the relevant line that changed from the previous review.
ilya-biryukov created this revision.
ClangdServer was owning objects passed to it in constructor for no good reason.
Lots of stuff was moved from the heap to the stack thanks to this change.
https://reviews.llvm.org/D34148
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/
ilya-biryukov created this revision.
https://reviews.llvm.org/D34151
Files:
clangd/ClangdServer.cpp
clangd/ClangdServer.h
unittests/clangd/ClangdTests.cpp
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/Clangd
ilya-biryukov added inline comments.
Comment at: clangd/ClangdUnit.cpp:156
assert(CCS->getTypedText());
+CCS->getPriority();
Item.kind = getKind(Result.CursorKind);
This is a no-op, commited incidentally?
Comment at: c
ilya-biryukov added inline comments.
Comment at: clangd/ClangdLSPServer.h:26
/// dispatch and ClangdServer together.
-class ClangdLSPServer {
+class ClangdLSPServer : public DiagnosticsConsumer {
public:
I would not expect ClangdLSPServer to be passed as a Diag
ilya-biryukov added a comment.
Hi @malaperle-ericsson. Thanks for the patch.
IndexingAction has a very simple interface, exactly what clangd needs and
nothing more.
BTW, we had a bug open for that: https://bugs.llvm.org/show_bug.cgi?id=33261,
feel free to reassign to yourself.
=
ilya-biryukov created this revision.
Herald added a subscriber: mgorny.
https://reviews.llvm.org/D34287
Files:
include/clang/Frontend/ASTUnit.h
include/clang/Frontend/PrecompiledPreamble.h
lib/Frontend/ASTUnit.cpp
lib/Frontend/CMakeLists.txt
lib/Frontend/PrecompiledPreamble.cpp
Index:
ilya-biryukov updated this revision to Diff 102993.
ilya-biryukov added a comment.
Removed a stray comment (of a previously removed declaration).
https://reviews.llvm.org/D34287
Files:
include/clang/Frontend/ASTUnit.h
include/clang/Frontend/PrecompiledPreamble.h
lib/Frontend/ASTUnit.cpp
ilya-biryukov added inline comments.
Comment at: include/clang/Frontend/PrecompiledPreamble.h:248
+/// doesn't restore the state \p CI had before calling AddImplicitPreamble,
only
+/// clears relevant settings, so that preamble is disabled in \p CI.
+} // namespace clang
---
ilya-biryukov updated this revision to Diff 103053.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.
- Added default initializers to PreambleFileHash.
- Update file comments to address klimek's questions.
https://reviews.llvm.org/D34287
Files:
include/clang/Front
ilya-biryukov added inline comments.
Comment at: include/clang/Frontend/PrecompiledPreamble.h:42-43
+/// destructors.
+/// An assertion will fire if two PCHTempFiles are created with the same name,
+/// so it's not intended to be used outside preamble-handling.
+class TempPCHFile
ilya-biryukov updated this revision to Diff 103203.
ilya-biryukov added a comment.
Removed PossiblyOwnedBuffer, added an extra copy instead.
This makes the code much simpler.
https://reviews.llvm.org/D34287
Files:
include/clang/Frontend/ASTUnit.h
include/clang/Frontend/PrecompiledPreamble.h
ilya-biryukov updated this revision to Diff 103216.
ilya-biryukov added a comment.
- Made TempPCHFile an inner class of PrecompiledPreamble.
- Made PreambleFileHash an inner class of PrecompiledPreamble.
- Changed stanalone functions to members.
- Removed some member accessors that were no longer
ilya-biryukov added inline comments.
Comment at: include/clang/Frontend/PrecompiledPreamble.h:124
+/// CanReusePreamble + AddImplicitPreamble to make use of it.
+class PrecompiledPreamble {
+public:
klimek wrote:
> ilya-biryukov wrote:
> > klimek wrote:
> > > If
ilya-biryukov created this revision.
It used to always call into the RealFileSystem before.
https://reviews.llvm.org/D34469
Files:
include/clang/Frontend/Utils.h
lib/Frontend/ASTUnit.cpp
lib/Frontend/CreateInvocationFromCommandLine.cpp
Index: lib/Frontend/CreateInvocationFromCommandLine
ilya-biryukov created this revision.
https://reviews.llvm.org/D34470
Files:
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/ClangdUnitStore.h
Index: clangd/ClangdUnitStore.h
=
ilya-biryukov added a comment.
Looks good now, but I think we definitely need to change
`unique_ptr>` to `vector` before submitting it.
I won't be available until next Wednesday, so feel free to submit without my
final approval.
Comment at: clangd/ClangdUnit.cpp:276
+class D
ilya-biryukov created this revision.
ilya-biryukov added reviewers: sepavloff, bkramer.
- Fixed an assert in Sema::InstantiateFunctionDefinition and added support for
instantiating a function template with skipped body.
- Properly call setHasSkippedBody for FunctionTemplateDecl passed to
Sema::A
ilya-biryukov added inline comments.
Comment at: clangd/Trace.cpp:134
+ Callback = T->beginSpan(Ctx, Name);
+ if (!Callback)
+return;
sammccall wrote:
> I'm not sure this is useful. Tracers that aren't interested in args can't opt
> out by providing a null
ilya-biryukov updated this revision to Diff 126951.
ilya-biryukov added a comment.
- Renamed TracingSession to Session (it is always used qualified via
trace::Session anyway)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40488
Files:
clangd/ClangdUnit.cpp
clangd/JSONRPCDi
ilya-biryukov updated this revision to Diff 126957.
ilya-biryukov added a comment.
Merged with head
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40489
Files:
clangd/Trace.cpp
clangd/Trace.h
Index: clangd/Trace.h
===
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM modulo a few comments.
AFAIK, @Nebiroth does not have a commit access, so I'll land this with minor
tweaks to unblock the other patch.
Comment at: include
ilya-biryukov added a comment.
Maybe we could use a struct containing only `range` and `message` (e.g.
`pair`) as a key to search for fixits instead of `Diagnostic`?
Having an `operator <` that does not take all fields into account seems shaky.
Repository:
rCTE Clang Tools Extra
https://rev
ilya-biryukov requested changes to this revision.
ilya-biryukov added inline comments.
This revision now requires changes to proceed.
Comment at: clangd/ClangdServer.cpp:446
std::vector Result;
- Resources->getAST().get()->runUnderLock([Pos, &Result, this](ParsedAST *AST)
{
ilya-biryukov added a comment.
Here are a few more comments.
Comment at: clangd/CodeComplete.cpp:847
+ if (Opts.Index && SCInfo.SSInfo) {
+// FIXME: log warning with logger if sema code completion have collected
+// results.
NIT: FIXME(ioeric)?
Also,
ilya-biryukov added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:284
+void ClangdLSPServer::onCodeHover(Ctx C, TextDocumentPositionParams &Params) {
+
+ auto H = Server.findHover(C,
NIT: remove empty line
Comment at: clangd/ClangdSe
ilya-biryukov requested changes to this revision.
ilya-biryukov added a comment.
This revision now requires changes to proceed.
Thanks for addressing the comments quickly.
I took another look and added a few more comments.
This moves in the right direction, though, this is really close to landing.
ilya-biryukov added inline comments.
Comment at: lib/Sema/SemaDecl.cpp:12184
Decl *Sema::ActOnSkippedFunctionBody(Decl *Decl) {
- if (FunctionDecl *FD = dyn_cast_or_null(Decl))
+ if (FunctionDecl *FD = Decl->getAsFunction())
FD->setHasSkippedBody();
sepav
ilya-biryukov added inline comments.
Comment at: lib/Sema/SemaDecl.cpp:12184
Decl *Sema::ActOnSkippedFunctionBody(Decl *Decl) {
- if (FunctionDecl *FD = dyn_cast_or_null(Decl))
+ if (FunctionDecl *FD = Decl->getAsFunction())
FD->setHasSkippedBody();
ilya-
ilya-biryukov updated this revision to Diff 127473.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
- Added a check for null in ActOnSkippedBody
Repository:
rC Clang
https://reviews.llvm.org/D41237
Files:
lib/Sema/SemaDecl.cpp
lib/Sema/SemaTemplateInstantia
ilya-biryukov requested changes to this revision.
ilya-biryukov added inline comments.
This revision now requires changes to proceed.
Comment at: include/clang/Frontend/PrecompiledPreamble.h:249
+ /// from a CompilerInstance.
+ virtual void BeforeExecute(CompilerInstance &CI);
ilya-biryukov added inline comments.
Comment at: clangd/CodeComplete.cpp:334
+ Item.insertTextFormat = InsertTextFormat::PlainText;
+ // FIXME: sort symbols appropriately.
+ Item.sortText = "";
ioeric wrote:
> ilya-biryukov wrote:
> > NIT: FIXME(ioeric)?
> > A
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
LGTM. I still think that we should move the `SymbolIndex` out of the struct,
but don't want to block this patch.
Comment at: clangd/CodeComplete.h:64
+
+ // Populated internally by clangd, do not set.
+ ///
ilya-biryukov requested changes to this revision.
ilya-biryukov added a comment.
This revision now requires changes to proceed.
LGTM modulo the comment. Should I land this for you?
Comment at: include/clang/Frontend/PrecompiledPreamble.h:247
+ /// Can be used to store refere
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LG
Repository:
rC Clang
https://reviews.llvm.org/D41365
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llv
ilya-biryukov requested changes to this revision.
ilya-biryukov added a comment.
This revision now requires changes to proceed.
Another round of review
Comment at: clangd/ClangdUnit.cpp:113
+ CppFilePreambleCallbacks(IncludeReferenceMap &IRM)
+ : IRM(IRM) {}
---
ilya-biryukov added inline comments.
Comment at: clangd/index/Index.h:136
+ // Intern table for strings. Not StringPool as we don't refcount, just
insert.
+ llvm::StringSet Strings;
llvm::DenseMap Symbols;
A comment on why we use `BumpPtrAllocator` here mig
ilya-biryukov created this revision.
ilya-biryukov added reviewers: sepavloff, klimek.
Previsouly clang tried instantiating member initializers even if ctor
body was skipped, this caused spurious errors (see the test).
Repository:
rC Clang
https://reviews.llvm.org/D41492
Files:
lib/Sema/Se
ilya-biryukov added inline comments.
Comment at: clangd/XRefs.cpp:183
+ unsigned CharNumber = SourceMgr.getSpellingColumnNumber(
+ DeclMacrosFinder->getSearchedLocation());
+
ilya-biryukov wrote:
> Replace with `DeclMacrosFinder->getSearchedLocation
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added a subscriber: klimek.
To make building preambles faster and keep them smaller.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41495
Files:
clangd/ClangdUnit.cpp
Index: clangd/Clangd
ilya-biryukov added a comment.
I haven't done proper benchmarks, but here are some numbers from running this
on my machine:
Sizes of preamble with (before the change) and without (after the change)
functon bodies:
| File | Before | After |
| ClangdUnit.cpp | 49.58M
ilya-biryukov added inline comments.
Comment at: clangd/ClangdUnit.cpp:536
+ // so we set SkipFunctionBodies back to false after preamble is built.
+ assert(!CI->getFrontendOpts().SkipFunctionBodies);
+ CI->getFrontendOpts().SkipFunctionBodies = true;
-
ilya-biryukov updated this revision to Diff 127995.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
- Updated a comment
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41495
Files:
clangd/ClangdUnit.cpp
Index: clangd/ClangdUnit.cpp
ilya-biryukov added inline comments.
Comment at: clangd/index/Index.cpp:39
+ [](const Symbol &S, const SymbolID &I) {
+ return S.ID == I;
+ });
Should this be `S.ID < I`?
ilya-biryukov added a comment.
IIUC, you want to pass an overlay so that clang-tidy will read the file opened
in the IDE from a different directory?
Could you provide add `-ivfsoverlay` option to your compile command instead?
$ clang --help | grep vfs
-ivfsoverlay Overlay the virtual fi
ilya-biryukov added a comment.
I'm not sure if it's ok to ignore the shared `FileManager` here.
@klimek, @bkramer, @alexfh, does tooling library rely on having the same
`FileManager` for all invocations? Is it just a performance optimization or
there's more to it?
Comment at:
ilya-biryukov added a comment.
In https://reviews.llvm.org/D41535#963859, @vladimir.plyashkun wrote:
> Unfortunately, `-ivfsoverlay` in the compile commands works for the compiler
> invocation, but it doesn't work for tooling.
This looks like a bug in tooling, but let's wait for responses on t
ilya-biryukov added inline comments.
Comment at: lib/Tooling/Tooling.cpp:287
+ if (Files->getVirtualFileSystem() != VirtualFileSystem) {
+Files = new FileManager(Invocation->getFileSystemOpts(),
VirtualFileSystem);
+ }
vladimir.plyashkun wrote:
> ilya-biry
ilya-biryukov added a comment.
In https://reviews.llvm.org/D41495#965627, @nik wrote:
> Hmm, could libclang profit from something like this, too? (or does it
> already?)
It could. The problem with ASTUnit is that it returns all diagnostics from
`store_diag_begin`/`end`, not just the ones from
ilya-biryukov added inline comments.
Comment at: include/clang/Frontend/ASTUnit.h:193
/// some number of calls.
- unsigned PreambleRebuildCounter;
+ unsigned PreambleRebuildCountdownCounter;
+
NIT: Maybe shorten to `PreambleRebuildCountdown`?
It's not a grea
ilya-biryukov added inline comments.
Comment at: clangd/XRefs.cpp:68
+ // declaration, and it could be a forward declaration.
+ auto Def = std::find_if(D->redecls_begin(), D->redecls_end(),
+ [](const Decl *D) { return IsDefinition(D); });
-
ilya-biryukov added inline comments.
Comment at: clangd/tool/ClangdMain.cpp:122
+llvm::cl::desc(
+"YAML-format global symbol file to build the static index. It is only "
+"available when 'enable-index-based-completion' is enabled."),
Let's als
ilya-biryukov added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:89
+// violations.
+if (ND->isInAnonymousNamespace())
return true;
Why don't we include symbols from anonymous namespaces too?
They are very similar to static symbols
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM (see the review comment about adding a comment in the code too)
Comment at: clangd/index/SymbolCollector.cpp:89
+// violations.
+if (ND->isInAnonym
ilya-biryukov added a comment.
In https://reviews.llvm.org/D41823#970725, @hokein wrote:
> With this patch, the index-based code completion will not provide any
> candidates for `ns1::MyClass::^` (as the index-based completion drops all
> results from clang's completion engine), we need a way
ilya-biryukov added a comment.
@malaperle, hi! Both new diff and updating this works, so feel free the one
that suits you best. I tend to look over the whole change on each new round of
reviews anyway.
Comment at: clangd/ClangdUnit.cpp:113
+ CppFilePreambleCallbacks(Includ
ilya-biryukov added inline comments.
Comment at: clangd/ClangdLSPServer.h:40
+ bool BuildDynamicSymbolIndex,
+ std::unique_ptr StaticIdx);
sammccall wrote:
> is there a reason ClangdServer should own this? I can imagine this
>
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added a subscriber: klimek.
This patch removes hidden items from code completion.
Hidden items are the ones that are hidden by other items (e.g., by
other items in the child scopes).
This patch addresses a parti
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
LGTM. I'd add a comment pointing to code in include-fixer we're borrowing,
though. (See the relevant comment)
Comment at: clangd/index/SymbolCollector.cpp:80
+: decl(unless
ilya-biryukov added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:80
+: decl(unless(isExpansionInMainFile())),
+hasDeclContext(anyOf(namespaceDecl(),
translationUnitDecl(),
+*D, *ASTCtx)
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added a subscriber: klimek.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41907
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
ilya-biryukov added inline comments.
Comment at: clangd/ClangdUnit.h:51
+using IncludeReferenceMap = std::unordered_map;
+
We use `unordered_map` as a `vector>` here. (i.e. we never look up
values by key, because we don't only the range, merely a point in the
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43648
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: ioeric, jkorous-apple, klimek.
Found by asan. Fiddling with code completion AST after
FrontendAction::Exceute can lead to errors.
Calling the callback in ProcessCodeCompleteResults to make sur
ilya-biryukov updated this revision to Diff 136707.
ilya-biryukov added a comment.
Remove code refactoring from the patch.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44000
Files:
clangd/CodeComplete.cpp
Index: clangd/CodeComplete.cpp
=
ilya-biryukov updated this revision to Diff 136710.
ilya-biryukov added a comment.
Remove a trace for "sema cleanup", it is not very informative now that we run
the callback under the "sema completion" trace
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44000
Files:
clangd/
ilya-biryukov updated this revision to Diff 136714.
ilya-biryukov marked 7 inline comments as done.
ilya-biryukov added a comment.
Addressed review comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44000
Files:
clangd/CodeComplete.cpp
Index: clangd/CodeComplete.cpp
===
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM, but note the unreachable code comment
Comment at: unittests/clangd/FuzzyMatchTests.cpp:35
+
+ friend raw_ostream &operator<<(raw_ostream &OS, const Expec
ilya-biryukov added inline comments.
Comment at: clangd/ClangdServer.h:94
+ /// Returns a statically allocated instance. (Stateless, safe to share).
+ static RealFileSystemProvider *get();
+
Maybe return a reference instead of a pointer?
Comm
ilya-biryukov created this revision.
ilya-biryukov added reviewers: hokein, ioeric, sammccall.
Herald added subscribers: jkorous-apple, mgorny, klimek.
The new implementation attaches notes to diagnostic message and shows
the original diagnostics in the message of the note.
Repository:
rCTE Cl
ilya-biryukov added a comment.
This revision is still missing tests, but I wanted to get feedback for the
overall design and the new diagnostic messages. So sending out for review now.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44142
_
ilya-biryukov added inline comments.
Comment at: clangd/Diagnostics.h:28
+/// DiagList.
+struct PersistentDiag {
+ llvm::StringRef Message;
This could probably use a better name
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44142
_
ilya-biryukov added a comment.
I would vouch for adding a log level instead. It's a very well understood
concept that certainly covers this use-case and can be useful in other places.
WDYT?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44226
ilya-biryukov added a comment.
In https://reviews.llvm.org/D44226#1030639, @simark wrote:
> In https://reviews.llvm.org/D44226#1030625, @ilya-biryukov wrote:
>
> > I would vouch for adding a log level instead. It's a very well understood
> > concept that certainly covers this use-case and can be
ilya-biryukov added inline comments.
Comment at: clang-tools-extra/clang-tidy/add_new_check.py:24
with open(filename, 'r') as f:
-lines = f.readlines()
+lines = [x for x in f.readlines()]
`readlines()` returns a list in Python3 too.
Maybe remove list
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM.
I think this was part of initial changed and I asked to make it this way, but I
don't remember why.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44251
ilya-biryukov added a comment.
Sorry, being pedantic here... We're not saving time on parsing, but on walking
over the AST :-)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44251
___
cfe-commits mailing list
cfe-commits@lists.llvm
ilya-biryukov added inline comments.
Comment at: clangd/Logger.cpp:19
Logger *L = nullptr;
+bool Verbose_ = false;
} // namespace
Could we move the flag to implementation of `Logger`?
I.e.:
```
class Logger {
virtual log(const llvm::Twine &Message, bool Verbo
ilya-biryukov added inline comments.
Comment at: clangd/Logger.cpp:19
Logger *L = nullptr;
+bool Verbose_ = false;
} // namespace
simark wrote:
> ilya-biryukov wrote:
> > Could we move the flag to implementation of `Logger`?
> > I.e.:
> > ```
> > class Logger {
ilya-biryukov added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:329
+ {"title",
+ llvm::formatv("Apply FixIt {0}", GetFixitMessage(D.message))},
{"command", ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND},
sammccall wrote
ilya-biryukov updated this revision to Diff 137589.
ilya-biryukov marked 9 inline comments as done.
ilya-biryukov added a comment.
This is not final, there are still unadressed comments.
- Significantly simplified the implementation by removing the DiagList, etc.
- Addressed some of the rest of t
ilya-biryukov requested changes to this revision.
ilya-biryukov added inline comments.
This revision now requires changes to proceed.
Comment at: clangd/ClangdLSPServer.cpp:101
json::obj{
-{"textDocumentSync", 1},
+{"textDocumentSync", 2},
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM. Thanks for the fix!
https://reviews.llvm.org/D44217
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm
ilya-biryukov added inline comments.
Comment at: clangd/ClangdServer.cpp:557
// 64-bit unsigned integer.
if (Version < LastReportedDiagsVersion)
return;
When you'll try remove the `DraftMgr`, this piece of code will be hard to
refactor because i
ilya-biryukov added inline comments.
Comment at: unittests/clangd/XRefsTests.cpp:587
- const char *HeaderContents = R"cpp([[]]int a;)cpp";
+ const char *HeaderContents = R"cpp([[]]#ifndef TEST_H_
+ #define TEST_H_
I would g
ilya-biryukov created this revision.
ilya-biryukov added reviewers: bkramer, sammccall, ioeric, hokein.
The relevant failing assertion message is:
../tools/clang/lib/Sema/SemaInit.cpp:8411: PerformCopyInitialization():
Assertion `InitE && "No initialization expression?"' failed.
See the added te
ilya-biryukov updated this revision to Diff 137743.
ilya-biryukov added a comment.
- Added a comment
Repository:
rC Clang
https://reviews.llvm.org/D44300
Files:
lib/Sema/SemaOverload.cpp
test/CodeCompletion/enable-if-attr-crash.cpp
Index: test/CodeCompletion/enable-if-attr-crash.cpp
==
ilya-biryukov updated this revision to Diff 137977.
ilya-biryukov marked 8 inline comments as done.
ilya-biryukov added a comment.
Addressed review comments.
The only thing that's left on my todo-list is a test for toLSPDiags().
Everything else should be ready.
Repository:
rCTE Clang Tools Ex
ilya-biryukov updated this revision to Diff 137985.
ilya-biryukov added a comment.
- Added unit test for toLSPDiags
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44142
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.
ilya-biryukov updated this revision to Diff 137987.
ilya-biryukov marked 14 inline comments as done.
ilya-biryukov added a comment.
- Addressed review comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44142
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
cla
ilya-biryukov added inline comments.
Comment at: clangd/Diagnostics.h:35
+ DiagnosticsEngine::Level Severity;
+ llvm::SmallVector FixIts;
+ // Since File is only descriptive, we store a separate flag to distinguish
sammccall wrote:
> sammccall wrote:
> > As di
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:329
+ {"title",
+ llvm::formatv("Apply FixIt {0}", GetFixitMessage(D.message))},
{"command", ExecuteCommandParams::CLANGD_APPL
ilya-biryukov added inline comments.
Comment at: clangd/XRefs.cpp:201
std::vector MacroInfos = DeclMacrosFinder->takeMacroInfos();
+ if (!MacroInfos.empty()) {
+for (auto Item : MacroInfos) {
I wonder whether we should fix the `DeclrationAndMacrosFinder`
ilya-biryukov updated this revision to Diff 138004.
ilya-biryukov added a comment.
- Replace equality comparison ops with matchers, they were only used in tests.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44142
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
c
ilya-biryukov accepted this revision.
ilya-biryukov added inline comments.
This revision is now accepted and ready to land.
Comment at: unittests/clangd/XRefsTests.cpp:587
- const char *HeaderContents = R"cpp([[]]int a;)cpp";
+ const char *HeaderContents = R"cpp([[]]#ifndef T
ilya-biryukov added inline comments.
Comment at: clangd/XRefs.cpp:201
std::vector MacroInfos = DeclMacrosFinder->takeMacroInfos();
+ if (!MacroInfos.empty()) {
+for (auto Item : MacroInfos) {
hokein wrote:
> ilya-biryukov wrote:
> > I wonder whether we sh
601 - 700 of 3283 matches
Mail list logo
