[struct_pack] better-read and develop,make code clean

[sunflower-knight]

What happened + What you expected to happen

After reading struct_pack source code , I think threre some code need to be changed , to help readers and developers understande it better .

Reproduction way

I will list what I think below

Anything else

developers can read it and discuss together.
I have many question and suggestion for struct_pack, this issue will not close by me before i write down all of them

Are you willing to submit a PR?

• Yes I am willing to submit a PR!

[sunflower-knight]

• struct_pack/reflection : span concept
  Before

  template <typename Type>
  concept dynamic_span = span<Type> && std::is_same_v<std::integral_constant<std::size_t,Type::extent>,
std::integral_constant<std::size_t,SIZE_MAX>>;

After

template<typename Type>
concept dynamic_span = requires
{
  span<Type>;
  std::is_same_v<std::integral_constant<std::size_t, Type::extent>,
                 std::integral_constant<std::size_t, std::dynamic_extent>>;
};

Comment
use std::dynamic_extent rather than SIZE_T_MAX

1. dynamic_extent is very easy to understand it is a dynamic.
2. if you use IDE ,when u click dynamic_extent will jump to the span dynamic_extent definition have a better read
3. try to not use macro
4. dynamic_extent is the initional definition for dynamic span . u can see it it cppreference https://en.cppreference.com/w/cpp/container/span and https://en.cppreference.com/w/cpp/container/span/dynamic_extent
   I think dynamic_extent ( std initial definition ) >>
   std::numeric_limits<std::size_t>::max()（Clang stl ） >>
   std::size_t dynamic_extent = -1 (GCC stl) >>
   SIZE_T_MAX (MACRO after you read artical and understand span )
   ps: >> means better

[sunflower-knight]

• struct_pack/reflection : span concept
  use Contiguous rather than continuous_container

template<typename Type>
concept continuous_container =
string <Type> || (container <Type> && requires(
Type container
) {
std::span{
container};
});

Because most of the developers comes from China, here is a reasons in Chinese
"contiguous" 和 "continuous" 是两个相关但有不同含义的单词。
"Contiguous"（连续的）：这个词用于描述物体、区域或元素在空间或位置上相互接触、相邻或紧密排列。在计算机科学中，"contiguous" 经常用于描述内存中连续分配的块，即相邻的内存地址。对于容器而言，"contiguous" 意味着容器的元素在内存中是连续存储的。
"Continuous"（连续的）：这个词用于描述时间上的持续性，表示没有间断或中断的状态。它强调了一个事件、过程或属性的持续性或连续性。在计算机科学中，"continuous" 通常与连续计算或连续数据流等概念相关，表示数据或计算是实时、持续不断地进行的。

[sunflower-knight]

struct_pack/reflection : varint concept

-------------------------------------------------------------old ------------------------------------
This code want to find out whether the Type T is std::varint .

template <typename Type>
constexpr inline bool is_variant_v = false;

template <typename... args>
constexpr inline bool is_variant_v<std::variant<args...>> = true;

template <typename T>
concept variant = is_variant_v<T>;

In cpp 17 , you can use variant_size_v to find how many types in varint , so you can know whether the Type T is std::varint

template <typename T>
concept variant = std::variant_size_v<T> >= 0; // use variant_size_v check whether the  Type T is std::varint

it will be easy and clean .
Additional it will be better to add comment there.

----------------------------------------------------------- new -------------------------------------

#include <iostream>
#include <variant>
struct Person {};
template <typename T>
concept is_variant_variant_size_value = requires {
  std::variant_size<T>::value; 
};
template <typename T>
concept is_variant_variant_size_v = requires {
  std::variant_size_v<T> ;
};
std::variant<int> ans;
int main() {
  std::cout << std::boolalpha;

  std::cout << is_variant_variant_size_value<decltype(ans)> << std::endl;
  std::cout << is_variant_variant_size_value<Person> << std::endl;
  std::cout << is_variant_variant_size_value<int> << std::endl;

  std::cout << is_variant_variant_size_v<decltype(ans)> << std::endl;
  //  std::cout << is_variant_variant_size_v<Person> << std::endl; //error
  //  std::cout <<  is_variant_variant_size_v<int> << std::endl;   //error

  return 0;
}

after test ，I know std::variant_size::value; is the correct method to do this .
If you use std::variant_size_v , when you give a type in concept, code will report a compile error .
If you are interesting, just copy the code above and run.

[yanminhui]

• struct_pack/reflection : span concept
  Before

  template <typename Type>
  concept dynamic_span = span<Type> && std::is_same_v<std::integral_constant<std::size_t,Type::extent>,
std::integral_constant<std::size_t,SIZE_MAX>>;

After

template<typename Type>
concept dynamic_span = requires
{
  span<Type>;
  std::is_same_v<std::integral_constant<std::size_t, Type::extent>,
                 std::integral_constant<std::size_t, std::dynamic_extent>>;
};

Comment use std::dynamic_extent rather than SIZE_T_MAX

1. dynamic_extent is very easy to understand it is a dynamic.
2. if you use IDE ,when u click dynamic_extent will jump to the span dynamic_extent definition have a better read
3. try to not use macro
4. dynamic_extent is the initional definition for dynamic span . u can see it it cppreference https://en.cppreference.com/w/cpp/container/span and https://en.cppreference.com/w/cpp/container/span/dynamic_extent
   I think dynamic_extent ( std initial definition ) >>
   std::numeric_limits<std::size_t>::max()（Clang stl ） >>
   std::size_t dynamic_extent = -1 (GCC stl) >>
   SIZE_T_MAX (MACRO after you read artical and understand span )
   ps: >> means better

simplify:

template <class T>
concept dynamic_span = span<Type> && T::extent == std::dynamic_extent;

[sunflower-knight]

template
concept dynamic_span = span && T::extent == std::dynamic_extent;

Here is the code to compare two integer value.
But dynamic_extent is size_t , if user's span, for example my_span, my_span::etent is int , the type check will not report a error when you compile your code .

T::extent == std::dynamic_extent; only check the value , but not check type.
I think check value and type together is better.

• struct_pack/reflection : span concept
  Before

  template <typename Type>
  concept dynamic_span = span<Type> && std::is_same_v<std::integral_constant<std::size_t,Type::extent>,
std::integral_constant<std::size_t,SIZE_MAX>>;

After

template<typename Type>
concept dynamic_span = requires
{
  span<Type>;
  std::is_same_v<std::integral_constant<std::size_t, Type::extent>,
                 std::integral_constant<std::size_t, std::dynamic_extent>>;
};

Comment use std::dynamic_extent rather than SIZE_T_MAX

1. dynamic_extent is very easy to understand it is a dynamic.
2. if you use IDE ,when u click dynamic_extent will jump to the span dynamic_extent definition have a better read
3. try to not use macro
4. dynamic_extent is the initional definition for dynamic span . u can see it it cppreference https://en.cppreference.com/w/cpp/container/span and https://en.cppreference.com/w/cpp/container/span/dynamic_extent
   I think dynamic_extent ( std initial definition ) >>
   std::numeric_limits<std::size_t>::max()（Clang stl ） >>
   std::size_t dynamic_extent = -1 (GCC stl) >>
   SIZE_T_MAX (MACRO after you read artical and understand span )
   ps: >> means better

simplify:

template <class T>
concept dynamic_span = span<Type> && T::extent == std::dynamic_extent;

[yanminhui]

template concept dynamic_span = span && T::extent == std::dynamic_extent;

Here is the code to compare two integer value. But dynamic_extent is size_t , if user's span, for example my_span, my_span::etent is int , the type check will not report a error when you compile your code .

T::extent == std::dynamic_extent; only check the value , but not check type. I think check value and type together is better.

• struct_pack/reflection : span concept
  Before

  template <typename Type>
  concept dynamic_span = span<Type> && std::is_same_v<std::integral_constant<std::size_t,Type::extent>,
std::integral_constant<std::size_t,SIZE_MAX>>;

After

template<typename Type>
concept dynamic_span = requires
{
  span<Type>;
  std::is_same_v<std::integral_constant<std::size_t, Type::extent>,
                 std::integral_constant<std::size_t, std::dynamic_extent>>;
};

Comment use std::dynamic_extent rather than SIZE_T_MAX

1. dynamic_extent is very easy to understand it is a dynamic.
2. if you use IDE ,when u click dynamic_extent will jump to the span dynamic_extent definition have a better read
3. try to not use macro
4. dynamic_extent is the initional definition for dynamic span . u can see it it cppreference https://en.cppreference.com/w/cpp/container/span and https://en.cppreference.com/w/cpp/container/span/dynamic_extent
   I think dynamic_extent ( std initial definition ) >>
   std::numeric_limits<std::size_t>::max()（Clang stl ） >>
   std::size_t dynamic_extent = -1 (GCC stl) >>
   SIZE_T_MAX (MACRO after you read artical and understand span )
   ps: >> means better

simplify:

template <class T>
concept dynamic_span = span<Type> && T::extent == std::dynamic_extent;

Put type trait in span concept instead of dynamic_span if want to check the type of extent.

[sunflower-knight]

struct_pack/reflection : aggregate class

• better static assert message
  When you serizlize a struct with private compile will have error below .
  

In yalanting website, it should tell users to use aggregate class.

• better weisite doc

1. Chinese API doc is not the same as English version , update Chinese version API or
2. update the important info in sourcecode such as 'limition' in Website docs

[poor-circle]

• struct_pack/reflection : span concept
  use Contiguous rather than continuous_container

template<typename Type>
concept continuous_container =
string <Type> || (container <Type> && requires(
Type container
) {
std::span{
container};
});

Because most of the developers comes from China, here is a reasons in Chinese "contiguous" 和 "continuous" 是两个相关但有不同含义的单词。 "Contiguous"（连续的）：这个词用于描述物体、区域或元素在空间或位置上相互接触、相邻或紧密排列。在计算机科学中，"contiguous" 经常用于描述内存中连续分配的块，即相邻的内存地址。对于容器而言，"contiguous" 意味着容器的元素在内存中是连续存储的。 "Continuous"（连续的）：这个词用于描述时间上的持续性，表示没有间断或中断的状态。它强调了一个事件、过程或属性的持续性或连续性。在计算机科学中，"continuous" 通常与连续计算或连续数据流等概念相关，表示数据或计算是实时、持续不断地进行的。

菜英文233

[poor-circle]

struct_pack/reflection : aggregate class

• better static assert message
  When you serizlize a struct with private compile will have error below .
  

In yalanting website, it should tell users to use aggregate class.

• better weisite doc

1. Chinese API doc is not the same as English version , update Chinese version API or
2. update the important info in sourcecode such as 'limition' in Website docs

See #278
We relax some limitation for non-aggregate type.
But you are right, we should improve the static assert message.

[sunflower-knight]

• struct_pack/reflection : std::size_t{} is useless and too long

#include <iostream>

constexpr std::size_t size_t_v{};//会调用到不同的类型 int32 int64
constexpr float float_v{};
constexpr double double_v{};

class MyPerson
{
 public:
	MyPerson() = default;
	void resize(std::size_t index)
	{
	}
};

template<typename T>
concept size_t_param =
requires(T string)
{
	string.resize(0);
	string.resize(size_t_v);
	string.resize(float_v);
	string.resize(double_v);
};
template<typename T>
requires size_t_param<T>
class Chindren
{
};

int main() {
	std::cout << std::boolalpha;
	Chindren<MyPerson> Tom ;
	return 0;
}
//判断是否有resize接口，可以传入许多不同的参数
//判断有resize接口 且 接口类型为 sizet类型的参数在模板编程中似乎是做不到的

To determine if there is a resize interface , you can use various types of arguments
But it seems impossible to determine in template programming whether there is a resize interface with a parameter of sizet type.

So I think use constexpt std::size_t size_t_v{}; and use size_t_v in everywhere we use std::size_t{} is better.

[sunflower-knight]

template concept dynamic_span = span && T::extent == std::dynamic_extent;
Here is the code to compare two integer value. But dynamic_extent is size_t , if user's span, for example my_span, my_span::etent is int , the type check will not report a error when you compile your code .
T::extent == std::dynamic_extent; only check the value , but not check type. I think check value and type together is better.

• struct_pack/reflection : span concept
  Before

  template <typename Type>
  concept dynamic_span = span<Type> && std::is_same_v<std::integral_constant<std::size_t,Type::extent>,
std::integral_constant<std::size_t,SIZE_MAX>>;

After

template<typename Type>
concept dynamic_span = requires
{
  span<Type>;
  std::is_same_v<std::integral_constant<std::size_t, Type::extent>,
                 std::integral_constant<std::size_t, std::dynamic_extent>>;
};

Comment use std::dynamic_extent rather than SIZE_T_MAX

1. dynamic_extent is very easy to understand it is a dynamic.
2. if you use IDE ,when u click dynamic_extent will jump to the span dynamic_extent definition have a better read
3. try to not use macro
4. dynamic_extent is the initional definition for dynamic span . u can see it it cppreference https://en.cppreference.com/w/cpp/container/span and https://en.cppreference.com/w/cpp/container/span/dynamic_extent
   I think dynamic_extent ( std initial definition ) >>
   std::numeric_limits<std::size_t>::max()（Clang stl ） >>
   std::size_t dynamic_extent = -1 (GCC stl) >>
   SIZE_T_MAX (MACRO after you read artical and understand span )
   ps: >> means better

simplify:

template <class T>
concept dynamic_span = span<Type> && T::extent == std::dynamic_extent;

Put type trait in span concept instead of dynamic_span if want to check the type of extent.

I think your opinion is better. resons :

1. put extent in concept span ,code will be clean .

template <typename Type>
concept span = container < Type> && requires(Type t) {
t = Type{(typename Type::value_type *)nullptr, std::size_t{}};
t.subspan(std::size_t{}, std::size_t{});
};

template <typename Type>
concept dynamic_span = span<Type> && std::is_same_v<std::integral_constant<std::size_t, Type::extent>, std::integral_constant<std::size_t, SIZE_MAX>>;

template <typename Type>
concept static_span = span<Type> && !dynamic_span<Type>;

2. currently, in the yalanting'c code , static span should satisfy concept span but should not satisfy dynamic span . In this way ,code will not check whether satatic span has a menber(extent)

I think it is a logical bug in yalanting now . Thank you for inspiring me and your comment !

I want to change the code like this , based on the results of our discussion

#include "span"

constexpr std::size_t size_t_v{};
template <typename Type>
concept span = container<Type> && requires(Type t) {
  // static size_t member : extent
  std::integral_constant<std::size_t, Type::extent>{};
  t = Type{(typename Type::value_type *)nullptr, size_t_v};
  t.subspan(size_t_v, size_t_v);
};

template <typename Type>
concept dynamic_span = span<Type> && Type::extent == std::dynamic_extent;

template <typename Type>
concept static_span = span<Type> && !dynamic_span<Type>;

[sunflower-knight]

• struct_pack/reflection : pair -> support stl::pair only

You can see in the yalanting website or doc . Currently, yalanting only support std::pair.
std::pair is one of the yalanting's Struct in yalanting type system.
Maybe we should not support the pair from other lib . such as folly::pair boost::pair ...................
We dont know how these pair implementation .
Actually I dont know whether there is a pair in folly, boost .
Anyway I have a new method to support it for std::pair only .

template<typename Type>
concept pair = std::is_same_v<std::remove_cvref_t<Type>,
				                 std::pair<typename std::remove_cvref_t<Type>::first_type,
					                        typename std::remove_cvref_t<Type>::second_type >>;

Here is the code for test

#include <iostream>
#include <utility>

template<typename Type>
concept pair = std::is_same_v<std::remove_cvref_t<Type>,
				                 std::pair<typename std::remove_cvref_t<Type>::first_type,
					                        typename std::remove_cvref_t<Type>::second_type >>;

template<typename T1, typename T2>
struct MyPair
{
	typedef T1 first_type;  ///< The type of the `first` member
	typedef T2 second_type; ///< The type of the `second` member

	T1 first;  ///< The first member
	T2 second; ///< The second member
};

int main()
{
	std::pair<int, double> p(1, 2.5);
	std::cout << std::boolalpha << pair<decltype(p)> << std::endl; // cout: true

	std::pair<int, double> &  pref = p ;
	std::cout << std::boolalpha << pair<decltype(pref)> << std::endl; // cout: true

	const std::pair<int, double> &  pref_const = p ;
	std::cout << std::boolalpha << pair<decltype(pref_const)> << std::endl; // cout: true

	const std::pair<int, double>   p_const = p ;
	std::cout << std::boolalpha << pair<decltype(p_const)> << std::endl; // cout: true

	MyPair<int, double> mp{ 1, 2.5 };
	std::cout << std::boolalpha << pair<decltype(mp)> << std::endl; // cout: false

	return 0;
}

I dont know too much about how the standard std::pair work , so please comment ,
How do you think of my code ?
And whether it is necessary to use remove_cvref_t in the new concept? Should we use remove_cvref_t to make const reference normal pair satisify the concept together ?

ps : If we dont use remove_cvref_t , only pair<decltype(p)> is true.

By the way ,

I am inspired by gcc-12 stl in c++/12/bits/stl_pair.h .
because __is_pair is not a standard , we can't use it directly.

  template<typename _Tp>
    inline constexpr bool __is_pair = false;

  template<typename _Tp, typename _Up>
    inline constexpr bool __is_pair<pair<_Tp, _Up>> = true;

  template<typename _Tp, typename _Up>
    inline constexpr bool __is_pair<const pair<_Tp, _Up>> = true;

• update website and doc .
  If you think yalanting should support other pair_like struct after consideration , all my code will be useless,
  but it is time to update our doc and website.

[sunflower-knight]

• struct_pack/reflection : expected -> UB

[poor-circle]

• struct_pack/reflection : pair -> support stl::pair only

You can see in the yalanting website or doc . Currently, yalanting only support std::pair. std::pair is one of the yalanting's Struct in yalanting type system. Maybe we should not support the pair from other lib . such as folly::pair boost::pair ................... We dont know how these pair implementation . Actually I dont know whether there is a pair in folly, boost . Anyway I have a new method to support it for std::pair only .

template<typename Type>
concept pair = std::is_same_v<std::remove_cvref_t<Type>,
				                 std::pair<typename std::remove_cvref_t<Type>::first_type,
					                        typename std::remove_cvref_t<Type>::second_type >>;

Here is the code for test

#include <iostream>
#include <utility>

template<typename Type>
concept pair = std::is_same_v<std::remove_cvref_t<Type>,
				                 std::pair<typename std::remove_cvref_t<Type>::first_type,
					                        typename std::remove_cvref_t<Type>::second_type >>;

template<typename T1, typename T2>
struct MyPair
{
	typedef T1 first_type;  ///< The type of the `first` member
	typedef T2 second_type; ///< The type of the `second` member

	T1 first;  ///< The first member
	T2 second; ///< The second member
};

int main()
{
	std::pair<int, double> p(1, 2.5);
	std::cout << std::boolalpha << pair<decltype(p)> << std::endl; // cout: true

	std::pair<int, double> &  pref = p ;
	std::cout << std::boolalpha << pair<decltype(pref)> << std::endl; // cout: true

	const std::pair<int, double> &  pref_const = p ;
	std::cout << std::boolalpha << pair<decltype(pref_const)> << std::endl; // cout: true

	const std::pair<int, double>   p_const = p ;
	std::cout << std::boolalpha << pair<decltype(p_const)> << std::endl; // cout: true

	MyPair<int, double> mp{ 1, 2.5 };
	std::cout << std::boolalpha << pair<decltype(mp)> << std::endl; // cout: false

	return 0;
}

I dont know too much about how the standard std::pair work , so please comment , How do you think of my code ? And whether it is necessary to use remove_cvref_t in the new concept? Should we use remove_cvref_t to make const reference normal pair satisify the concept together ?

ps : If we dont use remove_cvref_t , only pair<decltype(p)> is true.

By the way ,

I am inspired by gcc-12 stl in c++/12/bits/stl_pair.h . because __is_pair is not a standard , we can't use it directly.

  template<typename _Tp>
    inline constexpr bool __is_pair = false;

  template<typename _Tp, typename _Up>
    inline constexpr bool __is_pair<pair<_Tp, _Up>> = true;

  template<typename _Tp, typename _Up>
    inline constexpr bool __is_pair<const pair<_Tp, _Up>> = true;

• update website and doc .
  If you think yalanting should support other pair_like struct after consideration , all my code will be useless,
  but it is time to update our doc and website.

You are right. We should only support std::pair because struct_pack will try to memcpy it when the first_type & seconde_type are all trivial copyable. It may not work in third-party pair-like type.

[yanminhui]

• struct_pack/reflection : std::size_t{} is useless and too long

#include <iostream>

constexpr std::size_t size_t_v{};//会调用到不同的类型 int32 int64
constexpr float float_v{};
constexpr double double_v{};

class MyPerson
{
 public:
	MyPerson() = default;
	void resize(std::size_t index)
	{
	}
};

template<typename T>
concept size_t_param =
requires(T string)
{
	string.resize(0);
	string.resize(size_t_v);
	string.resize(float_v);
	string.resize(double_v);
};
template<typename T>
requires size_t_param<T>
class Chindren
{
};

int main() {
	std::cout << std::boolalpha;
	Chindren<MyPerson> Tom ;
	return 0;
}
//判断是否有resize接口，可以传入许多不同的参数
//判断有resize接口 且 接口类型为 sizet类型的参数在模板编程中似乎是做不到的

To determine if there is a resize interface , you can use various types of arguments But it seems impossible to determine in template programming whether there is a resize interface with a parameter of sizet type.

So I think use constexpt std::size_t size_t_v{}; and use size_t_v in everywhere we use std::size_t{} is better.

check signature if you want to check the type of parameter really.

template<typename T>
concept size_t_param = requires(T obj) {
    obj.resize(std::size_t{});
    { &T::resize } -> std::same_as<void(T::*)(std::size_t)>;
};