User Details
- User Since
- Apr 6 2017, 1:42 AM (272 w, 6 d)
- Roles
- Administrator
Mon, Jun 27
My 2 cents here. We should probably try hard to keep the cursor inside braces/parentheses for those patterns.
Having
namespace foo { decls }^ <- cursor here
hurts UX. It would be nice to get the old behavior back somehow.
I still have hopes for the VSCode discussion, maybe they can provide a workaround/willing to change their behavior.
@kadircet also suggested trying something like ${2:decls}$0. Could you check if that works in VSCode?
Fri, Jun 24
Thu, Jun 23
Wed, Jun 22
@erichkeane could you take another look at this?
- Update test, check error messages
Wed, Jun 15
LGTM. Please take a look the NIT about landing this as two commits.
- Fix typos and apply suggestions in documentation and comments
- Change test from .fail.cpp to .verify.cpp
Fri, Jun 10
- Do not introduce empty branches on asserts
- Test whether specialization or primary template gets picked
Fixed issue is: 54629
@saar.raz @erichkeane please let me know if you are the right people to review this.
The change might need a little more refactoring, e.g. placement new should probably move to a helper in Sema to align with what clang usually does.
Thu, Jun 9
Wed, Jun 8
Tue, Jun 7
LGTM. Thanks for the cleanup!
Mon, Jun 6
Sorry about the delay. Thanks for the cleanup! I only have a few minor comments here.
Fri, Jun 3
Thu, Jun 2
Answering my own question here: I ended up pointing CC and CXX to Clang instead of GCC when running CMake. This makes lit add the -verify flag to tests.
- Fix test failure
I've update change description and the added test. This should fix failures on buildbots and address the comments.
PTAL.
- Update test comment, only check in C++20 and beyond, add expected-errors
May 25 2022
Thanks for the quick response. I have addressed the comments, please take another look.
- Add _LIBCPP_ENABLE_CXX20_REMOVED_ALLOCATOR_VOID_SPECIALIZATION
- Document the new MACRO and add a release note
@ldionne friendly ping. Just want to make sure this does not fall off your radar.
Let me know if you need more time, no rush here from our side.
May 23 2022
- Move test to test/libcxx
- Switch test to 'compile.pass' and remove 'main'
May 20 2022
May 19 2022
Richard, thanks for course correcting. I was under impression that header modules are not in the standard, my mistake.
LGTM
May 18 2022
NIT: typo in the change description
Randam-access
Random
May 17 2022
- Revert gnu++20 fix
- Always pass -fheader-modules/-fno-header-modules when modules are enabled
May 12 2022
May 11 2022
Please allow some time for the clangd team to review this before landing.
Changes to filenames used to cause unintended consequences for us before. We switched to using file UIDs since then, but we're still not sure it's not going to break us.
May 9 2022
LGTM.
LGTM
LGTM.
May 6 2022
Many thanks for improving the running time, this slow test has bugged me since the day I first ran it.
I'd even be happy to remove it completely. It's slow, possibly flaky, so IMO the corresponding check does not carry its weight.
May 5 2022
@erichkeane sure, will do.
May 4 2022
- add FIXME for a memory leak
May 2 2022
Apr 28 2022
@labath, ah, sorry, you beat me to it.
NIT: change the commit message to say -text instead of binary
LGTM
- Mention the AutoType traversal does not traverse the constrained concept decl anymore and add a test for it
Apr 27 2022
Apr 26 2022
- clang-format the code
- Check 'Concept auto' completion,
- set -std=c++20 unconditionally,
- use char instead of int.
Just to be clear, I feel it is not RecursiveASTVisitor that's at fault here. It's rather design of the Index library that splits work into multiple visitors.
Requires expression is produces a template parameter list that suddenly appears out of nowhere in the BodyVisitor, whereas the code to handle it is in the DeclVisitor.
Yeah, that sounds promising. Various source locations inside the same nodes are probably very close. Probably even across nodes.
If we are willing to take on a bit more complexity, we could probably get even bigger wins by storing all source locations in a single array having indices into that array in the actual positions where we need the source locations. That way we can have:
- get the benefits of delta-encoding in the source locations array, probably even across nodes,
- probably still get quite good encoding for indices inside this array inside the actual AST nodes. Will definitely get those if we delta-encode the indices inside the same node.
I suspect there are requirements that will force us to store the array in chunks and allow searching for a particular chunk.
Overall LGTM, but I didn't look closely if there are cases that break now.
Do we have any tests to check this?
Apr 25 2022
LGTM. Nice savings
Jun 8 2020
Added a few minor changes before committing:
- surround names of .clang-format files with double backticks
- ensure no lines are longer than 80 characters