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

Recursive reflection #58

Merged

Conversation

alexkaratarakis
Copy link
Collaborator

No description provided.


inline constexpr std::string_view NULL_FIELD_TYPE_NAME{};

template <std::size_t MAXIMUM_LAYERS = 32>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make it so that this template parameter can be provided by the client? For example, perhaps we can make RecursionType a variant that supports passing in recursion depth if the client wants recursive reflection info.

Copy link
Collaborator Author

@alexkaratarakis alexkaratarakis Aug 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this was intended to be exposed. Let me consider a nice way to do this, as this type is not meant to be used by the user as it stands.

}
[[nodiscard]] constexpr std::string_view current_providing_base_class() const
{
return inheritance_stack_.empty() ? NULL_FIELD_TYPE_NAME : inheritance_stack_.top();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use a sentinel value instead of just std::optional?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went back and forth on this (and also for the same issue for enclosing_field_name() which is empty for the most external layer). Might go for optional.

Also, inconsistency: https://github.com/teslamotors/fixed-containers/pull/58/files#diff-f67f3de59b958f663c7f020e0cdf5d23c952ad56b4abe200caec3c3f60d9ec47R72
The other instance of this uses a hard-coded "".

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#61

{
using enum reflection_detail::RecursionType;
auto foo = reflection_detail::field_info_of<RECURSIVE, MyColors>();
// std::cout << foo.size() << std::endl;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to leave this commented-out line here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this function is for quickly investigating. I had not committed it until know, but after writing it for the n-th time, I left in a small stub in for the future.

@alexkaratarakis alexkaratarakis merged commit dac260a into teslamotors:main Aug 8, 2023
5 checks passed
@alexkaratarakis alexkaratarakis deleted the recursive_reflection branch August 8, 2023 06:25
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.

2 participants