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

Re-add Boost.Array where it is ODR-used #64

Closed
wants to merge 1 commit into from

Conversation

13steinj
Copy link

@13steinj 13steinj commented Dec 31, 2023

Determining the size of a type requires its definition; of which there is a "not-C++11-compliant" use and a C++11-agnostic use in .../converter_lexical_streams.hpp.

This was mistakenly forward-declared as part of af5ce2a.

Specifically, sizeof usages here:

In case I picked usages that aren't in develop's head (as ctrl-f'ing that file shows more usages than I expect), here's current usage as well:

// Adding constness to characters. Constness does not change layout
template <class C, std::size_t N>
typename boost::disable_if<boost::is_const<C>, bool>::type
operator<<(boost::array<C, N> const& input) noexcept {
static_assert(
sizeof(boost::array<const C, N>) == sizeof(boost::array<C, N>),
"boost::array<C, N> and boost::array<const C, N> must have exactly the same layout."
);
return ((*this) << reinterpret_cast<boost::array<const C, N> const& >(input));
}

#ifndef BOOST_NO_CXX11_HDR_ARRAY
// Making a Boost.Array from std::array
template <class C, std::size_t N>
bool operator<<(std::array<C, N> const& input) noexcept {
static_assert(
sizeof(std::array<C, N>) == sizeof(boost::array<C, N>),
"std::array and boost::array must have exactly the same layout. "
"Bug in implementation of std::array or boost::array."
);
return ((*this) << reinterpret_cast<boost::array<C, N> const& >(input));
}
#endif
template <class InStreamable>

Edit: changed GitHub /blame/ -> /blob/ since GitHub renders /blob/ nicely in comments/PRs.

Interestingly enough; in the "non-C++11-compliant" case the ifndef / function was not removed; so the right answer might be to remove that function to drop down to a single usage and then remove that usage or put it as part of some static-compile test instead of adding arrays back. I don't remember if a reinterpret cast is an odr-use.

Determining the size of a type requires its definition; of which there
is a "not-C++11-compliant" use and a C++11-agnostic use in `.../converter_lexical_streams.hpp`.

This was mistakenly forward-declared as part of af5ce2a.
@apolukhin
Copy link
Member

Thanks for the PR and issue report!

The whole reinterpret_casting of arrays has a bad smell. Rewriting that part of the library in #68

apolukhin added a commit that referenced this pull request Jan 21, 2024
@apolukhin
Copy link
Member

Fixed in 4bf37fb

Thanks again for the report!

@apolukhin apolukhin closed this Jan 21, 2024
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.

None yet

2 participants