Design question LWG 2861: basic_string should require that charT match traits::char_type

Design question LWG 2861: basic_string should require that charT match traits::char_type

[Daniel Krügler]

I'm now working on

http://cplusplus.github.io/LWG/lwg-defects.html#2861

The new wording state is now equivalent to basic_string_view, whose
current implementation doesn't bother verifying the requirement, so
this code (which as UB) currently compiles just fine:

#include <string>
#include <string_view>

struct MyTraits : std::char_traits<char>
{
  typedef unsigned char char_type;
};

int main()
{
  std::basic_string<char, MyTraits> my_string;
  std::basic_string_view<char, MyTraits> my_string_view;
}

So the least I could do is just - nothing. But it seems to me that we
could protect users from doing such silly things by adding a
static_assert to both basic_string and basic_string_view, the former
being equivalent to

#if __cplusplus >= 201103L
      static_assert(__are_same<value_type, _CharT>::value,
                    "traits_type::char_type must be equal to _CharT");
#endif

and the latter an unconditional

      static_assert(is_same<typename _Traits::char_type, _CharT>::value,
                    "traits_type::char_type must be equal to _CharT");

Would you agree with that course of action?

Thanks,

- Daniel

Re: Design question LWG 2861: basic_string should require that charT match traits::char_type

[Jonathan Wakely]

On 12 March 2017 at 13:21, Daniel Krügler <daniel.kruegler@gmail.com> wrote:
> I'm now working on
>
> http://cplusplus.github.io/LWG/lwg-defects.html#2861
>
> The new wording state is now equivalent to basic_string_view, whose
> current implementation doesn't bother verifying the requirement, so
> this code (which as UB) currently compiles just fine:
>
> #include <string>
> #include <string_view>
>
> struct MyTraits : std::char_traits<char>
> {
>   typedef unsigned char char_type;
> };
>
> int main()
> {
>   std::basic_string<char, MyTraits> my_string;
>   std::basic_string_view<char, MyTraits> my_string_view;
> }
>
> So the least I could do is just - nothing. But it seems to me that we
> could protect users from doing such silly things by adding a
> static_assert to both basic_string and basic_string_view, the former
> being equivalent to
>
> #if __cplusplus >= 201103L
>       static_assert(__are_same<value_type, _CharT>::value,
>                     "traits_type::char_type must be equal to _CharT");
> #endif
>
> and the latter an unconditional
>
>       static_assert(is_same<typename _Traits::char_type, _CharT>::value,
>                     "traits_type::char_type must be equal to _CharT");
>
> Would you agree with that course of action?

Not at this stage of gcc7 development. If the silly code compile fine
then we risk breaking working code, and we're too close to a release
to do that.

We can reconsider for gcc8 (but even then, the code has undefined
behaviour, so it would be a QoI choice whether to reject it or just
accept it, as we do for containers where Alloc::value_type doesn't
match the container's value_type).

Re: Design question LWG 2861: basic_string should require that charT match traits::char_type

[Daniel Krügler]

2017-03-13 11:56 GMT+01:00 Jonathan Wakely <jwakely.gcc@gmail.com>:
> On 12 March 2017 at 13:21, Daniel Krügler <daniel.kruegler@gmail.com> wrote:
>> I'm now working on
>>
>> http://cplusplus.github.io/LWG/lwg-defects.html#2861
>>
>> The new wording state is now equivalent to basic_string_view, whose
>> current implementation doesn't bother verifying the requirement, so
>> this code (which as UB) currently compiles just fine:
>>
>> #include <string>
>> #include <string_view>
>>
>> struct MyTraits : std::char_traits<char>
>> {
>>   typedef unsigned char char_type;
>> };
>>
>> int main()
>> {
>>   std::basic_string<char, MyTraits> my_string;
>>   std::basic_string_view<char, MyTraits> my_string_view;
>> }
>>
>> So the least I could do is just - nothing. But it seems to me that we
>> could protect users from doing such silly things by adding a
>> static_assert to both basic_string and basic_string_view, the former
>> being equivalent to
>>
>> #if __cplusplus >= 201103L
>>       static_assert(__are_same<value_type, _CharT>::value,
>>                     "traits_type::char_type must be equal to _CharT");
>> #endif
>>
>> and the latter an unconditional
>>
>>       static_assert(is_same<typename _Traits::char_type, _CharT>::value,
>>                     "traits_type::char_type must be equal to _CharT");
>>
>> Would you agree with that course of action?
>
> Not at this stage of gcc7 development. If the silly code compile fine
> then we risk breaking working code, and we're too close to a release
> to do that.

Is there a way to mark a patch suggestion for gcc8 and is so, how?

> We can reconsider for gcc8 (but even then, the code has undefined
> behaviour, so it would be a QoI choice whether to reject it or just
> accept it, as we do for containers where Alloc::value_type doesn't
> match the container's value_type).

Yes, sure, purely QoI, but the fix seems to be a no-brainer.

- Daniel

Re: Design question LWG 2861: basic_string should require that charT match traits::char_type

[Jonathan Wakely]

On 13 March 2017 at 11:33, Daniel Krügler <daniel.kruegler@gmail.com> wrote:
> 2017-03-13 11:56 GMT+01:00 Jonathan Wakely <jwakely.gcc@gmail.com>:
>> On 12 March 2017 at 13:21, Daniel Krügler <daniel.kruegler@gmail.com> wrote:
>>> I'm now working on
>>>
>>> http://cplusplus.github.io/LWG/lwg-defects.html#2861
>>>
>>> The new wording state is now equivalent to basic_string_view, whose
>>> current implementation doesn't bother verifying the requirement, so
>>> this code (which as UB) currently compiles just fine:
>>>
>>> #include <string>
>>> #include <string_view>
>>>
>>> struct MyTraits : std::char_traits<char>
>>> {
>>>   typedef unsigned char char_type;
>>> };
>>>
>>> int main()
>>> {
>>>   std::basic_string<char, MyTraits> my_string;
>>>   std::basic_string_view<char, MyTraits> my_string_view;
>>> }
>>>
>>> So the least I could do is just - nothing. But it seems to me that we
>>> could protect users from doing such silly things by adding a
>>> static_assert to both basic_string and basic_string_view, the former
>>> being equivalent to
>>>
>>> #if __cplusplus >= 201103L
>>>       static_assert(__are_same<value_type, _CharT>::value,
>>>                     "traits_type::char_type must be equal to _CharT");
>>> #endif
>>>
>>> and the latter an unconditional
>>>
>>>       static_assert(is_same<typename _Traits::char_type, _CharT>::value,
>>>                     "traits_type::char_type must be equal to _CharT");
>>>
>>> Would you agree with that course of action?
>>
>> Not at this stage of gcc7 development. If the silly code compile fine
>> then we risk breaking working code, and we're too close to a release
>> to do that.
>
> Is there a way to mark a patch suggestion for gcc8 and is so, how?

Just mention it in the email. Ideally ping the patch after gcc7 is
released so someone (probably me) can apply it once we're back in
Stage 1.