Skip to content
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

sfinae-protect operator== StringRef overload #3132

Merged

Conversation

vhavel
Copy link

@vhavel vhavel commented Mar 15, 2022

We encountered a c++ compilation issue when using p4c together with inja (https://github.com/pantor/inja) in one project. Inja headers define:

  template <class CharT, class Traits nssv_MSVC_ORDER(1)>
  nssv_constexpr bool operator==(basic_string_view<CharT, Traits> lhs,
                                 nssv_BASIC_STRING_VIEW_I(CharT, Traits) rhs) nssv_noexcept {

which is supposed to be called when basic_string_view<> is compared to const char*. But if p4c sources are also included, such call is ambiguous, because operator==(T, StringRef) also participates in overload resolution:

inja/include/inja/parser.hpp:207:31: error: ambiguous overload for ‘operator==’ (operand types are ‘nonstd::sv_lite::string_view {aka nonstd::sv_lite::basic_string_view<char>}’ and ‘const char [3]’)
           } else if (tok.text == "in") {
                      ~~~~~~~~~^~~~~~~

p4c/lib/stringref.h:145:10: note: candidate: bool operator==(T, const StringRef&) [with T = nonstd::sv_lite::basic_string_view<char>]
     bool operator==(T a, const StringRef &b) { return b == a; }
          ^~~~~~~~

inja/include/inja/string_view.hpp:1088:18: note: candidate: constexpr bool nonstd::sv_lite::operator==(nonstd::sv_lite::basic_string_view<CharT, Traits>, typename std::decay<nonstd::sv_lite::basic_string_view<CharT, Traits> >::type) [with CharT = char; Traits = std::char_traits<char>; typename std::decay<nonstd::sv_lite::basic_string_view<CharT, Traits> >::type = nonstd::sv_lite::basic_string_view<char>]
   nssv_constexpr bool operator==(basic_string_view<CharT, Traits> lhs,
                  ^~~~~~~~

This PR fixes this problem by employing SFINAE to enable operator==(T a, StringRef b) only if a == b is a valid expression.

@mihaibudiu
Copy link
Contributor

Frankly I don't know enough C++ to judge whether this is an improvement, but since the tests pass I will mark this as accepted. Ideally @ChrisDodd should review this, but he's been busy lately.

@mihaibudiu mihaibudiu requested a review from ChrisDodd March 15, 2022 17:59
Unrestricted operator==(T, StringRef) participates in overload
resolution even if StringRef::operator== cannot be called with T.
@vhavel vhavel force-pushed the vh/fix_stringref_operator_overloading branch from 592349e to 3f02bcc Compare March 17, 2022 16:25
@mihaibudiu mihaibudiu merged commit ece99c0 into p4lang:main Mar 17, 2022
@vhavel vhavel deleted the vh/fix_stringref_operator_overloading branch March 18, 2022 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants