- User Since
- Feb 4 2018, 8:41 PM (251 w, 2 d)
Fri, Nov 25
(I envisioned the implementation of augmentsSyntaxTokens=false to be "loop over the lexer tokens and turn a subset of them into additional tokens to return over LSP". The fact that that would have meant producing an "operator" token even for tok::star which is actually a declarator, is what clued me in to the fact that "operators" are a semantic category.)
Wed, Nov 23
Mon, Nov 21
FWIW, in codebases/components where I do code reviews, I tend to insist on template parameters having meaningful (and longer-than-one-character) names, and would find this quite useful.
Sun, Nov 20
A couple of high-level thoughts on this:
Sun, Nov 13
nit: isSelfContained/isSelfContainedHeader in commit message
Sat, Nov 12
Fri, Nov 11
Tue, Nov 8
Great minds :)
Mon, Nov 7
Sat, Nov 5
Haven't had a chance to look at this yet. I do see that the earier implementation in D91930 was the subject of some design discussions about performance with @kadircet, adding him as an additional reviewer.
Fri, Nov 4
Follow-up to D133757
Thanks for the patch!
Wed, Nov 2
Looks good, thanks
The test added in the previous patch, CompletionTest.Enums, needs to be updated to reflect this change (Scoped::Clangd3 now appears as a completion).
I don't think we need this much detail in the commit message; I think after the first line it would be sufficient to say:
Tue, Nov 1
Improve handling of config file diagnostics in lit test
Oct 31 2022
I believe this addresses the remaining review comments. I will follow up with a patch to rename QueryDriverDatabase.cpp to SystemIncludeExtractor.cpp.
reformulate test as lit test
address other review comments
format comment better
Oct 30 2022
(I'm going to re-request review since I have an outstanding question about the fix approach.)
Oct 29 2022
For reference, earlier work along these lines which never landed: https://reviews.llvm.org/D119077
(I frequently forget that arc diff does not update the commit message of an existing revision...)
Oct 24 2022
(apologies, I'm also very tardy in getting back to this...)
Address review comments
Oct 18 2022
Thanks for fixing!
Oct 17 2022
Landing the fix now
Looks like an error introduced when rebasing the patch: a recently added test case (for https://github.com/clangd/clangd/issues/1222) needs to be updated to reflect the _decl to _def change that this patch makes to all semantic highlighting tests. I'll land a fix.
I went ahead and landed this for you
Sep 30 2022
Thanks for the testcases!
Sep 28 2022
Speculative fix for an infinite recursion with no testcase. I'm happy to circle back to this if/when we get a testcase, but I don't think there's any downside to this patch in the meantime.
Sep 25 2022
This is mostly just a rebase on top of the update to D133756 to turn
CompileCommandsAdjuster into a unique_function, and addressing a few
Updated to use unique_function rather than inheritance
Sep 20 2022
Sep 19 2022
Sep 17 2022
Thanks for fixing!
Sep 15 2022
Thanks for the feedback! Yeah I'd be wary of introducing a corner case where the user passes --query-driver, and there is in fact a driver available to query in PATH, but we end up not querying it. Even if the outcome is the same in cases we can think of, it feels like a footgun that's going to bite some user somewhere.
I think the sticking point for just having QueryDriverDatabase run after the entirety of CommandMangler is this check in resolveDriver(), which replaces e.g. gcc with /path/to/clang/bin/gcc, which likely does not actually exist (i.e. QueryDriverDatabase will not be able to query it, when it might have been able to look up gcc in PATH).
Sep 14 2022
Sep 13 2022
So far I've only written a test for https://github.com/clangd/clangd/issues/1089.
Depends on https://reviews.llvm.org/D133756
This is a refactoring in preparation for D133757, please see that for motivation
Sep 11 2022
Sep 5 2022
Move test to StmtPrinterTests, add test for literal operator using pack
Aug 29 2022
Aug 13 2022
Related issue: https://github.com/clangd/clangd/issues/826
Aug 4 2022
Sorry, I'm travelling and probably won't be able to respond promptly, but my first thought here is that this doesn't seem as well motivated as usedAsMutableReference, since taking a pointer to an object involves using explicit syntax (&obj) that taking a reference does not. My mental model of usedAsMutableReference is that it's a workaround for the fact that C++ does not have explicit syntax for forming a mutable reference (compare Rust's ref obj which in my mind makes this modifier likewise unnecessary there).