Re: [libcxx] r276238 - Implement std::string_view as described in http://wg21.link/P0254R1. Reviewed as https://reviews.llvm.org/D21459

[Duncan P. N. Exon Smith via cfe-commits]

> On 2017-Jun-18, at 10:46, Duncan P. N. Exon Smith  
> wrote:
> 
>> 
>> On 2017-Jun-16, at 05:58, Duncan Exon Smith > > wrote:
>> 
>> 
>> On Jun 15, 2017, at 22:22, Eric Fiselier > > wrote:
>> 
>>> 
>>> 
>>> On Thu, Jun 15, 2017 at 11:00 PM, Duncan P. N. Exon Smith 
>>> > wrote:
>>> Your suggestion is essentially to replace experimental/string_view with 
>>> something like:
>>> 
>>> namespace std { inline namespace __1 { namespace experimental {
>>>   template 
>>>   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.
>>> 
>>> It can't matter.  are allowed to break both their API and 
>>> ABI as needed.
>>> 
>>> Also I was suggesting 
>>> 
>>>namespace std { namespace experimental {
>>>  using std::basic_string_view;
>>>  using std::string_view;
>>>   }}
>>>  
>>> This approach will break code that expects experimental::string_view and 
>>> std::string_view are different types:
>>> Example:
>>> 
>>>   void foo(std::string_view);
>>>   void foo(std::experimental::string_view);
>>>   foo(std::string_view{}); // ambiguous
> 
> FTR, it also breaks code that relies on string_view::clear(), which 
> disappeared.

More importantly, it breaks code that relies on string_view::to_string().  This 
one matters, since a "true" std::experimental::string_view wouldn't have the 
implicit conversions.

> 
 On Jun 15, 2017, at 21:55, Eric Fiselier > 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?
>>> 
>>> Not sure yet, I'm trying to figure out what types the `const Tp&` overloads
>>> are attempting to soak up. Is it only string_view? 
>> 
>> The type trait restricts it to things convertible to string_view that are 
>> not const char *.
> 
> I had a bit of a look at experimental/filesystem, and it relies pretty 
> heavily on the string/string_view conversions.  I still feel like this 
> approach might be "the right one", but perhaps it's not worth it.
> 
 /Eric
 
 On Thu, Jun 15, 2017 at 10:51 PM, Duncan P. N. Exon Smith 
 > wrote:
 
> On Jun 15, 2017, at 19:42, Eric Fiselier  > wrote:
> 
> 
> 
> On Thu, Jun 15, 2017 at 8:38 PM, Duncan P. N. Exon Smith 
> > 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 
>> template 
>> 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 

Re: [libcxx] r276238 - Implement std::string_view as described in http://wg21.link/P0254R1. Reviewed as https://reviews.llvm.org/D21459

[Duncan P. N. Exon Smith via cfe-commits]

> On 2017-Jun-16, at 05:58, Duncan Exon Smith  wrote:
> 
> 
> On Jun 15, 2017, at 22:22, Eric Fiselier > 
> wrote:
> 
>> 
>> 
>> On Thu, Jun 15, 2017 at 11:00 PM, Duncan P. N. Exon Smith 
>> > wrote:
>> Your suggestion is essentially to replace experimental/string_view with 
>> something like:
>> 
>> namespace std { inline namespace __1 { namespace experimental {
>>   template 
>>   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.
>> 
>> It can't matter.  are allowed to break both their API and 
>> ABI as needed.
>> 
>> Also I was suggesting 
>> 
>>namespace std { namespace experimental {
>>  using std::basic_string_view;
>>  using std::string_view;
>>   }}
>>  
>> This approach will break code that expects experimental::string_view and 
>> std::string_view are different types:
>> Example:
>> 
>>   void foo(std::string_view);
>>   void foo(std::experimental::string_view);
>>   foo(std::string_view{}); // ambiguous

FTR, it also breaks code that relies on string_view::clear(), which disappeared.

>>> On Jun 15, 2017, at 21:55, Eric Fiselier >> > 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?
>> 
>> Not sure yet, I'm trying to figure out what types the `const Tp&` overloads
>> are attempting to soak up. Is it only string_view? 
> 
> The type trait restricts it to things convertible to string_view that are not 
> const char *.

I had a bit of a look at experimental/filesystem, and it relies pretty heavily 
on the string/string_view conversions.  I still feel like this approach might 
be "the right one", but perhaps it's not worth it.

>>> /Eric
>>> 
>>> On Thu, Jun 15, 2017 at 10:51 PM, Duncan P. N. Exon Smith 
>>> > wrote:
>>> 
 On Jun 15, 2017, at 19:42, Eric Fiselier > wrote:
 
 
 
 On Thu, Jun 15, 2017 at 8:38 PM, Duncan P. N. Exon Smith 
 > 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 
> template 
> 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  > wrote:
> 
> It *shouldn't* include , that's a given.
> 
> IIRC, and Marshall would know better, I believe it was untenable to
> maintain a version of  that didn't depend on  after 
> making
> the changes required for C++17.
> 
> However inspecting  now it does seem possible that the 
> entanglement
> is avoidable.Though it's also likely I'm just not seeing the whole 
> 

Re: [libcxx] r276238 - Implement std::string_view as described in http://wg21.link/P0254R1. Reviewed as https://reviews.llvm.org/D21459

[Duncan Exon Smith via cfe-commits]

> On Jun 15, 2017, at 22:22, Eric Fiselier  wrote:
> 
> 
> 
>> On Thu, Jun 15, 2017 at 11:00 PM, Duncan P. N. Exon Smith 
>>  wrote:
>> Your suggestion is essentially to replace experimental/string_view with 
>> something like:
>> 
>> namespace std { inline namespace __1 { namespace experimental {
>>   template 
>>   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.
> 
> It can't matter.  are allowed to break both their API and 
> ABI as needed.
> 
> Also I was suggesting 
> 
>namespace std { namespace experimental {
>  using std::basic_string_view;
>  using std::string_view;
>   }}
>  
> This approach will break code that expects experimental::string_view and 
> std::string_view are different types:
> Example:
> 
>   void foo(std::string_view);
>   void foo(std::experimental::string_view);
>   foo(std::string_view{}); // ambiguous
> 
>> 
>>> On Jun 15, 2017, at 21:55, Eric Fiselier  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?
> 
> Not sure yet, I'm trying to figure out what types the `const Tp&` overloads
> are attempting to soak up. Is it only string_view? 

The type trait restricts it to things convertible to string_view that are not 
const char *.

> 
>  
>> 
>>> /Eric
>>> 
 On Thu, Jun 15, 2017 at 10:51 PM, Duncan P. N. Exon Smith 
  wrote:
 
> On Jun 15, 2017, at 19:42, Eric Fiselier  wrote:
> 
> 
> 
> On Thu, Jun 15, 2017 at 8:38 PM, Duncan P. N. Exon Smith 
>  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 
>>> template 
>>> 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  wrote:
>>> 
>>> It *shouldn't* include , that's a given.
>>> 
>>> IIRC, and Marshall would know better, I believe it was untenable to
>>> maintain a version of  that didn't depend on  
>>> after making
>>> the changes required for C++17.
>>> 
>>> However inspecting  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 
>>> wrote:
 
 > On Jul 20, 2016, at 22:31, Marshall Clow via cfe-commits 
 >  wrote:
 >
 > Modified: libcxx/trunk/include/string
 > URL: 
 > http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=276238=276237=276238=diff

Re: [libcxx] r276238 - Implement std::string_view as described in http://wg21.link/P0254R1. Reviewed as https://reviews.llvm.org/D21459

[Eric Fiselier via cfe-commits]

On Jun 15, 2017 11:22 PM, "Eric Fiselier"  wrote:



On Thu, Jun 15, 2017 at 11:00 PM, Duncan P. N. Exon Smith <
dexonsm...@apple.com> wrote:

> Your suggestion is essentially to replace experimental/string_view with
> something like:
>
> namespace std { inline namespace __1 { namespace experimental {
>   template 
>   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.
>

It can't matter.  are allowed to break both their API and
ABI as needed.

Also I was suggesting

   namespace std { namespace experimental {
 using std::basic_string_view;
 using std::string_view;
  }}

This approach will break code that expects experimental::string_view and
std::string_view are different types:
Example:

  void foo(std::string_view);
  void foo(std::experimental::string_view);
  foo(std::string_view{}); // ambiguous


* redeclaration, not ambiguous ovls.



> On Jun 15, 2017, at 21:55, Eric Fiselier  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?
>

Not sure yet, I'm trying to figure out what types the `const Tp&` overloads
are attempting to soak up. Is it only string_view?



>
> /Eric
>
> On Thu, Jun 15, 2017 at 10:51 PM, Duncan P. N. Exon Smith <
> dexonsm...@apple.com> wrote:
>
>>
>> On Jun 15, 2017, at 19:42, Eric Fiselier  wrote:
>>
>>
>>
>> On Thu, Jun 15, 2017 at 8:38 PM, Duncan P. N. Exon Smith <
>> 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 
>>> template 
>>> 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  wrote:
>>>
>>> It *shouldn't* include , that's a given.
>>>
>>> IIRC, and Marshall would know better, I believe it was untenable to
>>> maintain a version of  that didn't depend on  after
>>> making
>>> the changes required for C++17.
>>>
>>> However inspecting  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>wrote:
>>>

 > On Jul 20, 2016, at 22:31, Marshall Clow via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:
 >
 > Modified: libcxx/trunk/include/string
 > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/includ
 e/string?rev=276238=276237=276238=diff
 > 
 ==
 >
 > @@ -435,6 +461,7 @@ basic_string operator "" s( co
 > */
 >
 > #include <__config>
 > +#include 

 This breaks the following, valid, C++14 code:

 #include 
 #include 
 using namespace std;
 using std::experimental::string_view;
 void f() { string_view sv; }

 Should  #include  even when we're not in C++17
 mode?  Why?

 

Re: [libcxx] r276238 - Implement std::string_view as described in http://wg21.link/P0254R1. Reviewed as https://reviews.llvm.org/D21459

[Eric Fiselier via cfe-commits]

On Thu, Jun 15, 2017 at 11:00 PM, Duncan P. N. Exon Smith <
dexonsm...@apple.com> wrote:

> Your suggestion is essentially to replace experimental/string_view with
> something like:
>
> namespace std { inline namespace __1 { namespace experimental {
>   template 
>   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.
>

It can't matter.  are allowed to break both their API and
ABI as needed.

Also I was suggesting

   namespace std { namespace experimental {
 using std::basic_string_view;
 using std::string_view;
  }}

This approach will break code that expects experimental::string_view and
std::string_view are different types:
Example:

  void foo(std::string_view);
  void foo(std::experimental::string_view);
  foo(std::string_view{}); // ambiguous


> On Jun 15, 2017, at 21:55, Eric Fiselier  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?
>

Not sure yet, I'm trying to figure out what types the `const Tp&` overloads
are attempting to soak up. Is it only string_view?



>
> /Eric
>
> On Thu, Jun 15, 2017 at 10:51 PM, Duncan P. N. Exon Smith <
> dexonsm...@apple.com> wrote:
>
>>
>> On Jun 15, 2017, at 19:42, Eric Fiselier  wrote:
>>
>>
>>
>> On Thu, Jun 15, 2017 at 8:38 PM, Duncan P. N. Exon Smith <
>> 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 
>>> template 
>>> 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  wrote:
>>>
>>> It *shouldn't* include , that's a given.
>>>
>>> IIRC, and Marshall would know better, I believe it was untenable to
>>> maintain a version of  that didn't depend on  after
>>> making
>>> the changes required for C++17.
>>>
>>> However inspecting  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>wrote:
>>>

 > On Jul 20, 2016, at 22:31, Marshall Clow via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:
 >
 > Modified: libcxx/trunk/include/string
 > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/includ
 e/string?rev=276238=276237=276238=diff
 > 
 ==
 >
 > @@ -435,6 +461,7 @@ basic_string operator "" s( co
 > */
 >
 > #include <__config>
 > +#include 

 This breaks the following, valid, C++14 code:

 #include 
 #include 
 using namespace std;
 using std::experimental::string_view;
 void f() { string_view sv; }

 Should  #include  even when we're not in C++17
 mode?  Why?

 > #include 
 > #include 
 > #include   // For EOF.

>>>
>>
>>
>
>

Re: [libcxx] r276238 - Implement std::string_view as described in http://wg21.link/P0254R1. Reviewed as https://reviews.llvm.org/D21459

[Duncan P. N. Exon Smith via cfe-commits]

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

namespace std { inline namespace __1 { namespace experimental {
  template 
  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  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 
> > wrote:
> 
>> On Jun 15, 2017, at 19:42, Eric Fiselier > > wrote:
>> 
>> 
>> 
>> On Thu, Jun 15, 2017 at 8:38 PM, Duncan P. N. Exon Smith 
>> > 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 
>>> template 
>>> 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 >> > wrote:
>>> 
>>> It *shouldn't* include , that's a given.
>>> 
>>> IIRC, and Marshall would know better, I believe it was untenable to
>>> maintain a version of  that didn't depend on  after 
>>> making
>>> the changes required for C++17.
>>> 
>>> However inspecting  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 
>>> >wrote:
>>> 
>>> > On Jul 20, 2016, at 22:31, Marshall Clow via cfe-commits 
>>> > > wrote:
>>> >
>>> > Modified: libcxx/trunk/include/string
>>> > URL: 
>>> > http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=276238=276237=276238=diff
>>> >  
>>> > 
>>> > ==
>>> >
>>> > @@ -435,6 +461,7 @@ basic_string operator "" s( co
>>> > */
>>> >
>>> > #include <__config>
>>> > +#include 
>>> 
>>> This breaks the following, valid, C++14 code:
>>> 
>>> #include 
>>> #include 
>>> using namespace std;
>>> using std::experimental::string_view;
>>> void f() { string_view sv; }
>>> 
>>> Should  #include  even when we're not in C++17 mode?  
>>> Why?
>>> 
>>> > #include 
>>> > #include 
>>> > #include   // For EOF.
> 
> 
> 

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

Re: [libcxx] r276238 - Implement std::string_view as described in http://wg21.link/P0254R1. Reviewed as https://reviews.llvm.org/D21459

[Eric Fiselier via cfe-commits]

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.

/Eric

On Thu, Jun 15, 2017 at 10:51 PM, Duncan P. N. Exon Smith <
dexonsm...@apple.com> wrote:

>
> On Jun 15, 2017, at 19:42, Eric Fiselier  wrote:
>
>
>
> On Thu, Jun 15, 2017 at 8:38 PM, Duncan P. N. Exon Smith <
> 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 
>> template 
>> 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  wrote:
>>
>> It *shouldn't* include , that's a given.
>>
>> IIRC, and Marshall would know better, I believe it was untenable to
>> maintain a version of  that didn't depend on  after
>> making
>> the changes required for C++17.
>>
>> However inspecting  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>wrote:
>>
>>>
>>> > On Jul 20, 2016, at 22:31, Marshall Clow via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>> >
>>> > Modified: libcxx/trunk/include/string
>>> > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/
>>> include/string?rev=276238=276237=276238=diff
>>> > 
>>> ==
>>> >
>>> > @@ -435,6 +461,7 @@ basic_string operator "" s( co
>>> > */
>>> >
>>> > #include <__config>
>>> > +#include 
>>>
>>> This breaks the following, valid, C++14 code:
>>>
>>> #include 
>>> #include 
>>> using namespace std;
>>> using std::experimental::string_view;
>>> void f() { string_view sv; }
>>>
>>> Should  #include  even when we're not in C++17
>>> mode?  Why?
>>>
>>> > #include 
>>> > #include 
>>> > #include   // For EOF.
>>>
>>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [libcxx] r276238 - Implement std::string_view as described in http://wg21.link/P0254R1. Reviewed as https://reviews.llvm.org/D21459

[Eric Fiselier via cfe-commits]

I should mention that  depends on C++17
string_view in older dialects :-S
So this change will break that.

I would prefer to fix your specific use case by making
std::experimental::string_view literally be
std::string_view.

/Eric

On Thu, Jun 15, 2017 at 8:42 PM, Eric Fiselier  wrote:

>
>
> On Thu, Jun 15, 2017 at 8:38 PM, Duncan P. N. Exon Smith <
> 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 
>> template 
>> 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.
>
>
>>
>> On Jun 15, 2017, at 18:35, Eric Fiselier  wrote:
>>
>> It *shouldn't* include , that's a given.
>>
>> IIRC, and Marshall would know better, I believe it was untenable to
>> maintain a version of  that didn't depend on  after
>> making
>> the changes required for C++17.
>>
>> However inspecting  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> wrote:
>>
>>>
>>> > On Jul 20, 2016, at 22:31, Marshall Clow via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>> >
>>> > Modified: libcxx/trunk/include/string
>>> > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/str
>>> ing?rev=276238=276237=276238=diff
>>> > 
>>> ==
>>> >
>>> > @@ -435,6 +461,7 @@ basic_string operator "" s( co
>>> > */
>>> >
>>> > #include <__config>
>>> > +#include 
>>>
>>> This breaks the following, valid, C++14 code:
>>>
>>> #include 
>>> #include 
>>> using namespace std;
>>> using std::experimental::string_view;
>>> void f() { string_view sv; }
>>>
>>> Should  #include  even when we're not in C++17
>>> mode?  Why?
>>>
>>> > #include 
>>> > #include 
>>> > #include   // For EOF.
>>>
>>>
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [libcxx] r276238 - Implement std::string_view as described in http://wg21.link/P0254R1. Reviewed as https://reviews.llvm.org/D21459

[Duncan P. N. Exon Smith via cfe-commits]

On Jun 15, 2017, at 19:42, Eric Fiselier  wrote:On Thu, Jun 15, 2017 at 8:38 PM, Duncan P. N. Exon Smith  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 template 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.

avoid-string-view-in-cxx14.patch
Description: Binary data
 On Jun 15, 2017, at 18:35, Eric Fiselier  wrote:It *shouldn't* include , that's a given.IIRC, and Marshall would know better, I believe it was untenable tomaintain a version of  that didn't depend on  after makingthe changes required for C++17.However inspecting  now it does seem possible that the entanglementis avoidable.Though it's also likely I'm just not seeing the whole picture. /EricOn Thu, Jun 15, 2017 at 6:42 PM, Duncan P. N. Exon Smith wrote:> On Jul 20, 2016, at 22:31, Marshall Clow via cfe-commits  wrote:>> Modified: libcxx/trunk/include/string> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=276238=276237=276238=diff> ==>> @@ -435,6 +461,7 @@ basic_string operator "" s( co> */>> #include <__config>> +#include This breaks the following, valid, C++14 code:    #include     #include     using namespace std;    using std::experimental::string_view;    void f() { string_view sv; }Should  #include  even when we're not in C++17 mode?  Why?> #include > #include > #include   // For EOF.___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [libcxx] r276238 - Implement std::string_view as described in http://wg21.link/P0254R1. Reviewed as https://reviews.llvm.org/D21459

[Eric Fiselier via cfe-commits]

On Thu, Jun 15, 2017 at 8:38 PM, Duncan P. N. Exon Smith <
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 
> template 
> 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.


>
> On Jun 15, 2017, at 18:35, Eric Fiselier  wrote:
>
> It *shouldn't* include , that's a given.
>
> IIRC, and Marshall would know better, I believe it was untenable to
> maintain a version of  that didn't depend on  after
> making
> the changes required for C++17.
>
> However inspecting  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> wrote:
>
>>
>> > On Jul 20, 2016, at 22:31, Marshall Clow via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>> >
>> > Modified: libcxx/trunk/include/string
>> > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/str
>> ing?rev=276238=276237=276238=diff
>> > 
>> ==
>> >
>> > @@ -435,6 +461,7 @@ basic_string operator "" s( co
>> > */
>> >
>> > #include <__config>
>> > +#include 
>>
>> This breaks the following, valid, C++14 code:
>>
>> #include 
>> #include 
>> using namespace std;
>> using std::experimental::string_view;
>> void f() { string_view sv; }
>>
>> Should  #include  even when we're not in C++17
>> mode?  Why?
>>
>> > #include 
>> > #include 
>> > #include   // For EOF.
>>
>>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [libcxx] r276238 - Implement std::string_view as described in http://wg21.link/P0254R1. Reviewed as https://reviews.llvm.org/D21459

[Duncan P. N. Exon Smith via cfe-commits]

Ah, also the enable_if for same:

> template
> _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS
> basic_string(const _Tp& __t, size_type __pos, size_type __n,
>  const allocator_type& __a = allocator_type(),
>  typename 
> enable_if<__can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value, 
> void>::type* = 0);

(Even if it is a conforming extension, it's fairly user hostile.  Doesn't mean 
it's necessarily wrong, but...)

> On Jun 15, 2017, at 19:38, Duncan P. N. Exon Smith via cfe-commits 
>  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 
>> template 
>> 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
>> }
> 
> I suppose the decision was made so that std::string could take advantage of 
> it.
> 
> Is it a conforming extension?
> 
>> On Jun 15, 2017, at 18:35, Eric Fiselier > > wrote:
>> 
>> It *shouldn't* include , that's a given.
>> 
>> IIRC, and Marshall would know better, I believe it was untenable to
>> maintain a version of  that didn't depend on  after 
>> making
>> the changes required for C++17.
>> 
>> However inspecting  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 
>> > wrote:
>> 
>> > On Jul 20, 2016, at 22:31, Marshall Clow via cfe-commits 
>> > > wrote:
>> >
>> > Modified: libcxx/trunk/include/string
>> > URL: 
>> > http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=276238=276237=276238=diff
>> >  
>> > 
>> > ==
>> >
>> > @@ -435,6 +461,7 @@ basic_string operator "" s( co
>> > */
>> >
>> > #include <__config>
>> > +#include 
>> 
>> This breaks the following, valid, C++14 code:
>> 
>> #include 
>> #include 
>> using namespace std;
>> using std::experimental::string_view;
>> void f() { string_view sv; }
>> 
>> Should  #include  even when we're not in C++17 mode?  
>> Why?
>> 
>> > #include 
>> > #include 
>> > #include   // For EOF.
>> 
>> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

Re: [libcxx] r276238 - Implement std::string_view as described in http://wg21.link/P0254R1. Reviewed as https://reviews.llvm.org/D21459

[Duncan P. N. Exon Smith via cfe-commits]

I just started working on a patch to add #if guards, and the first interesting 
thing I found was the basic_string constructor:

> template 
> template 
> 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
> }

I suppose the decision was made so that std::string could take advantage of it.

Is it a conforming extension?

> On Jun 15, 2017, at 18:35, Eric Fiselier  wrote:
> 
> It *shouldn't* include , that's a given.
> 
> IIRC, and Marshall would know better, I believe it was untenable to
> maintain a version of  that didn't depend on  after 
> making
> the changes required for C++17.
> 
> However inspecting  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 
> > wrote:
> 
> > On Jul 20, 2016, at 22:31, Marshall Clow via cfe-commits 
> > > wrote:
> >
> > Modified: libcxx/trunk/include/string
> > URL: 
> > http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=276238=276237=276238=diff
> >  
> > 
> > ==
> >
> > @@ -435,6 +461,7 @@ basic_string operator "" s( co
> > */
> >
> > #include <__config>
> > +#include 
> 
> This breaks the following, valid, C++14 code:
> 
> #include 
> #include 
> using namespace std;
> using std::experimental::string_view;
> void f() { string_view sv; }
> 
> Should  #include  even when we're not in C++17 mode?  
> Why?
> 
> > #include 
> > #include 
> > #include   // For EOF.
> 
> 

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

Re: [libcxx] r276238 - Implement std::string_view as described in http://wg21.link/P0254R1. Reviewed as https://reviews.llvm.org/D21459

[Eric Fiselier via cfe-commits]

It *shouldn't* include , that's a given.

IIRC, and Marshall would know better, I believe it was untenable to
maintain a version of  that didn't depend on  after
making
the changes required for C++17.

However inspecting  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> wrote:

>
> > On Jul 20, 2016, at 22:31, Marshall Clow via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >
> > Modified: libcxx/trunk/include/string
> > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/
> string?rev=276238=276237=276238=diff
> > 
> ==
> >
> > @@ -435,6 +461,7 @@ basic_string operator "" s( co
> > */
> >
> > #include <__config>
> > +#include 
>
> This breaks the following, valid, C++14 code:
>
> #include 
> #include 
> using namespace std;
> using std::experimental::string_view;
> void f() { string_view sv; }
>
> Should  #include  even when we're not in C++17 mode?
> Why?
>
> > #include 
> > #include 
> > #include   // For EOF.
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [libcxx] r276238 - Implement std::string_view as described in http://wg21.link/P0254R1. Reviewed as https://reviews.llvm.org/D21459

[Duncan P. N. Exon Smith via cfe-commits]

> On Jul 20, 2016, at 22:31, Marshall Clow via cfe-commits 
>  wrote:
> 
> Modified: libcxx/trunk/include/string
> URL: 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=276238=276237=276238=diff
> ==
> 
> @@ -435,6 +461,7 @@ basic_string operator "" s( co
> */
> 
> #include <__config>
> +#include 

This breaks the following, valid, C++14 code:

#include 
#include 
using namespace std;
using std::experimental::string_view;
void f() { string_view sv; }

Should  #include  even when we're not in C++17 mode?  Why?

> #include 
> #include 
> #include   // For EOF.

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