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

Add static_asserts for template parameters of basic_fixed_string #30

Open
unterumarmung opened this issue Aug 17, 2021 · 5 comments
Open
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@unterumarmung
Copy link
Owner

Example from implementation of basic_string_view by Microsoft: https://github.com/microsoft/STL/blob/main/stl/inc/xstring#L1224-L1230

@unterumarmung unterumarmung added enhancement New feature or request good first issue Good for newcomers labels Aug 17, 2021
@DBJDBJ
Copy link

DBJDBJ commented Aug 17, 2021

static_assert() inside a template controls only the instatiation of the type definition of the template. Not the mechanism of template type definition.

template <typename T>
struct not_a_vector {
  static_assert(std::is_trivially_copyable_v<T>,
                "\n\nnot_a_vector<>T -- can handle only trivially "
                "copiable types!\n\n");
} ;

// no warning even if -Wall is used
   using should_not_compile_but_it_does
               = not_a_vector<std::string>  ;

Consider instead controling the type definition.

template <typename T,
std::enable_if_t< std::is_trivially_copyable_v<T>, int > = 0
>
class better_not_a_vector {
} ;

using should_not_compile_and_it_does_not =
               better_not_a_vector<std::string>;

using should_compile_and_it_does =
               better_not_a_vector<bool>;

better_not_a_vector<T> can not be promoted to type definition if the author's requirement is not satisfied.

Gobolt here

@unterumarmung
Copy link
Owner Author

I don't consider this as an issue. It will be a compile-time error anyway, when the type alias is used.
I don't see any harm in postponing the compile-time error.

@DBJDBJ
Copy link

DBJDBJ commented Aug 18, 2021

if not_a_vector is in the library not owned by you (it might be owned by MS STL for example) that will go unnoticed, and that might be a problem.

For example:

// this is inside dudkinlib.h
#include <not_a_vector> // not owned by dudkin ltd
// no warning even if -Wall is used
namespace dudkin {
   using  vector_of_strings // should_not_compile_but_it_does
               = not_a_vector<std::string>  ;
}

It might happen this will go unnoticed for some months, and then it will be noticed by some important customer.

// DUDKINLIB paying customer code, few months later
#include <dudkinlib.h>

dudkin::vector_of_strings    customer_ ; // does not compile

What is the solution? not_a_vector vendor has this philosophy called "I don't see any harm in postponing the compile-time error", and consequently does not (or can not) change not_a_vector. Customer has paid for dudkinlib and demands a solution from dudkin ltd.

@DBJDBJ
Copy link

DBJDBJ commented Aug 18, 2021

Another example from

https://github.com/unterumarmung/hyper_log_log/blob/master/hll/hyper_log_log.hxx
template<typename T, std::size_t k>
class hyper_log_log
{
public:
    static_assert(k >= 4 && k <= 30, "k must be in a range [4; 30]");
} ; 

using timebomb_type = hyper_log_log<bool, 1> ;

// few months (or years) after:
// timebomb_type boom ;

Just a forward declared type constrained template is enough to stop wrong type definitions.

// forward declaration of a type constrained template
template <typename T, std::size_t k,
          std::enable_if_t<k >= 4 && k <= 30, int> = 0>
class better_hyper_log_log ;

User (or you) can not introduce types that will not compile few months or years down the line

// does not compile
// using impossible_type = better_hyper_log_log<bool, 1> ;

using ok_type = better_hyper_log_log<bool, 13> ;

This peculiarity is what one might describe as "C++ foot gun with a timer" ...

@DBJDBJ
Copy link

DBJDBJ commented Aug 20, 2021

Actually, this is the correct solution

template< typename T, std::size_t k>
#if __cplusplus >= 202002L
    // C++20 (and later) code
requires // requires-clause (ad-hoc constraint)
(k >= 4 && k <= 30) 
#endif
class constrained_hyper_log_log ;

using constrained_type = constrained_hyper_log_log<bool, 13> ;

Constrained types are not constrained instantiation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants