Your suggestion is essentially to replace experimental/string_view with 
something like:

    namespace std { inline namespace __1 { namespace experimental {
      template <class CharT>
      using basic_string_view = _VSTD::basic_string_view;
    }}}

That breaks:
1. User compiles 1.cpp with older toolchain.  1.cpp implements 
foo(std::experimental::string_view).
2. User compiles 2.cpp with newer toolchain.  2.cpp calls 
foo(std::experimental::string_view).
3. User links 1.o with 2.o.

I'm not sure if this matters.

> On Jun 15, 2017, at 21:55, Eric Fiselier <e...@efcs.ca> wrote:
> 
> I would also want to do serious performance analysis on this patch. Does 
> removing the string_view overloads cause less optimal overloads to be chosen? 
> Perhaps allocating ones?
> That would be really unfortunate, and I'm not sure that's in the best 
> interest of our users at large.

Less optimal compared to what?  C++17 code?

> /Eric
> 
> On Thu, Jun 15, 2017 at 10:51 PM, Duncan P. N. Exon Smith 
> <dexonsm...@apple.com <mailto:dexonsm...@apple.com>> wrote:
> 
>> On Jun 15, 2017, at 19:42, Eric Fiselier <e...@efcs.ca 
>> <mailto:e...@efcs.ca>> wrote:
>> 
>> 
>> 
>> On Thu, Jun 15, 2017 at 8:38 PM, Duncan P. N. Exon Smith 
>> <dexonsm...@apple.com <mailto:dexonsm...@apple.com>> wrote:
>> I just started working on a patch to add #if guards, and the first 
>> interesting thing I found was the basic_string constructor:
>> 
>>> template <class _CharT, class _Traits, class _Allocator>
>>> template <class _Tp>
>>> basic_string<_CharT, _Traits, _Allocator>::basic_string(
>>>              const _Tp& __t, size_type __pos, size_type __n, const 
>>> allocator_type& __a,
>>>                      typename 
>>> enable_if<__can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value, 
>>> void>::type *)
>>>     : __r_(__second_tag(), __a)
>>> {
>>>     __self_view __sv = __self_view(__t).substr(__pos, __n);
>>>     __init(__sv.data(), __sv.size());
>>> #if _LIBCPP_DEBUG_LEVEL >= 2
>>>     __get_db()->__insert_c(this);
>>> #endif
>>> }
>> 
>> 
>> That constructor was added in C++17, so removing it along with string_view 
>> should be OK.
>> Assuming we don't use it to implement older constructors using a single 
>> template.
>> 
>>  
>> I suppose the decision was made so that std::string could take advantage of 
>> it.
>> 
>> Is it a conforming extension?
>> 
>> No, because it can change the meaning of otherwise well defined code, as you 
>> pointed out initially. 
> 
> Let me know if this patch is along the right lines.  If so, I'll finish it up 
> and put it on phab.
> 
> experimental/filesystem/path.cpp doesn't compile, since 
> experimental/filesystem uses things like operator+=(string, string_view) 
> extensively.  But I'd like an early opinion on the approach before I dig in.
> 
> In string, the only function that needed to be rewritten was 
> string::compare(size, size, string, size, size).  I'm nervous that filesystem 
> will be a bigger job.
> 
> 
> 
>>  
>> 
>>> On Jun 15, 2017, at 18:35, Eric Fiselier <e...@efcs.ca 
>>> <mailto:e...@efcs.ca>> wrote:
>>> 
>>> It *shouldn't* include <string_view>, that's a given.
>>> 
>>> IIRC, and Marshall would know better, I believe it was untenable to
>>> maintain a version of <string> that didn't depend on <string_view> after 
>>> making
>>> the changes required for C++17.
>>> 
>>> However inspecting <string> now it does seem possible that the entanglement
>>> is avoidable.Though it's also likely I'm just not seeing the whole picture. 
>>> 
>>> /Eric
>>> 
>>> On Thu, Jun 15, 2017 at 6:42 PM, Duncan P. N. Exon Smith 
>>> <dexonsm...@apple.com <mailto:dexonsm...@apple.com>>wrote:
>>> 
>>> > On Jul 20, 2016, at 22:31, Marshall Clow via cfe-commits 
>>> > <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote:
>>> >
>>> > Modified: libcxx/trunk/include/string
>>> > URL: 
>>> > http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=276238&r1=276237&r2=276238&view=diff
>>> >  
>>> > <http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=276238&r1=276237&r2=276238&view=diff>
>>> > ==============================================================================
>>> >
>>> > @@ -435,6 +461,7 @@ basic_string<char32_t> operator "" s( co
>>> > */
>>> >
>>> > #include <__config>
>>> > +#include <string_view>
>>> 
>>> This breaks the following, valid, C++14 code:
>>> 
>>>     #include <string>
>>>     #include <experimental/string_view>
>>>     using namespace std;
>>>     using std::experimental::string_view;
>>>     void f() { string_view sv; }
>>> 
>>> Should <string> #include <string_view> even when we're not in C++17 mode?  
>>> Why?
>>> 
>>> > #include <iosfwd>
>>> > #include <cstring>
>>> > #include <cstdio>  // For EOF.
> 
> 
> 

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to