malaperle (Marc-Andre Laperle)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 27 2016, 9:06 PM (99 w, 1 d)

Recent Activity

Wed, Jun 13

malaperle created D48159: [clangd] Implement hover for "auto" and "decltype".
Wed, Jun 13, 8:33 PM
malaperle added a comment to D48071: [clangd] Add an option controlling caching of compile commands..

Does this change affect the switching of compilation database, through workspace/didChangeConfiguration ?

Wed, Jun 13, 7:09 AM

Mon, Jun 11

malaperle added a comment to D47950: [clangd] FuzzyMatch: forbid tail-tail matches after a miss: [pat] !~ "panther".

Very nice! I tried "std" and got much less (unimportant) results. I see something a bit weird with "getStandardResourceDir" but it might be VSCode. Here, I guess it's the "d" in Dir that matches but what's odd is that VS Code will highlight the first "d", i.e. in "standard". But it doesn't look like there is any way to control this in the LSP, is there?

That's right. VSCode applies its fuzzymatch again client-side to determine which characters to highlight, and it chooses the wrong (IMO) "d" here. We wouldn't match [std] against "getStandardResourceZir".
(Worse, they use the same algorithm to rerank our results if client-side filtering applies...)

Mon, Jun 11, 10:19 AM
malaperle added a comment to D47950: [clangd] FuzzyMatch: forbid tail-tail matches after a miss: [pat] !~ "panther".

Very nice! I tried "std" and got much less (unimportant) results. I see something a bit weird with "getStandardResourceDir" but it might be VSCode. Here, I guess it's the "d" in Dir that matches but what's odd is that VS Code will highlight the first "d", i.e. in "standard". But it doesn't look like there is any way to control this in the LSP, is there?

Mon, Jun 11, 8:58 AM

Wed, Jun 6

malaperle added a reviewer for D47847: [clangd] Simplify matches in FindSymbols tests: simark.
Wed, Jun 6, 2:42 PM
malaperle added inline comments to D47846: [clangd] Implementation of textDocument/documentSymbol.
Wed, Jun 6, 2:42 PM
malaperle created D47847: [clangd] Simplify matches in FindSymbols tests.
Wed, Jun 6, 2:36 PM
malaperle added a comment to D47821: [clangd] Make workspace/symbols actually rank its results..

This works much better! Just a nit.

Wed, Jun 6, 2:17 PM
malaperle created D47846: [clangd] Implementation of textDocument/documentSymbol.
Wed, Jun 6, 2:11 PM

Tue, Jun 5

malaperle committed rL334018: [clangd] Remove unused variables.
[clangd] Remove unused variables
Tue, Jun 5, 7:12 AM
malaperle committed rCTE334018: [clangd] Remove unused variables.
[clangd] Remove unused variables
Tue, Jun 5, 7:12 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Tue, Jun 5, 7:12 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Tue, Jun 5, 7:12 AM
malaperle committed rL334017: [clangd] Add "member" symbols to the index.
[clangd] Add "member" symbols to the index
Tue, Jun 5, 7:06 AM
malaperle committed rCTE334017: [clangd] Add "member" symbols to the index.
[clangd] Add "member" symbols to the index
Tue, Jun 5, 7:06 AM
malaperle closed D44954: [clangd] Add "member" symbols to the index.
Tue, Jun 5, 7:06 AM
malaperle closed D44954: [clangd] Add "member" symbols to the index.
Tue, Jun 5, 7:06 AM

Mon, Jun 4

malaperle added a comment to D47643: Rewrite JSON dispatcher loop using C IO (FILE*) instead of std::istream..

PS I've checked it on my Mac and lldb seems to attach just fine.

Mon, Jun 4, 9:17 PM
malaperle created D47737: [clangd] Remove unused variables.
Mon, Jun 4, 12:33 PM

Sun, Jun 3

malaperle added a comment to D47643: Rewrite JSON dispatcher loop using C IO (FILE*) instead of std::istream..

@malaperle: would you mind patching this in and checking whether attaching a debugger still works on Mac (this was the reason for the EINTR loop, right?)

I want to preserve this but now people other than me are complaining about old clangds hanging around and eating all their CPU :-)

Sun, Jun 3, 9:55 PM

Fri, Jun 1

malaperle added inline comments to D44954: [clangd] Add "member" symbols to the index.
Fri, Jun 1, 12:33 PM
malaperle updated the diff for D44954: [clangd] Add "member" symbols to the index.

Address comments.

Fri, Jun 1, 12:33 PM

Wed, May 30

malaperle updated the diff for D44954: [clangd] Add "member" symbols to the index.

Update with suggestions.

Wed, May 30, 2:37 PM
malaperle added inline comments to D44954: [clangd] Add "member" symbols to the index.
Wed, May 30, 2:35 PM

Mon, May 28

malaperle added inline comments to D44954: [clangd] Add "member" symbols to the index.
Mon, May 28, 11:58 AM
malaperle added inline comments to D44954: [clangd] Add "member" symbols to the index.
Mon, May 28, 11:54 AM
malaperle updated the diff for D44954: [clangd] Add "member" symbols to the index.

Address comments.

Mon, May 28, 11:54 AM

Thu, May 24

malaperle updated subscribers of D47187: [clangd] Skip .inc headers when canonicalizing header #include..

I think this might have broken the "shared lib" build? Could you double check that you don't need to add "clangDriver" ?

Thu, May 24, 8:56 AM

Wed, May 23

malaperle added a comment to D47223: [clangd] Handle enumerators in named, unscoped enums similarly to scoped enums.

I'm not sure if we have tests for that, but I remember that we kept the enumerators in the outer scope so that completion could find them..
Am I right that this patch will change the behavior and we won't get enumerators in the following example:

/// foo.h
enum Foo {
  A, B, C
};

/// foo.cpp
#include "foo.h"

int a = ^ // <-- A, B, C should be in completion list here.
Wed, May 23, 7:33 PM
malaperle updated the diff for D44954: [clangd] Add "member" symbols to the index.

Use "SupportGlobalCompletion", white-list decl contexts, add more tests

Wed, May 23, 7:23 PM
malaperle added a comment to D47272: [clangd] Build index on preamble changes instead of the AST changes.

We do not to rely on symbols from the main file anyway, since any info hat those provide can always be taken from the AST.

I'll be adding those soon for workspace symbols... And also for document symbols.

I can add extra code to build pieces for the AST later. This is not hard to do, but would require rearranging some code a bit more.
Will try to send the change tomorrow for review tomorrow. Does that SG?

Wed, May 23, 11:38 AM
malaperle added a comment to D47272: [clangd] Build index on preamble changes instead of the AST changes.

We do not to rely on symbols from the main file anyway, since any info hat those provide can always be taken from the AST.

Wed, May 23, 11:28 AM

May 22 2018

malaperle added a comment to D44954: [clangd] Add "member" symbols to the index.

What scopes will non-scoped enum members have?

Hmm. I think all of them, since you can refer them like that in code too. Case #1 doesn't work but that was the case before this patch so it can probably be addressed separately. I'll add some tests though!

I would vote for making queries En::A and A match the enumerator, but not ::A. The reasoning is: yes, you can reference it this way in a C++ file, but workspaceSymbol is not a real C++ resolve and I think it should match the outline of the code rather than the actual C++ lookup rules.
E.g. I wouldn't expect it to match symbols from base classes, etc. This should also simplify implementation too.

I don't have a strong opinion, so I can try this suggestion!

May 22 2018, 1:42 PM
malaperle added a reviewer for D47223: [clangd] Handle enumerators in named, unscoped enums similarly to scoped enums: ilya-biryukov.
May 22 2018, 1:42 PM
malaperle created D47223: [clangd] Handle enumerators in named, unscoped enums similarly to scoped enums.
May 22 2018, 1:36 PM

May 18 2018

malaperle added a comment to D44954: [clangd] Add "member" symbols to the index.

It's probably better to consider this in a future patch. Maybe something like the first suggestion: vector::push_back and match both. Otherwise, I would think it might be a bit too verbose to have to spell out all of the specialization. Maybe we could allow it too. So... all of the above? :)

This certainly does not have to be addressed in this patch. Just wanted to collect opinions on what the behavior we want in the long term.
My thoughts would be towards allowing only vector::push_back and make it match both push_backs: in vector<bool> and vector<T>.
Other cases might work too, but I wouldn't try implementing something that matches specializations. It's just too complicated in the general case. This will only be used in workspaceSymbol and it's fine to give a few more results there...

May 18 2018, 7:57 AM
malaperle updated subscribers of D47063: [clangd] Keep only a limited number of idle ASTs in memory.

Maybe we can even store enough information to not need the AST for navigation at all and build it only for features like refactorings.
@sammccall, let me know what are your thoughts on all of this.

May 18 2018, 7:56 AM

May 17 2018

malaperle added a comment to D44954: [clangd] Add "member" symbols to the index.

A few questions regarding class members. To pinpoint some interesting cases and agree on how we want those to behave in the long run.

How do we handle template specializations? What will the qualified names of those instantiations be?
I.e. how do I query for push_back inside a vector? Which of the following queries should produce a result?

  • vector::push_back. Should it match both vector<T>::push_back and vector<bool>::push_back or only the first one?
  • vector<bool>::push_back
  • vector<int>::push_back
May 17 2018, 1:42 PM

May 16 2018

malaperle added a comment to D44954: [clangd] Add "member" symbols to the index.

@ioeric You mentioned in D46751 that it would make sense to add a flag to disable indexing members. Could you comment on that? What kind of granularity were you thinking? Would a "member" flag cover both class members (member vars and functions) and enum class enumerators for example? I think that would be reasonable. But I will also add symbols in main files too, so another flag for that? Hmmm.

May 16 2018, 1:07 PM
malaperle added a reviewer for D44954: [clangd] Add "member" symbols to the index: ioeric.
May 16 2018, 1:04 PM

May 15 2018

malaperle added a comment to D46751: [clangd] Filter out private proto symbols in SymbolCollector..

@malaperle to expand on what Eric said, adding proto hacks with false positives and no way to turn them off is indeed not the way to go here!
There's probably going to be other places we want to filter symbols too, and it should probably be extensible/customizable in some way.
We don't yet have enough examples to know what the structure should be (something regex based, a code-plugin system based on Registry, or something in between), thus the simplest/least invasive option for now (it's important for our internal rollout to have *some* mitigation in place).
@ioeric can you add a comment near the proto-filtering stuff indicating we should work out how to make this extensible?

I agree with all of that. What I don't quite understand is why a flag is not ok? Just a fail-safe switch in the mean time? You can even leave it on by default so your internal service is not affected.

I think a flag doesn't solve much of the problem, and adds new ones:

  • users have to find the flag, and work out how to turn it on in their editor, and (other than embedders) few will bother. And each flag hurts the usability of all the other flags.
  • if this heuristic is usable only sometimes, that's at codebase granularity, not user granularity. Flags don't work that way. (Static index currently has this problem...)
  • these flags end up in config files, so if we later remove the flag we'll *completely* break clangd for such users
May 15 2018, 10:14 AM
malaperle added a comment to D46751: [clangd] Filter out private proto symbols in SymbolCollector..

@malaperle to expand on what Eric said, adding proto hacks with false positives and no way to turn them off is indeed not the way to go here!
There's probably going to be other places we want to filter symbols too, and it should probably be extensible/customizable in some way.
We don't yet have enough examples to know what the structure should be (something regex based, a code-plugin system based on Registry, or something in between), thus the simplest/least invasive option for now (it's important for our internal rollout to have *some* mitigation in place).
@ioeric can you add a comment near the proto-filtering stuff indicating we should work out how to make this extensible?

May 15 2018, 8:32 AM
malaperle added a comment to D46751: [clangd] Filter out private proto symbols in SymbolCollector..

I think this is good if that's true that the comment is always there. I think it would be OK for this to be enabled by default, with a general option to turn heuristics off. Not sure what to call it... -use-symbol-filtering-heuristics :)

We have other filters in the symbol collector that we think could improve user experience, and we don't provide options for those.

May 15 2018, 7:41 AM

May 11 2018

malaperle added inline comments to D44954: [clangd] Add "member" symbols to the index.
May 11 2018, 12:44 PM
malaperle retitled D44954: [clangd] Add "member" symbols to the index from [clangd] [RFC] Add more symbols to the index to [clangd] Add "member" symbols to the index.
May 11 2018, 12:41 PM
malaperle updated the diff for D44954: [clangd] Add "member" symbols to the index.

Rework to include member vars/functions and scoped enumerators.

May 11 2018, 12:40 PM
malaperle added a comment to D46751: [clangd] Filter out private proto symbols in SymbolCollector..

Can there be an option for this? This seems very library specific and could break other code bases. Ideally, there would be a generic mechanism for this kind of filtering, i.e. specify a pattern of excluded files or symbol names. But I understand this would be cumbersome because you want to filter only *some* symbol names in *some* files, so it would be difficult for users to specify this intersection of conditions on command-line arguments, for example. I think this needs to be discussed a bit more or have this turned off by default (with an option to turn on!) until there is a more general solution for this kind of filtering.

Having user-configurable filtering may certainly be useful, but requires some design to get right.
And even if we have it, I think there's value in automatically handling popular frameworks, unless we know it might break other code in practice.

May 11 2018, 7:40 AM
malaperle added a comment to D46751: [clangd] Filter out private proto symbols in SymbolCollector..

Can there be an option for this? This seems very library specific and could break other code bases. Ideally, there would be a generic mechanism for this kind of filtering, i.e. specify a pattern of excluded files or symbol names. But I understand this would be cumbersome because you want to filter only *some* symbol names in *some* files, so it would be difficult for users to specify this intersection of conditions on command-line arguments, for example. I think this needs to be discussed a bit more or have this turned off by default (with an option to turn on!) until there is a more general solution for this kind of filtering.

May 11 2018, 6:55 AM

Apr 24 2018

malaperle added a comment to D44882: [clangd] Implementation of workspace/symbol request.

It makes sense, but @bkramer came up with some deep magic in rL330754 so I think we're actually good now.

Apr 24 2018, 6:50 PM
malaperle added a comment to D44882: [clangd] Implementation of workspace/symbol request.

So this fails if there's no standard library available without flags, which is the case in google's test environment to ensure hermeticity :-(

In the short-term, we've disabled the test internally - did it trigger any buildbot failures?
In the medium term we should probably find a better way to test this :(

Apr 24 2018, 9:16 AM

Apr 23 2018

malaperle committed rCTE330637: [clangd] Implementation of workspace/symbol request.
[clangd] Implementation of workspace/symbol request
Apr 23 2018, 1:04 PM
malaperle committed rL330637: [clangd] Implementation of workspace/symbol request.
[clangd] Implementation of workspace/symbol request
Apr 23 2018, 1:04 PM
malaperle closed D44882: [clangd] Implementation of workspace/symbol request.
Apr 23 2018, 1:04 PM

Apr 19 2018

malaperle updated the diff for D44882: [clangd] Implementation of workspace/symbol request.

Remove more includes.

Apr 19 2018, 7:59 PM
malaperle added a comment to D44882: [clangd] Implementation of workspace/symbol request.

Sorry for the embarrassing clean-ups I forgot!

Apr 19 2018, 7:54 PM
malaperle updated the diff for D44882: [clangd] Implementation of workspace/symbol request.

Address comments.

Apr 19 2018, 7:53 PM

Apr 18 2018

malaperle updated subscribers of D45717: [clangd] Using index for GoToDefinition..
Apr 18 2018, 8:00 PM
Herald updated subscribers of D45717: [clangd] Using index for GoToDefinition..
Apr 18 2018, 8:00 PM

Apr 17 2018

malaperle added a comment to D44882: [clangd] Implementation of workspace/symbol request.

Any objections to the latest version?

Apr 17 2018, 8:41 AM

Apr 13 2018

malaperle added a comment to D44882: [clangd] Implementation of workspace/symbol request.

Thanks, I have landed the patch today, you need to update your patch (I think it is mostly about removing the code of reading-file stuff).

Apr 13 2018, 7:22 AM
malaperle updated the diff for D44882: [clangd] Implementation of workspace/symbol request.

Address comments. Simplify with using line/col from index.

Apr 13 2018, 7:20 AM

Apr 12 2018

malaperle added inline comments to D44882: [clangd] Implementation of workspace/symbol request.
Apr 12 2018, 9:41 PM
malaperle updated the diff for D44882: [clangd] Implementation of workspace/symbol request.

Address comments.

Apr 12 2018, 9:41 PM
malaperle added a comment to D44882: [clangd] Implementation of workspace/symbol request.

Still LG, thanks!
I'll look into the testing issue.

I thought about it after... I think it was because I was trying to test with std::unordered_map (to prevent multiple results) which needs std=c++11, I'll try with something else.

Worth a shot, but don't count on it - for c++ clang switched to using std=c++11 by default a while ago.

Apr 12 2018, 8:10 PM
malaperle added a comment to D44882: [clangd] Implementation of workspace/symbol request.

@malaperle, what's your plan of this patch? Are you going to land it before D45513? With the Line&Column info in the index, this patch could be simplified.

Apr 12 2018, 10:33 AM
malaperle added a comment to D44882: [clangd] Implementation of workspace/symbol request.

Still LG, thanks!
I'll look into the testing issue.

Apr 12 2018, 7:46 AM

Apr 11 2018

malaperle added a comment to D44882: [clangd] Implementation of workspace/symbol request.

(BTW @hokein has D45513 to add row/col to the index, which will allow all the file-reading stuff to be cleaned up, but no need to wait on that since it's all working now).

Apr 11 2018, 9:22 PM
malaperle updated the diff for D44882: [clangd] Implementation of workspace/symbol request.

Address comments.

Apr 11 2018, 9:22 PM

Apr 10 2018

malaperle committed rCTE329725: [clangd] Use operator<< to prevent printers issues in Gtest.
[clangd] Use operator<< to prevent printers issues in Gtest
Apr 10 2018, 10:39 AM
malaperle committed rL329725: [clangd] Use operator<< to prevent printers issues in Gtest.
[clangd] Use operator<< to prevent printers issues in Gtest
Apr 10 2018, 10:39 AM
malaperle closed D44764: [clangd] Use operator<< to prevent printers issues in Gtest.
Apr 10 2018, 10:39 AM
malaperle added a comment to D44764: [clangd] Use operator<< to prevent printers issues in Gtest.

Thanks!

Apr 10 2018, 10:39 AM

Apr 9 2018

malaperle committed rL329574: [clangd-vscode] Update VScode dependencies.
[clangd-vscode] Update VScode dependencies
Apr 9 2018, 7:35 AM
malaperle committed rCTE329574: [clangd-vscode] Update VScode dependencies.
[clangd-vscode] Update VScode dependencies
Apr 9 2018, 7:35 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Apr 9 2018, 7:35 AM
malaperle added a comment to D45285: [clangd-vscode] Update VScode dependencies.

Do we need to bump the version of the extension and do a new release or anything like that? Or leave this for later?

We should bump the version and republish the extension into VSCode marketplace.
@hokein has more context on how to properly do that.

Once you commit this patch, I'm happy to make a new release.

Apr 9 2018, 7:28 AM

Apr 5 2018

malaperle added a comment to D44764: [clangd] Use operator<< to prevent printers issues in Gtest.

@sammccall Are you OK with the latest version? Thanks!

Apr 5 2018, 10:10 AM
malaperle added a comment to D45285: [clangd-vscode] Update VScode dependencies.

Do we need to bump the version of the extension and do a new release or anything like that? Or leave this for later?

Apr 5 2018, 8:46 AM

Apr 4 2018

malaperle added inline comments to D44882: [clangd] Implementation of workspace/symbol request.
Apr 4 2018, 1:36 PM
malaperle created D45285: [clangd-vscode] Update VScode dependencies.
Apr 4 2018, 1:34 PM
malaperle retitled D45285: [clangd-vscode] Update VScode dependencies from [clangd-vscode] Vscode dependencies to [clangd-vscode] Update VScode dependencies.
Apr 4 2018, 1:34 PM
malaperle added inline comments to D44882: [clangd] Implementation of workspace/symbol request.
Apr 4 2018, 12:57 PM
malaperle updated the diff for D44882: [clangd] Implementation of workspace/symbol request.

Address comments.

Apr 4 2018, 12:57 PM

Apr 1 2018

malaperle added inline comments to D44882: [clangd] Implementation of workspace/symbol request.
Apr 1 2018, 8:29 PM
malaperle added a comment to D44226: [clangd] Add -log-lsp-to-stderr option.

Gentle ping! It would be nice to have this so make Clangd less "verbose".

Apr 1 2018, 7:47 PM

Mar 29 2018

malaperle added inline comments to D44764: [clangd] Use operator<< to prevent printers issues in Gtest.
Mar 29 2018, 8:49 PM
malaperle updated the diff for D44764: [clangd] Use operator<< to prevent printers issues in Gtest.

Use raw_ostream instead.

Mar 29 2018, 8:48 PM
malaperle added inline comments to D44882: [clangd] Implementation of workspace/symbol request.
Mar 29 2018, 11:00 AM
malaperle added a comment to D44882: [clangd] Implementation of workspace/symbol request.

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 when navigating to a function definition which moved in the unsaved main file. But it will be more general once header updates are handled better.

Mar 29 2018, 9:19 AM
malaperle updated the diff for D44882: [clangd] Implementation of workspace/symbol request.

Use the DraftStore for offset -> line/col when there is a draft available.

Mar 29 2018, 9:18 AM
malaperle committed rCTE328792: [clangd] Mark "Source Hover" as implemented in the docs.
[clangd] Mark "Source Hover" as implemented in the docs
Mar 29 2018, 7:56 AM
malaperle committed rL328792: [clangd] Mark "Source Hover" as implemented in the docs.
[clangd] Mark "Source Hover" as implemented in the docs
Mar 29 2018, 7:53 AM
malaperle closed D45044: [clangd] Mark "Source Hover" as implemented in the docs.
Mar 29 2018, 7:53 AM
malaperle added a reviewer for D45044: [clangd] Mark "Source Hover" as implemented in the docs: simark.
Mar 29 2018, 7:35 AM
malaperle created D45044: [clangd] Mark "Source Hover" as implemented in the docs.
Mar 29 2018, 7:35 AM

Mar 28 2018

malaperle added inline comments to D44764: [clangd] Use operator<< to prevent printers issues in Gtest.
Mar 28 2018, 8:01 PM
malaperle retitled D44764: [clangd] Use operator<< to prevent printers issues in Gtest from [clangd] Use a header for common inclusions for tests to [clangd] Use operator<< to prevent printers issues in Gtest.
Mar 28 2018, 8:01 PM
malaperle updated the diff for D44764: [clangd] Use operator<< to prevent printers issues in Gtest.

Use operator<< where we can

Mar 28 2018, 7:57 PM
malaperle added a comment to D44764: [clangd] Use operator<< to prevent printers issues in Gtest.

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 get gmock transitively via this header. This goes against common practice, is inconsistent with the rest of LLVM, and confuses tools (including clangd itself!). It would need to be carefully documented, but I don't think this is enough.
Mar 28 2018, 7:45 PM
malaperle added inline comments to D44882: [clangd] Implementation of workspace/symbol request.
Mar 28 2018, 2:00 PM