Re: [patch] Implement ISO/IEC TS 18822 C++ File system TS

[Jonathan Wakely <jwakely@redhat.com>]

On 01/05/15 21:28 +0200, Daniel Krügler wrote:
>2015-05-01 20:22 GMT+02:00 Jonathan Wakely <jwakely@redhat.com>:
>> On 01/05/15 19:03 +0200, Daniel Krügler wrote:
>>>
>>> b/libstdc++-v3/src/filesystem/path.cc:
>>>
>>> - path::compare(const path& p) const noexcept:
>>>
>>> Shouldn't the implementation of this noexcept function not try to
>>> create copies of path objects? Couldn't _Cmpt just hold references to
>>> _M_pathname?
>>
>> All your other comments are definitely correct and I'll make the
>> relevant fixes soon, but I'm not sure what you mean here, could you
>> clarify?
>
>My remembrance of the difference in noexcept qualifications of
>
>int  compare(const path& p) const noexcept;
>int  compare(const string_type& s) const;
>int  compare(const value_type* s) const;
>
>is that the latter are allowed to allocate memory to be implemented as
>the standard writes the corresponding functions for std::basic_string:
>
>int compare(const basic_string& str) const noexcept;
>int compare(const charT* s) const;
>
>Returns: compare(basic_string(s)).
>
>But if I read your implementation of path::compare(const path& p)
>correctly it *also* may allocate memory by copying _M_pathname into a
>_Cmpt object.

Yes, I agree that there's a bug here that could cause it to
std::terminate.

>I was wondering whether for this comparison there exists
>real need to *copy* _M_pathname (potentially exceeding the memory
>limits). Wouldn't it be possible to define a _CmptRef type that for
>the purpose of implementing compare(const path& p) just refers to
>references of the _M_pathname and therefore doesn't need to allocate
>any dynamic storage? (I should have spoken of a new type _CmptRef and
>not of your existing _Cmpt).


Ah, I see what you mean (I thought you were suggesting some
improvements to _Cmpt itself ... which might be possible so I'm glad
you made me think about it :-)

I think I wrote compare() like that because it was easier, and when I
first implemented this we had COW strings so it wouldn't throw when
copying. That isn't true now, so I need to change it.