[PATCH] D69520: [libc++] Disallow dynamic -> static span conversions
Jan Wilken Dörrie via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sun Nov 3 03:59:29 PST 2019
jdoerrie added inline comments.
================
Comment at: libcxx/test/std/containers/views/span.cons/span.fail.cpp:78
-
-// Try to remove const and/or volatile (static -> static)
- {
----------------
mclow.lists wrote:
> Ok. The comment here is wrong; this is testing dynamic -> static.
>
> However, why are you removing these (failing) tests?
>
Considering that `Extent == dynamic_extent || Extent == OtherExtent` should be checked first I felt these particular tests distracted from the actual root cause of the error. I added now a new section that performs explicit Extent checks.
================
Comment at: libcxx/test/std/containers/views/span.cons/span.pass.cpp:80
+
+ return s1.data() == nullptr && s1.size() == 0 && s2.data() == nullptr &&
+ s2.size() == 0 && s3.data() == nullptr && s3.size() == 0;
----------------
mclow.lists wrote:
> Please line these up like the other ones were. Makes it easy to see copy-pasta errors:
> ```
> return s1.data() == nullptr && s1.size() == 0
> && s2.data() == nullptr && s2.size() == 0
> && s3.data() == nullptr && s3.size() == 0;
> ```
FWIW my formatting was performed by `clang-format`. Do you think it's worth wrapping this block in
```
// clang-format off
...
// clang-format on
```
so that this doesn't regress in the future?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69520/new/
https://reviews.llvm.org/D69520
More information about the cfe-commits
mailing list