The default constructor of a static span requires _Extent == 0 so SFINAE it out rather than using a static_assert
mclow.lists EricWF miscco CaseyCarter
- Group Reviewers
- rG79941086fba3: [libc++][span] SFINAE span default constructor on Extent == 0
Hm, this fixes a todo that was written by you..
No seriously I believe this is indeed a correctness issue.
First the standard marks this as a "Constraint". As far as my standardese goes this requires that this function should be SFINAEd out.
Also if we would not remove the default constructor span<T, 2> would fulfill the`default_constructible` concept although it does not.
So I believe there is no alternative to removing that constructor.
That is a compelling argument.
Since you pointed this out, we probably need a test showing that this is true for:
- dynamically sized spans
- statically sized spans of size 0
- but NOT for statically sized spans of size > 0
Library concepts are not yet implemented.
I had a look at it over the weekend. The problem is that requires expressions are not yet implemented so we cannot even add those concepts yet. I didnt find out how to pass -fconcepts to gcc-9 in LIT and was too lazy to build everything on windows with MSVC.
So I would keep them as a todo?
|17 ↗||(On Diff #236609)|
Late comment: <span> isn't guaranteed to export __cpp_lib_concepts; this should include <version> before checking __cpp_lib_concepts.
You are absolutely right.
While it is technically correct as both span and concepts are a C++20 feature I belive this change is not helpful. It is just soo much better :(
That said, we should switch to concepts once the library concepts and ranges are fully available as that is needed for conformance of the range based contructors.
How do I revert an update in phabricator?
I'm committing this, but I'm removing concepts from the tests.
Please don't require features we don't implement like that, it's just pointless. You basically have dead code in your test as it stands.