[libcxx-commits] [PATCH] D69520: [libc++] Disallow dynamic -> static span conversions

[Jan Wilken Dörrie via Phabricator via libcxx-commits]

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