Extend support for StrEqualityMatcher class and factory functions for wide char strings#4926
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
b2c80fd to
7b84bf4
Compare
|
This PR resolves #4912 |
…Eq/StrCaseNe functions for strings based on the wide characters Extended StringLike to support wide character types Added more tests Restricted possible char types
73e79ba to
c477238
Compare
| *os << (expect_eq ? "is " : "isn't "); | ||
| *os << "equal to "; | ||
| if (!case_sensitive_) { | ||
| if (!CaseSensitive::value) { |
There was a problem hiding this comment.
Could now be constexpr-if if I'm not mistaken.
(a C++17 feature, which should not be a problem)
| template <typename T, | ||
| typename = std::enable_if_t< | ||
| std::is_constructible_v<std::basic_string<char_type_traits_t<T>>, T>>> | ||
| using StringLike = T; |
There was a problem hiding this comment.
If I'm reading it correctly, then you changed the meaning of StringLike.
Before, it would be defined for any type from which you could construct a std::string.
After, it is only defined for c strings, basic_strings and basic_string_views of any character type. This seems to betray the intention of StringLike, you basically could have used std::basic_string_view directly as it, too, supports all three types.
To phrase it differently, suppose my codebase had the following type with a conversion function:
class MyString {
// implementation omitted
operator std::string() const
};
Without having tested this, the compiler in my head says that I can do StrEq(MyString{ "foo" }, "foo") without your change, but I can't anymore with your change. It's thus a breaking change.
You probably need to do something along these lines:
template <typename T, typename CharT, typename = typename std::enable_if_t<
std::is_constructible_v<std::basic_string<CharT>, T>>>
using StringLike = T;
And then use it like so:
template <typename T = std::string,
typename CharT = internal::char_type_traits_t<T>>
PolymorphicMatcher<internal::StrEqualityMatcher<std::basic_string<CharT>>> StrEq(
const internal::StringLike<T, CharT>& str) {
return MakePolymorphicMatcher(
internal::StrEqualityMatcher<std::basic_string<CharT>>(std::basic_string<CharT>(str), true));
}
But as I'm not a maintainer and since there are no tests for such a class like MyString this can or cannot be OK, I simply can't say.
Feature description
Extending support of wide characters for StrEqualityMatcher and factory functions using the class.
Feature request: #4912.
Analysis and design
StrEqualityMatcherand the factory functions using it supportstd::wstring, it's not implemented the same way it is done forstd::string. For instanceStrEqobject forstd::stringcan be created out ofchar *,std::string,std::string_viewthanks to usinginternal::StringLiketemplate alias. Whereas the version forstd::wstringonly supportsconst std::wstring&.Also there absolutely no support for char[8|16|32]_t whatsoever.
* - does not support creation from a
std::basic_string_view<>Solution description
The solution proposed generalizes the
std::stringversion of the class and functions for the whole variety of the character types.Factory functions
HasSubstr,StartsWith,EndsWithare also extended (atm lacking the tests).* - Case insensitive comparison is not provided by the standard library, thus is not implemented here.
Areas affected
Public interface - factory functions (e.g.
StrEq) are backward compatible.Extended
StringLikewhich is an internal type has been extended to include wide characters.StrEqualityMatcherclass (also an internal entity) has been changed the following way:case_sensitivewas removed in favour of the template parameter. This was done in order to avoid calling case insensitive comparison functions for the character types that don't support such a comparison (char[8|16|32]_t).