This resolves the NB comment about the construction of a fixed-size span from a dynamic range
Details
- Reviewers
ldionne mclow.lists EricWF CaseyCarter - Group Reviewers
Restricted Project - Commits
- rG6d2599e4f776: [libcxx][span] Implement P1976R2
Diff Detail
- Repository
- rG LLVM Github Monorepo
- Build Status
Buildable 46441 Build 48906: arc lint + arc unit
Event Timeline
Thanks for the review, I am unsure whether one should really update the feature test macro without the other changes in place (tuple interface, relaxed array constructors etc)
If you want to update I wouldnt mind too much though
libcxx/include/span | ||
---|---|---|
67 | Sure will do | |
228 | Because we do not have library concepts and ranges yet. The _Container&& constructor you mention is the range constructor that requires ranges::contiguous_range, ranges::sized_range and ranges::borrowed_range. |
@ldionne do you require any changes here (especially regarding the feature test macro)
I approved the other span changes, so the feature test macro can be updated now, right? If so, please update it and go ahead, otherwise please explain :-).
@ldionne Thanks a lot for the review. I have added the feature test macro as I did not find it anywhere else.
Note that I do not have commit access, so you would need to commit those revisions
You need to modify the feature test macro in libcxx/utils/generate_feature_test_macro_components.py, and then generate the other files (<version>, the documentation and the tests) from it. More details in libcxx/docs/DesignDocs/FeatureTestMacros.rst.
@ldionne Thanks a lot for the hint. I updated the generate_feature_test_macro_components.py script and also fixed the incorrect value provided to __cpp_lib_to_array
I do not really know why there are changes to execution. Maybe someone forgot to run the script?
It seems it finally worked. No idea what went wrong as the changes were still in there
We're almost there! When you run generate_feature_test_macro_components.py, it prints out something like producing new <version> header as /var/folders/10/r6bw68bs5b9gwjtrnl9dz0vm0000gn/T/version.Xe5egJ at the end. You must mv /var/...../version.asdasd libcxx/include/version to replace the current <version> header with that one. Otherwise the __cpp_lib_span macro won't be defined properly. Currently, this causes the tests to fail with this patch applied.
Also, please exclude changes to other tests like execution.version.pass.cpp from this patch -- I'll generate them separately, I think it's an oversight.
I've updated the status page in commit 182adf120ccffe937d95d5b447f6506f82f615ec. I guess it was an oversight.
Could you please adjust the synopsis too?