- User Since
- Aug 26 2016, 6:53 AM (216 w, 6 d)
Add comment+assert to clarify
Fix nullptr condition (probably dead, but...)
Sounds good (though I ran into issues calling this hasName specifically, see below). There's overlap with hasAttr(Kind) but I think that only handles decls, you can't bind the attribute itself, and it's harder to extend to the other stuff you mentioned.
So maybe hasAttr gets deprecated, or maybe has(attr(hasName("foo"))) is a mouthful for a common case so the shorthand is nice.
Address review comments.
Add hasAttrName() and make isImplicit() support Attr too.
Only deoptimize selection for *explicit* attributes
Cool, this makes sense to me. I hadn't realized this doesn't go through typoexpr, that's a bit unfortunate we can't address this just in one place.
What's the goal here?
Tue, Oct 20
Thanks for the comments - haven't addressed them yet, but wanted to clarify scope first.
Yep, let's revert to the previous state and land that, and I'll puzzle over the examples you give (because always returning false shouldn't affect behavior, just performance).
Update docs, dynamic registry and tests.
Mon, Oct 19
Based on a discussion with @rsmith - the formal definition of pack expansion is that it expands to nested parenthesized expressions expressions.
375f7a416031b3a011a5a88ba5f80f5879d775ca should fix this.
Sorry about the delay.
This is a supported configuration: apple clang toolchain (including libc++) is supposed to work back to 6.0. I'll get this fixed today.
(for real this time)
Oops, forgot newly added files :-\
Fri, Oct 16
This looks really nice!
Wed, Oct 14
(sorry out today and haven't looked at code yet)
Mon, Oct 12
This is magnificently difficult :-)
I've sunk a fair few hours into understanding it now, and need to send some comments based on the interface+tests.
Some of these will be misguided or answered by the implementation, but I'm afraid I can't fit it all into my head (on a reasonable timeframe) until I understand it better.
This is fine as is, but could consider retiring the flags instead.
Sorry, I thought this was accepted. Had addressed the two comments, happy to address more or revert, LMK
Thanks! This seems totally complete now, though I bet there's another case possible somehow!
Fri, Oct 9
Taking another look at the header list, there are a couple of classes of symbols beyond c/c++ standard library.
Another slighty silly reason: because of layering, a code action is going to have a hard time getting at ClangdLSPServer's profile (or even ClangdServer's).
Whereas a custom method will be implemented at that layer.
Yeah, we can fairly easily make this tighter if it's noisy. My suspicion is people will rather complain about ranking than these results being totally irrelevant.
Nice that we now have workspace/symbols users that can send this feedback!
Is this https://github.com/clangd/clangd/issues/554 ? :-)
Nice catch, LG with readability nits
Wed, Oct 7
Refactor the hack into a more appropriate place.
this is at least going to take some important locks, hopefully only briefly.
Now there's lots of usage (which looks good!) i'm finding it a bit hard to keep track of what the tree will look like overall.
Still LGTM, commit it already :-D