malaperle added inline comments.
Comment at: clangd/ClangdUnit.cpp:1173
+H.range = L.range;
+Ref = getDataBufferFromSourceRange(AST, SR, L);
+ }
malaperle wrote:
> malaperle wrote:
> > I get the same crash as I mentioned before if I hover on
malaperle added inline comments.
Comment at: clangd/Protocol.h:453
+struct MarkedString {
+ /**
+ * MarkedString can be used to render human readable text. It is either a
The doc should use /// like the others
https://reviews.llvm.org/D35894
_
malaperle added inline comments.
Comment at: clangd/ClangdUnit.cpp:1135
+RawComment *RC =
+AST.getASTContext().getRawCommentForDeclNoCache(LocationDecl);
+if (RC) {
Not sure why but I don't get the comments when I hover on "push_back"
malaperle added inline comments.
Comment at: clangd/ClangdUnit.h:320
+
+StringRef getDataBufferFromSourceRange(ParsedAST &AST, SourceRange SR,
+ Location L);
I think this can be only in the source file, in a an anonymous name
malaperle added inline comments.
Comment at: clangd/ClangdUnit.cpp:1029
+ SourceLocation LocStart = ValSourceRange.getBegin();
+ SourceLocation LocEnd = Lexer::getLocForEndOfToken(ValSourceRange.getEnd(),
0,
+ SourceMgr, Lang
malaperle added inline comments.
Comment at: clangd/ClangdUnit.cpp:1117
+
+ if (!DeclVector.empty()) {
+const Decl *LocationDecl = DeclVector[0];
malaperle wrote:
> I think we need to simplify this part a bit. I'll try to find a way. Feel
> free to wait unt
malaperle requested changes to this revision.
malaperle added inline comments.
This revision now requires changes to proceed.
Comment at: clangd/Protocol.cpp:498
+// Default Hover values
+Hover H;
+return json::obj{
remove, we have to return the conte
malaperle added a comment.
I copy/pasted the comments again so that you don't miss them.
Comment at: clangd/ClangdUnit.cpp:1117
+
+ if (!DeclVector.empty()) {
+const Decl *LocationDecl = DeclVector[0];
malaperle wrote:
> malaperle wrote:
> > I think we nee
malaperle added inline comments.
Comment at: clangd/ClangdUnit.cpp:1071
+Location getDeclarationLocation(ParsedAST &AST,
+const SourceRange &ValSourceRange) {
llvm::ErrorOr
Comment at: clangd/ClangdUnit.cpp:107
malaperle added inline comments.
Comment at: clangd/ClangdUnit.cpp:1117
+
+ if (!DeclVector.empty()) {
+const Decl *LocationDecl = DeclVector[0];
malaperle wrote:
> malaperle wrote:
> > malaperle wrote:
> > > I think we need to simplify this part a bit. I'll
malaperle requested changes to this revision.
malaperle added a comment.
This revision now requires changes to proceed.
very minor comments
Comment at: clangd/ClangdServer.h:288
+ /// Get document highlights for a symbol hovered on.
+ llvm::Expected>>
+ findDocumentHighlight
malaperle requested changes to this revision.
malaperle added inline comments.
This revision now requires changes to proceed.
Comment at: clangd/ClangdLSPServer.cpp:228
llvm::toString(Items.takeError()));
+
C.reply(json::ary(Items->Value));
---
malaperle added inline comments.
Comment at: clangd/ClangdServer.cpp:526
+if (!AST)
+ llvm::make_error(
+ "invalid AST",
missing return?
I get a warning that looks legit:
./tools/clang/tools/extra/clangd/ClangdServer.cpp:526:7: warning: igno
malaperle added inline comments.
Comment at: clangd/Protocol.h:26
#include "llvm/ADT/Optional.h"
-#include
+#include "llvm/Support/YAMLParser.h"
#include
Nebiroth wrote:
> malaperle wrote:
> > revert this change?
> #include is not needed.
I meant removing YA
malaperle accepted this revision.
malaperle added a comment.
This looks good to me. @ilya-biryukov Any objections?
I think we can do further iterations later to handle macros and other specific
cases (multiple Decls at the position, etc) . It's already very useful
functionality.
Repository:
malaperle requested changes to this revision.
malaperle added inline comments.
This revision now requires changes to proceed.
Comment at: clangd/ClangdServer.cpp:528
+ Value = llvm::make_error(
+ "invalid AST",
+ llvm::errc::invalid_argument);
-
malaperle added a comment.
> - Look up in parent directories for a .clang-format file. Use "LLVM" as
> fallback style.
> - Move cached fixits to ClangdServer, remove fixits once the client closes
> the document.
> - Fix translations between LSP rows/cols (0-based) and clang row/cols
> (1-based)
malaperle added a comment.
Can you also reformat the code too with clang-format? I think there's a few
things in ClangdUnit.h/.cpp
Comment at: clangd/ClangdServer.cpp:521
+ std::shared_ptr Resources = Units.getFile(File);
+ assert(Resources && "Calling findDocumentHighlights
malaperle added a comment.
Sorry for the delay, I'm looking into this now. I actually don't query (or even
store!) any symbol names in the index interface now, only Occurrences queried
by USR which is enough for findReferences and findDefinitions. It looks like
storing names is the main part in
malaperle requested changes to this revision.
malaperle added a comment.
This revision now requires changes to proceed.
Sorry I forgot to submit this additional comment in my last review pass :(
Comment at: clangd/Protocol.cpp:365
+ {"range", toJSON(DH.range)},
+ {"ki
malaperle accepted this revision.
malaperle added a subscriber: sammccall.
malaperle added a comment.
This looks good to me. @ilya-biryukov, @sammccall Any objections?
I think we can do further iterations later to handle macros and other specific
cases (multiple Decls at the position, etc) . It'
malaperle added a comment.
You can get the preprocessor from the ASTContext, no?
Repository:
rC Clang
https://reviews.llvm.org/D40884
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi
malaperle added a comment.
In https://reviews.llvm.org/D40884#946630, @ioeric wrote:
> In https://reviews.llvm.org/D40884#946506, @malaperle wrote:
>
> > You can get the preprocessor from the ASTContext, no?
>
>
> I don't think `ASTContext` contains preprocessor information.
My bad, it was the
malaperle added a comment.
Hi! Have you looked into https://reviews.llvm.org/D40548 ? Maybe we need to
coordinate the two a bit.
Comment at: clangd/Symbol.h:37
+// The class presents a C++ symbol, e.g. class, function.
+struct Symbol {
+ // The symbol identifier, using USR.
-
malaperle added a comment.
In https://reviews.llvm.org/D40897#946911, @hokein wrote:
> Our rough plan would be
>
> 1. Define the Symbol structure.
> 2. Design the interfaces of SymbolIndex, ASTIndex.
> 3. Combine 1) and 2) together to make global code completion work (we'd use
> YAML dataset for
malaperle added inline comments.
Comment at: clangd/Symbol.h:37
+// The class presents a C++ symbol, e.g. class, function.
+struct Symbol {
+ // The symbol identifier, using USR.
sammccall wrote:
> hokein wrote:
> > malaperle wrote:
> > > I think it would be nic
malaperle added inline comments.
Comment at: include/indexstore/IndexStoreCXX.h:84
+
+ std::pair getLineCol() {
+unsigned line, col;
ioeric wrote:
> nathawes wrote:
> > malaperle wrote:
> > > From what I understand, this returns the beginning of the occurren
malaperle added inline comments.
Comment at: clangd/Symbol.h:23
+ // The path of the source file where a symbol occurs.
+ std::string FilePath;
+ // The offset to the first character of the symbol from the beginning of the
malaperle wrote:
> sammccall wrote:
>
malaperle added a comment.
As a follow-up, here's the interface for querying the index that I am using
right now. It's meant to be able to retrieve from any kind of "backend", i.e.
in-memory, ClangdIndexDataStore, libIndexStore, etc. I was able to implement
"Open Workspace Symbol" (which is clo
malaperle added a comment.
In https://reviews.llvm.org/D39050#948500, @akyrtzi wrote:
> @malaperle, to clarify we are not suggesting that you write your own parser,
> the suggestion is to use clang in 'fast-scan' mode to get the structure of
> the declarations of a single file, see `CXTranslati
malaperle added a comment.
(From https://reviews.llvm.org/D40548) Here's the interface for querying the
index that I am using right now. It's meant to be able to retrieve from any
kind of "backend", i.e. in-memory, ClangdIndexDataStore, libIndexStore, etc. I
was able to implement "Open Workspac
malaperle added inline comments.
Comment at: clangd/Symbol.h:37
+// The class presents a C++ symbol, e.g. class, function.
+struct Symbol {
+ // The symbol identifier, using USR.
malaperle wrote:
> sammccall wrote:
> > hokein wrote:
> > > malaperle wrote:
> > >
malaperle added inline comments.
Comment at: clangd/Symbol.h:37
+// The class presents a C++ symbol, e.g. class, function.
+struct Symbol {
+ // The symbol identifier, using USR.
sammccall wrote:
> malaperle wrote:
> > malaperle wrote:
> > > sammccall wrote:
> >
malaperle requested changes to this revision.
malaperle added inline comments.
This revision now requires changes to proceed.
Comment at: clangd/ClangdServer.cpp:292
+
+ if (path.compare(path.length() - 4, 4, ".cpp") == 0) {
+path = path.substr(0, (path.length() - 4));
-
malaperle added a comment.
In the commit message, you could add that we will use and index of symbols
later to be able to switch from header to source file when the file names don't
match. This is more or less the justification of why we want this in Clangd and
not on the client.
https://revi
malaperle requested changes to this revision.
malaperle added a comment.
This revision now requires changes to proceed.
Can you also update your code with the latest SVN trunk? The patch does not
apply cleanly anymore.
Comment at: clangd/ClangdServer.cpp:11
#include "ClangdSe
malaperle added a comment.
In https://reviews.llvm.org/D35894#828702, @ilya-biryukov wrote:
> I just wanted to take a step back and discuss what we want to get from code
> hover in the first place.
>
> I would expect a typical code hover to be a bit more involved and include:
>
> - "kind" of a s
malaperle added a comment.
In https://reviews.llvm.org/D35894#829109, @ilya-biryukov wrote:
> > I think all of those would be great. Our objective is to bring basic but
> > correct features that will put us close to parity with Eclipse CDT, so that
> > our users can transition. In CDT only the
malaperle added a comment.
@Nebiroth I think it's OK to put this on hold until we make the "semantic"
hover and figure out how to have both. From our perspective, this is going
beyond parity of what we had before but it's seems like the right thing to do.
https://reviews.llvm.org/D35894
___
malaperle added a comment.
In https://reviews.llvm.org/D35894#830178, @ilya-biryukov wrote:
> In https://reviews.llvm.org/D35894#829190, @malaperle wrote:
>
> > @Nebiroth I think it's OK to put this on hold until we make the "semantic"
> > hover and figure out how to have both. From our perspect
malaperle added inline comments.
Comment at: clangd/ProtocolHandlers.cpp:260
+ Dispatcher.registerHandler(
+ "textDocument/switchSourceHeader",
+ llvm::make_unique(Out, Callbacks));
ilya-biryukov wrote:
> ilya-biryukov wrote:
> > I don't see this metho
malaperle created this revision.
Herald added subscribers: cfe-commits, MaskRay, ioeric, jkorous-apple,
ilya-biryukov, klimek.
This mitigates a possible issue in tests that print objects on failure. It is
possible that there will be two different instantiations of the printer template
for a given
malaperle added a comment.
In https://reviews.llvm.org/D44764#1045682, @ilya-biryukov wrote:
> In https://reviews.llvm.org/D44764#1045648, @simark wrote:
>
> > We could create a file `ClangdTesting.h" which includes everything tested
> > related (gtest, gmock, printers, syncapi, etc). The conven
malaperle updated this revision to Diff 139598.
malaperle added a comment.
Use a header for common inclusions for tests
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44764
Files:
unittests/clangd/ClangdTesting.h
unittests/clangd/ClangdTests.cpp
unittests/clangd/ClangdUni
malaperle created this revision.
Herald added subscribers: cfe-commits, MaskRay, ioeric, jkorous-apple, mgrang,
ilya-biryukov, mgorny, klimek.
This is a basic implementation of the "workspace/symbol" request which is
used to find symbols by a string query. Since this is similar to code completion
malaperle added a comment.
In https://reviews.llvm.org/D44882#1048043, @ilya-biryukov wrote:
> - Unconditionally storing much more symbols in the index might have subtle
> performance implications, especially for our internal use (the codebase is
> huge). I bet that internally we wouldn't want
malaperle added inline comments.
Comment at: clangd/WorkspaceSymbols.cpp:165
+[](const SymbolInformation &A, const SymbolInformation &B) {
+ return A.name < B.name;
+});
ilya-biryukov wrote:
> We have `FuzzyMatcher`, it can pr
malaperle marked 9 inline comments as done.
malaperle added a comment.
In https://reviews.llvm.org/D44882#1048355, @sammccall wrote:
> Can we split this patch up (it's pretty large anyway) and land the
> `workspaceSymbols` implementation first, without changing the indexer
> behavior?
>
> I thi
malaperle updated this revision to Diff 139968.
malaperle marked 4 inline comments as done.
malaperle added a comment.
Split the patch so that this part only has the workspace/symbol part
and the index model will be updated separately.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.o
malaperle created this revision.
Herald added subscribers: cfe-commits, MaskRay, ioeric, jkorous-apple,
ilya-biryukov, klimek.
This adds more symbols to the index:
- member variables and functions
- symbols in main files
- enum constants in scoped enums
This is a WIP meant as a starting point f
malaperle planned changes to this revision.
malaperle added inline comments.
Comment at: clangd/index/Index.h:142
+ // Whether or not the symbol should be considered for completion.
+ bool ForCompletion = false;
/// A brief description of the symbol that can be displayed in
malaperle added a comment.
The index changes are moved here: https://reviews.llvm.org/D44954 I haven't
changed the patch yet though, I just removed it from this one.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44882
___
cfe-comm
malaperle added inline comments.
Comment at: clangd/SourceCode.cpp:110
+
+llvm::Optional offsetRangeToLocation(SourceManager &SourceMgr,
+ StringRef File,
MaskRay wrote:
> May I ask a question about the conversion bet
malaperle added a comment.
In https://reviews.llvm.org/D44764#1046766, @sammccall wrote:
> I understand the template instantiations here are a mess and we need to solve
> the problem. However I'm not sure this is the right direction:
>
> - it violates IWYU, and we expect people consistently to g
malaperle updated this revision to Diff 140180.
malaperle added a comment.
Use operator<< where we can
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44764
Files:
clangd/Protocol.cpp
clangd/Protocol.h
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/JSONExprTests
malaperle added inline comments.
Comment at: unittests/clangd/JSONExprTests.cpp:19
-void PrintTo(const Expr &E, std::ostream *OS) {
- llvm::raw_os_ostream(*OS) << llvm::formatv("{0:2}", E);
-}
This one I couldn't change to operator<< because there was already on
malaperle created this revision.
Herald added subscribers: cfe-commits, MaskRay, ioeric, ilya-biryukov, klimek.
Signed-off-by: Marc-Andre Laperle
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D45044
Files:
docs/clangd.rst
Index: docs/clangd.rst
This revision was automatically updated to reflect the committed changes.
Closed by commit rL328792: [clangd] Mark "Source Hover" as
implemented in the docs (authored by malaperle, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D45044
Fil
malaperle updated this revision to Diff 140272.
malaperle added a comment.
Use the DraftStore for offset -> line/col when there is a draft available.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44882
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
clangd/Clangd
malaperle added a comment.
I realized that using the draft store has limited improvement for now because
not many symbol locations come from main files (.cpp, etc) and when headers are
modified, the main files are not reindexed right now. So the draft store will
give better location for example
malaperle added inline comments.
Comment at: clangd/FindSymbols.cpp:31
+
+ if (supportedSymbolKinds &&
+ std::find(supportedSymbolKinds->begin(), supportedSymbolKinds->end(),
MaskRay wrote:
> This std::find loop adds some overhead to each candidate... In my
malaperle updated this revision to Diff 140382.
malaperle marked an inline comment as done.
malaperle added a comment.
Use raw_ostream instead.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44764
Files:
clangd/Protocol.cpp
clangd/Protocol.h
unittests/clangd/CodeCompleteT
malaperle added inline comments.
Comment at: clangd/Protocol.h:673
json::Expr toJSON(const CompletionItem &);
+std::ostream &operator<<(std::ostream &, const CompletionItem &);
sammccall wrote:
> I think raw_ostream should work fine here, it's what we've done
malaperle added a comment.
Herald added a subscriber: MaskRay.
Gentle ping! It would be nice to have this so make Clangd less "verbose".
Comment at: clangd/ClangdLSPServer.cpp:412
+llvm::raw_string_ostream OS(Message);
+OS << "method not found (" << Method << ")";
+
malaperle marked an inline comment as done.
malaperle added inline comments.
Comment at: clangd/FindSymbols.cpp:31
+
+ if (supportedSymbolKinds &&
+ std::find(supportedSymbolKinds->begin(), supportedSymbolKinds->end(),
MaskRay wrote:
> MaskRay wrote:
> > ma
malaperle added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:99
Params.capabilities.textDocument.completion.completionItem.snippetSupport;
+ if (Params.capabilities.workspace && Params.capabilities.workspace->symbol &&
+ Params.capabilities.workspace->sym
malaperle updated this revision to Diff 141028.
malaperle marked 27 inline comments as done.
malaperle added a comment.
Address comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44882
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
malaperle added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:101
+ // All clients should support those.
+ for (SymKindUnderlyingType I = SymbolKindMin;
+ I <= static_cast(SymbolKind::Array); ++I)
I'd like to add some test to exercise the change
malaperle created this revision.
Herald added subscribers: cfe-commits, MaskRay, ioeric, jkorous-apple,
ilya-biryukov, klimek.
malaperle retitled this revision from "[clangd-vscode] Vscode dependencies" to
"[clangd-vscode] Update VScode dependencies".
This allows the extension to work with LSP 3
malaperle added a comment.
Do we need to bump the version of the extension and do a new release or
anything like that? Or leave this for later?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D45285
___
cfe-commits mailing list
cfe-c
malaperle added a comment.
@sammccall Are you OK with the latest version? Thanks!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44764
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/lis
malaperle added a comment.
In https://reviews.llvm.org/D45285#1061374, @hokein wrote:
> In https://reviews.llvm.org/D45285#1061243, @ilya-biryukov wrote:
>
> > In https://reviews.llvm.org/D45285#1058567, @malaperle wrote:
> >
> > > Do we need to bump the version of the extension and do a new rele
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE329574: [clangd-vscode] Update VScode dependencies
(authored by malaperle, committed by ).
Changed prior to commit:
h
malaperle added a comment.
Thanks!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44764
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL329725: [clangd] Use operator<< to prevent printers
issues in Gtest (authored by malaperle, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D44764
malaperle added a comment.
Can you revert the formatting changes in places you haven’t touched (mainly
astunit.cpp)? I think you should be able to do that with git checkout -p HEAD~1
https://reviews.llvm.org/D39375
___
cfe-commits mailing list
cfe-
malaperle requested changes to this revision.
malaperle added inline comments.
This revision now requires changes to proceed.
Comment at: clangd/ClangdLSPServer.cpp:51
+ "definitionProvider": true,
+ "configurationChangeProvider": true
}})");
--
malaperle abandoned this revision.
malaperle added a comment.
Already committed.
Repository:
rL LLVM
https://reviews.llvm.org/D39735
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit
This revision was automatically updated to reflect the committed changes.
Closed by commit rL317585: [clangd] Fix opening declarations located in
non-preamble inclusion (authored by malaperle).
Repository:
rL LLVM
https://reviews.llvm.org/D39705
Files:
clang-tools-extra/trunk/clangd/ClangdU
malaperle requested changes to this revision.
malaperle added inline comments.
This revision now requires changes to proceed.
Comment at: clangd/ClangdLSPServer.cpp:205
+
+ if (!(H.contents[0].codeBlockLanguage == "" &&
+H.contents[0].markdownString == "" &&
malaperle added inline comments.
Comment at: lib/Index/IndexUnitWriter.cpp:212
+ return true;
+};
+
extra semi-colon (noticed this warning while compiling)
https://reviews.llvm.org/D39050
___
cfe-commits mailing l
malaperle added inline comments.
Comment at: lib/Index/IndexingAction.cpp:562
+
+ SourceManager &SM = CI.getSourceManager();
+ DiagnosticsEngine &Diag = CI.getDiagnostics();
As a first attempt, I tried to use index::createIndexDataRecordingAction in
combinatio
malaperle added inline comments.
Comment at: lib/Index/IndexRecordWriter.cpp:155
+if (!IsNew) {
+ llvm::errs() << "Index: Duplicate USR! " << SymInfo.USR << "\n";
+ // FIXME: print more information so it's easier to find the declaration.
I'm getting
malaperle added inline comments.
Comment at: include/indexstore/IndexStoreCXX.h:75
+ bool foreachRelation(llvm::function_ref receiver)
{
+#if INDEXSTORE_HAS_BLOCKS
+return indexstore_occurrence_relations_apply(obj,
^bool(indexstore_symbol_relation_t sym_rel) {
malaperle added a comment.
I'm not sure it's better to use the InclusionDirective callback for this. We
need to get the includes in two places: in the preamble and non-preamble. In
the preamble we use the callback, have to store some temporary stuff because we
don't have a SourceManager in Incl
malaperle added a comment.
In https://reviews.llvm.org/D38639#920487, @ilya-biryukov wrote:
> I think we should never iterate through `SourceManager`, as it's much easier
> to get wrong than using the callbacks. I agree that all that fiddling with
> callbacks is unfortunate, but it's well worth
malaperle added inline comments.
Comment at: clangd/Protocol.h:295
+
+struct ClangdConfigurationParams {
+
ilya-biryukov wrote:
> Maybe call it `ClangdConfigurationParamsChange` to make it clear those are
> diffs, not the actual params?
The idea was that we can
malaperle added inline comments.
Comment at: clangd/Protocol.h:295
+
+struct ClangdConfigurationParams {
+
ilya-biryukov wrote:
> malaperle wrote:
> > ilya-biryukov wrote:
> > > Maybe call it `ClangdConfigurationParamsChange` to make it clear those
> > > are dif
malaperle added a comment.
Hi! I got a bit further in my experiment in integrating this in Clangd. I put
some comments (in the first more complete revision). But since the scope of
this patch changed, if you feel like we should take the discussions elsewhere,
please let me know! Thanks!
malaperle added a comment.
Hi! You put "[clangd]" in the title, but I don't believe it's related to clangd
but rather just "clang"?
https://reviews.llvm.org/D39834
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bi
malaperle added a comment.
Hi William, can you quickly go over the diff before I do a review? There are a
few more-obvious things that can be addressed, i.e. commented lines, extra
new/deleted lines, temporary code, lower case variable names, etc.
https://reviews.llvm.org/D38425
___
malaperle-ericsson added a comment.
In https://reviews.llvm.org/D30675#700917, @bkramer wrote:
> lg, do you have commit access?
No I do not have commit access. Could you commit it? Thank you very much!
https://reviews.llvm.org/D30675
___
cfe-comm
malaperle-ericsson added a comment.
Just to make things clear, I'm really not sure if this is the right approach
and I'm looking for opinions before going further in that direction (code
completion).
Repository:
rL LLVM
https://reviews.llvm.org/D31019
malaperle-ericsson added a subscriber: bkramer.
malaperle-ericsson added a comment.
In https://reviews.llvm.org/D31019#702631, @bkramer wrote:
> It's not. the goal is to get rid of ASTUnit inside of clangd in the long term
> as it's a big problem for extensibility. libclang is just a wrapper for
malaperle-ericsson added a comment.
Perfect. Thanks a lot for the explanation!
Repository:
rL LLVM
https://reviews.llvm.org/D31019
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
malaperle-ericsson abandoned this revision.
malaperle-ericsson added a comment.
Abandoned because of wrong approach.
Repository:
rL LLVM
https://reviews.llvm.org/D31019
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org
malaperle-ericsson added a comment.
Urg, I was working on this too :) I'll compare implementations and provide
comments if I find anything good to suggest.
https://reviews.llvm.org/D31328
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http
malaperle-ericsson added inline comments.
Comment at: test/clangd/formatting.test:14
+# CHECK: "codeActionProvider": true,
+# CHECK: "completionProvider": {"resolveProvider": false,
"triggerCharacters": [".",">"]}
# CHECK: }}}
It would be good eventually to
malaperle-ericsson added a comment.
Ideas/Observations:
- One thing I has done in my version is to introduce "ASTUnitRunnable", a
lambda function type that has an ASTUnit as parameter and is executed by the
ASTManager. So the ASTManager takes care of locking the AST but the actual code
using t
malaperle-ericsson updated this revision to Diff 94815.
malaperle-ericsson marked 6 inline comments as done.
malaperle-ericsson added a comment.
Fixed typos and other comments.
https://reviews.llvm.org/D31887
Files:
docs/clangd.rst
docs/index.rst
Index: docs/index.rst
=
201 - 300 of 329 matches
Mail list logo
