Skip to content

Conversation

@joaquintides
Copy link
Member

@joaquintides joaquintides commented May 26, 2025

This is part 1 of the process of migrating hash_is_avalanching from Boost.Unordered to Boost.ContainerHash; part 2 will redefine boost::unordered::hash_is_avalanching as a using declaration of boost::container_hash::hash_is_avalanching and deprecate <boost/unordered/hash_traits.hpp>.

BOOST_TEST_TRAIT_FALSE(( hash_is_avalanching< boost::hash<std::basic_string<my_char> > > ));
#endif

BOOST_TEST_TRAIT_FALSE(( hash_is_avalanching< std::hash<std::string > > ));
Copy link
Member

Choose a reason for hiding this comment

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

Why are you testing this?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's an example of a struct without a nested is_avalanching.

Copy link
Member

Choose a reason for hiding this comment

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

Please use a struct without a nested is_avalanching, then.

I don't want to guarantee that hash_is_avalanching is false for std::hash<std::string>.

BOOST_TEST_TRAIT_FALSE(( hash_is_avalanching< std::hash<std::string > > ));
BOOST_TEST_TRAIT_TRUE(( hash_is_avalanching< X > ));
BOOST_TEST_TRAIT_TRUE(( hash_is_avalanching< Y > ));
BOOST_TEST_TRAIT_FALSE(( hash_is_avalanching< Z > ));
Copy link
Member

Choose a reason for hiding this comment

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

Let's put these three in their own test (plus one without is_avalanching) and leave this one alone.

@pdimov
Copy link
Member

pdimov commented May 27, 2025

Looks almost good to go. You just need to remove one include to make the cmake tests pass.

@pdimov
Copy link
Member

pdimov commented May 27, 2025

I wonder whether boost::container_hash is the right namespace for this trait. It contains traits classes that are specific for ContainerHash and for its internal use, whereas this one is rather more general and public.

boost:: might be a better place for it. Its name is sufficiently prefixed with hash_, like everything else ContainerHash puts into boost::.

@pdimov pdimov merged commit 0a00016 into boostorg:develop May 28, 2025
59 checks passed
joaquintides added a commit to boostorg/bloom that referenced this pull request May 28, 2025
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