[libcxx-commits] [PATCH] D114912: [libc++] [P1614] Hidden-friend operator<=> for string_view.

[Joe Loser via Phabricator via libcxx-commits]

jloser added inline comments.


================
Comment at: libcxx/include/string_view:716
+        using _Rp = __comp_cat_t<_Traits, weak_ordering>;
+        static_assert(__is_comparison_category_v<_Rp>, "Mandates: R denotes a comparison category type");
+        return __lhs.compare(__rhs) <=> 0;
----------------
Quuxplusone wrote:
> jloser wrote:
> > Is the message more useful for developers //using libc++// or implementors? I suspect the former, in which case I would mention something about `traits​::​comparison_­category` rather than `R`. I claim `R` is not useful for developers as that requires them to look at the Standard to figure out `R` means `traits​::​comparison_­category`. WDYT?
> Very fair point. I was kinda sneaking in the camel's nose of a new style, that whenever we insert a `static_assert` for literally no other reason than "the Standard says mandate this," we should make the message of the `static_assert` match the wording in the Standard. Which, if the reader is familiar with the Standard, should make it pretty obvious what's going on and why. //But,// you correctly observe that most readers won't be familiar with the Standard and so copying the Standard wording (while consistent (if we did it consistently)) isn't terribly helpful to the reader.
> 
> I don't mind changing this to either the more traditional
> ```
> static_assert(__is_comparison_category_v<_Rp>, "Traits::comparison_category must be a comparison category type");
> ```
> or the hybrid
> ```
> static_assert(__is_comparison_category_v<_Rp>, "Mandates: Traits::comparison_category denotes a comparison category type");
> ```
> Thoughts (from anyone at all)?
I weakly prefer the hybrid:

```
static_assert(__is_comparison_category_v<_Rp>, "Mandates: Traits::comparison_category denotes a comparison category type");
```

I'd be fine with either of the two proposed different messages though.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114912/new/

https://reviews.llvm.org/D114912