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

std::search called in search() when compiling with C++14 #50

Open
eyalroz opened this issue May 27, 2022 · 9 comments
Open

std::search called in search() when compiling with C++14 #50

eyalroz opened this issue May 27, 2022 · 9 comments

Comments

@eyalroz
Copy link

eyalroz commented May 27, 2022

I'm trying to compile one of the examples of my cuda-api-wrappers library, which uses string-view-lite, using C++14 instead of C++11 like I was compiling it so far. I get:

... snip ...
/home/eyalroz/src/mine/cuda-api-wrappers/examples/other/jitify/string_view.hpp:572:23: error: call to non-‘constexpr’ function ‘_FIter1 std::search(_FIter1, _FIter1, _FIter2, _FIter2) [with _FIter1 = const char*; _FIter2 = const char*]’
  572 |     return std::search( haystack.begin(), haystack.end(), needle.begin(), needle.end() );
      |            ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
... snip...

and, indead, std::search is not constexpr before C++20.

@eyalroz eyalroz changed the title std::search called in search() std::search called in search() when compiling with C++14 May 27, 2022
@eyalroz
Copy link
Author

eyalroz commented May 27, 2022

Oh, actually, maybe it's a dupe of my other bug.

@oliverlee
Copy link

You can try defining __OPTIMIZE__ before including string_view.hpp or build with optimizations enabled.

#if defined(__OPTIMIZE__)
// gcc, clang provide __OPTIMIZE__
// Expect tail call optimization to make search() non-recursive:
template< class CharT, class Traits = std::char_traits<CharT> >
constexpr const CharT* search( basic_string_view<CharT, Traits> haystack, basic_string_view<CharT, Traits> needle )
{
return haystack.starts_with( needle ) ? haystack.begin() :
haystack.empty() ? haystack.end() : search( haystack.substr(1), needle );
}
#else // OPTIMIZE
// non-recursive:
template< class CharT, class Traits = std::char_traits<CharT> >
constexpr const CharT* search( basic_string_view<CharT, Traits> haystack, basic_string_view<CharT, Traits> needle )
{
return std::search( haystack.begin(), haystack.end(), needle.begin(), needle.end() );
}
#endif // OPTIMIZE

https://stackoverflow.com/questions/14618249/is-there-a-builtin-macro-defined-when-optimisation-is-enabled-in-clang

@martinmoene
Copy link
Owner

@eyalroz, @oliverlee

The unconditional constexpr in the non-optimize case of constexpr const CharT* search() should perhaps be replaced with something like nssv_constexpr20_lib.

I'd like nssv_HAVE_CONSTEXPR_20_LIB to be not overly restrictive as it may be as show below. Suggestions are quite welcome.

// Presence of C++20 library features:

#define nssv_HAVE_CONSTEXPR_20_LIB  nssv_CPP20_000

// ...

#if  nssv_HAVE_CONSTEXPR_20_LIB
# define nssv_constexpr20_lib  constexpr
#else
# define nssv_constexpr20_lib  /*constexpr*/
#endif

// ...

#else // OPTIMIZE

// non-recursive:

template< class CharT, class Traits = std::char_traits<CharT> >
nssv_constexpr20_lib const CharT* search( basic_string_view<CharT, Traits> haystack, basic_string_view<CharT, Traits> needle )
{
    return std::search( haystack.begin(), haystack.end(), needle.begin(), needle.end() );
}

#endif // OPTIMIZE

@oliverlee
Copy link

I think it makes sense to remove the branch on _OPTIMIZE_. By removing the call to std::search, this would allow use in constant expressions with debug builds.

@martinmoene
Copy link
Owner

However, a possibly recursive search, possibly exhausting the stack doesn't look like the right thing to do at default.

@oliverlee
Copy link

Ah right. In that case, what about replacing the second implementation with something like:

template< class CharT, class Traits = std::char_traits<CharT> >
nssv_constexpr14 const CharT* search( basic_string_view<CharT, Traits> haystack, basic_string_view<CharT, Traits> needle )
{
    while (needle.size() <= haystack.size()) {
        if (haystack.starts_with(needle)) {
            return haystack.cbegin();
        }

        haystack = basic_string_view<CharT, Traits>{ haystack.begin() + 1, haystack.size() - 1U };
    }

    return haystack.cend();
}

@oliverlee
Copy link

Although I suppose it would make sense to allow a user to select std::search via a preprocessor macro (or vice-versa).

@martinmoene
Copy link
Owner

@oliverlee Thanks for your suggestions!

@oliverlee
Copy link

Thanks for sharing your implementation of string view!

martinmoene added a commit that referenced this issue Dec 29, 2023
@oliverlee)

Allow to avoid constexpr with std::search() and select a local implementation for it for C++14 constexpr.
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

No branches or pull requests

3 participants