Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Can you also update docs/Status/RangesPaper.csv?
libcxx/include/__ranges/data.h | ||
---|---|---|
76 | Is there any reason to close the ranges namespace above and then reopen it here? | |
78 | I think we agreed to only indent inside __cpo namespaces, so this struct should not have any extra indentation. | |
84 | Question: would decltype(auto) work here? |
LGTM
libcxx/include/__ranges/data.h | ||
---|---|---|
83–85 | The standard only says static_cast<const T&>(), but that doesn't make much sense, so I'm guessing that's a wording defect? |
libcxx/include/__ranges/data.h | ||
---|---|---|
76 | https://reviews.llvm.org/D89057#inline-1117677 | |
78 | True, we did. But I guess I only took that as far as reverting the indentation in uninitialized_foo; I didn't actually go remove the existing indentation in begin/end/cbegin/etc. This was copied from cbegin. I'll go remove that existing indentation tomorrow, unless someone else objects before then (which will certainly give me enough of an excuse to avoid doing it, again ;)) | |
83–85 | Serendipitously, Casey Carter emphasized the answer just today in D116991. The standard uses T to refer to the "type" of the expression, which is never a reference type (although it may be cv-qualified). On paper, you can have an lvalue expression of type const int, but you can't have an lvalue expression of type const int&. So when in libc++ we take a parameter of declared type _Tp&&, that corresponds to a paper expression E of "type" remove_reference_t<_Tp>. | |
84 | No, because we need SFINAE. |
LGTM (modulo the one agreed-upon style nitpick).
libcxx/include/__ranges/data.h | ||
---|---|---|
76 | Personally, I'd rather have everything in a single contiguous namespace. When I see this reopening, it makes me wonder whether this file contains other nested namespaces (whatever they might be) and leads me to expand the file and start searching around. | |
84 | Thanks for the link! |
LGTM with passing CI and updated status paper.
Interesting learn about what the Standard means when it says "the type of an expression"!
libcxx/docs/Status/RangesPaper.csv | ||
---|---|---|
100 ↗ | (On Diff #399529) | I think you should also add your name here (so that if anyone is considering taking one of the remaining items in this section, they would know to talk to you). |
Is there any reason to close the ranges namespace above and then reopen it here?