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