-
Notifications
You must be signed in to change notification settings - Fork 58
string constructors and conversions are implicit #649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
string constructors and conversions are implicit #649
Conversation
|
GCOVR code coverage report https://649.url.prtest.cppalliance.org/gcovr/index.html |
3579b27 to
ad7a93a
Compare
|
GCOVR code coverage report https://649.url.prtest.cppalliance.org/gcovr/index.html |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #649 +/- ##
===========================================
- Coverage 96.88% 96.87% -0.02%
===========================================
Files 147 147
Lines 7924 7929 +5
===========================================
+ Hits 7677 7681 +4
- Misses 247 248 +1
Continue to review full report at Codecov.
|
|
@klemens-morgenstern is this ok? |
| #endif | ||
| > | ||
| url_view( | ||
| String const& s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now you have two overloads which are the same no? string_view is convertible to string_view. Does this compile?
string_view s = "";
url_view u( s );
Isn't this ambiguous?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works fine. It's exactly the same pattern as pct_string_view:
- the compiler picks up the
string_viewoverload because it's not templated - the compiler picks up the
Stringoverload when it's convertible tostring_viewbut notstring_view
If all things were directly convertible to string_view, we would just need the first overload. But we have two implicit conversion steps sometimes.
I included your test to ensure that's OK. The test is somewhat redundant because most other tests already construct these url_views from string_views, but that's not explicit..
| // url_view(string_view) no ambiguous | ||
| { | ||
| string_view s = ""; | ||
| BOOST_TEST_NO_THROW(url_view( s )); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the template overload has a lower priority than the string_view overload?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes :)
|
GCOVR code coverage report https://649.url.prtest.cppalliance.org/gcovr/index.html |
|
I would have thought a ctor from stringview is eny, but I will trust your judgement. looks good. |
|
GCOVR code coverage report https://649.url.prtest.cppalliance.org/gcovr/index.html |
fix #648, #650