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

C++ unordered_set hash/comparator requirements #1348

Closed
veripoolbot opened this issue Sep 16, 2018 · 10 comments
Closed

C++ unordered_set hash/comparator requirements #1348

veripoolbot opened this issue Sep 16, 2018 · 10 comments

Comments

@veripoolbot
Copy link
Contributor

@veripoolbot veripoolbot commented Sep 16, 2018


Author Name: Kevin Kiningham (@kkiningh)
Original Redmine Issue: 1348 from https://www.veripool.org


Both the hash and comparator functors provided to std::unordered_set must be copy constructible per the C++ standard. Otherwise, compiling verilator with libc++ fails with the following errors

"error: static_assert failed "the specified hash does not meet the Hash requirements""
"error: static_assert failed "the specified comparator is required to be copy constructible""

The error goes away if both EqSenTree and HashSenTree are made copyable.

Also, for some reason std::less and std::less are considered non-invokable in SortByValueMap::removeKeyFromOldVal, which causes a warning. I'm not totally sure why this is since it looks like both have a const operator< member function defined.

You can check for this by adding the following static assert to the first line of removeKeyFromOldVal.

static_assert(std::__invokable<const T_KeyCompare &, const T_Key &, const T_Key &>::value, "Not invokable");

(In c++17 you can use std::is_invocable instead of std::__invokable)

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Sep 16, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-09-16T23:56:45Z


Which OS distribution and GCC?

It sounds like you were able to get it working, could you please post the patch that fixes this?

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Sep 17, 2018


Original Redmine Comment
Author Name: Kevin Kiningham (@kkiningh)
Original Date: 2018-09-17T00:24:00Z


OSX 10.13.6 and apple's version of clang. I'm also using libc++ instead of libstdc++.

Apple LLVM version 9.1.0 (clang-902.0.39.2)
Target: x86_64-apple-darwin17.7.0
Thread model: posix
</code>

I attached a patch for the first problem, but I don't know how to resolve the second.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Sep 17, 2018


Original Redmine Comment
Author Name: Kevin Kiningham (@kkiningh)
Original Date: 2018-09-17T00:54:17Z


I have two other small patches that fix warnings from clang - one fixes a format error and the other fixes an unused variable error.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Sep 17, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-09-17T10:42:01Z


I pushed to git your three patches, thanks.

As to the invokable one I wasn't able to get an assertion compiling to show the issue, I'll need to do more research, what is the exact compiler message you get?

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Sep 17, 2018


Original Redmine Comment
Author Name: Kevin Kiningham (@kkiningh)
Original Date: 2018-09-17T15:39:06Z


Here's the full messages.

From Compiling src/V3Scoreboard.cpp:

In file included from src/V3Scoreboard.cpp:24:
In file included from bazel-out/darwin-fastbuild/bin/_virtual_includes/verilator_libV3/V3Scoreboard.h:33:
In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/map:442:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/__tree:1819:22: warning: the specified comparator type does not provide a const call operator [-Wuser-defined-warnings]
                      __trigger_diagnostics()), "");
                      ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/set:400:28: note: in instantiation of member function 'std::__1::__tree<const ScoreboardTestElem *, V3Scoreboard<ScoreboardTestElem, unsigned int, std::__1::less<ScoreboardTestElem> >::CmpElems, std::__1::allocator<const ScoreboardTestElem *> >::~__tree' requested here
class _LIBCPP_TEMPLATE_VIS set
                            ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/memory:1550:14: note: in instantiation of function template specialization 'std::__1::allocator_traits<std::__1::allocator<std::__1::__tree_node<std::__1::__value_type<unsigned int, std::__1::set<const ScoreboardTestElem *, V3Scoreboard<ScoreboardTestElem, unsigned int, std::__1::less<ScoreboardTestElem> >::CmpElems, std::__1::allocator<const ScoreboardTestElem *> > >, void *> > >::__destroy<std::__1::pair<const unsigned int, std::__1::set<const ScoreboardTestElem *, V3Scoreboard<ScoreboardTestElem, unsigned int, std::__1::less<ScoreboardTestElem> >::CmpElems, std::__1::allocator<const ScoreboardTestElem *> > > >' requested here
             {__destroy(__has_destroy<allocator_type, _Tp*>(), __a, __p);}
              ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/__tree:1833:24: note: in instantiation of function template specialization 'std::__1::allocator_traits<std::__1::allocator<std::__1::__tree_node<std::__1::__value_type<unsigned int, std::__1::set<const ScoreboardTestElem *, V3Scoreboard<ScoreboardTestElem, unsigned int, std::__1::less<ScoreboardTestElem> >::CmpElems, std::__1::allocator<const ScoreboardTestElem *> > >, void *> > >::destroy<std::__1::pair<const unsigned int, std::__1::set<const ScoreboardTestElem *, V3Scoreboard<ScoreboardTestElem, unsigned int, std::__1::less<ScoreboardTestElem> >::CmpElems, std::__1::allocator<const ScoreboardTestElem *> > > >' requested here
         __node_traits::destroy(__na, _NodeTypes::__get_ptr(__nd->__value_));
                        ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/__tree:1821:3: note: in instantiation of member function 'std::__1::__tree<std::__1::__value_type<unsigned int, std::__1::set<const ScoreboardTestElem *, V3Scoreboard<ScoreboardTestElem, unsigned int, std::__1::less<ScoreboardTestElem> >::CmpElems, std::__1::allocator<const ScoreboardTestElem *> > >, std::__1::__map_value_compare<unsigned int, std::__1::__value_type<unsigned int, std::__1::set<const ScoreboardTestElem *, V3Scoreboard<ScoreboardTestElem, unsigned int, std::__1::less<ScoreboardTestElem> >::CmpElems, std::__1::allocator<const ScoreboardTestElem *> > >, std::__1::less<unsigned int>, true>, std::__1::allocator<std::__1::__value_type<unsigned int, std::__1::set<const ScoreboardTestElem *, V3Scoreboard<ScoreboardTestElem, unsigned int, std::__1::less<ScoreboardTestElem> >::CmpElems, std::__1::allocator<const ScoreboardTestElem *> > > > >::destroy' requested here
  destroy(__root());
  ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/map:805:28: note: in instantiation of member function 'std::__1::__tree<std::__1::__value_type<unsigned int, std::__1::set<const ScoreboardTestElem *, V3Scoreboard<ScoreboardTestElem, unsigned int, std::__1::less<ScoreboardTestElem> >::CmpElems, std::__1::allocator<const ScoreboardTestElem *> > >, std::__1::__map_value_compare<unsigned int, std::__1::__value_type<unsigned int, std::__1::set<const ScoreboardTestElem *, V3Scoreboard<ScoreboardTestElem, unsigned int, std::__1::less<ScoreboardTestElem> >::CmpElems, std::__1::allocator<const ScoreboardTestElem *> > >, std::__1::less<unsigned int>, true>, std::__1::allocator<std::__1::__value_type<unsigned int, std::__1::set<const ScoreboardTestElem *, V3Scoreboard<ScoreboardTestElem, unsigned int, std::__1::less<ScoreboardTestElem> >::CmpElems, std::__1::allocator<const ScoreboardTestElem *> > > > >::~__tree' requested here
class _LIBCPP_TEMPLATE_VIS map
                            ^
src/V3Scoreboard.cpp:46:48: note: in instantiation of member function 'V3Scoreboard<ScoreboardTestElem, unsigned int, std::__1::less<ScoreboardTestElem> >::V3Scoreboard' requested here
     V3Scoreboard<ScoreboardTestElem, uint32_t> sb(ScoreboardTestElem::scoreFn, true);
                                                ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/__tree:970:7: note: from 'diagnose_if' attribute on '__trigger_diagnostics':
       _LIBCPP_DIAGNOSE_WARNING(!__invokable<_Compare const&, _Tp const&, _Tp const&>::value,
       ^                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/__config:1095:20: note: expanded from macro '_LIBCPP_DIAGNOSE_WARNING'
     __attribute__((diagnose_if(__VA_ARGS__, "warning")))
                    ^           ~~~~~~~~~~~
1 warning generated.
</code>

From Compiling src/V3Partition.cpp:

In file included from src/V3Partition.cpp:27:
In file included from bazel-out/darwin-fastbuild/bin/_virtual_includes/verilator_libV3/V3Os.h:25:
In file included from bazel-out/darwin-fastbuild/bin/_virtual_includes/verilator_libV3/V3Error.h:29:
In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/map:442:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/__tree:1819:22: warning: the specified comparator type does not provide a const call operator [-Wuser-defined-warnings]
                      __trigger_diagnostics()), "");
                      ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/__tree:960:70: note: in instantiation of member function 'std::__1::__tree<const MergeCandidate *, V3Scoreboard<MergeCandidate, unsigned int, std::__1::less<MergeCandidate> >::CmpElems, std::__1::allocator<const MergeCandidate *> >::~__tree' requested here
     template <class, class, class> friend class _LIBCPP_TEMPLATE_VIS set;
                                                                      ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/memory:1759:31: note: in instantiation of function template specialization 'std::__1::pair<const unsigned int, std::__1::set<const MergeCandidate *, V3Scoreboard<MergeCandidate, unsigned int, std::__1::less<MergeCandidate> >::CmpElems, std::__1::allocator<const MergeCandidate *> > >::pair<const unsigned int &>' requested here
             ::new((void*)__p) _Up(_VSTD::forward<_Args>(__args)...);
                               ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/memory:1670:18: note: in instantiation of function template specialization 'std::__1::allocator<std::__1::__tree_node<std::__1::__value_type<unsigned int, std::__1::set<const MergeCandidate *, V3Scoreboard<MergeCandidate, unsigned int, std::__1::less<MergeCandidate> >::CmpElems, std::__1::allocator<const MergeCandidate *> > >, void *> >::construct<std::__1::pair<const unsigned int, std::__1::set<const MergeCandidate *, V3Scoreboard<MergeCandidate, unsigned int, std::__1::less<MergeCandidate> >::CmpElems, std::__1::allocator<const MergeCandidate *> > >, const std::__1::piecewise_construct_t &, std::__1::tuple<const unsigned int &>, std::__1::tuple<> >' requested here
             {__a.construct(__p, _VSTD::forward<_Args>(__args)...);}
                  ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/memory:1516:14: note: in instantiation of function template specialization 'std::__1::allocator_traits<std::__1::allocator<std::__1::__tree_node<std::__1::__value_type<unsigned int, std::__1::set<const MergeCandidate *, V3Scoreboard<MergeCandidate, unsigned int, std::__1::less<MergeCandidate> >::CmpElems, std::__1::allocator<const MergeCandidate *> > >, void *> > >::__construct<std::__1::pair<const unsigned int, std::__1::set<const MergeCandidate *, V3Scoreboard<MergeCandidate, unsigned int, std::__1::less<MergeCandidate> >::CmpElems, std::__1::allocator<const MergeCandidate *> > >, const std::__1::piecewise_construct_t &, std::__1::tuple<const unsigned int &>, std::__1::tuple<> >' requested here
             {__construct(__has_construct<allocator_type, _Tp*, _Args...>(),
              ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/__tree:2192:20: note: in instantiation of function template specialization 'std::__1::allocator_traits<std::__1::allocator<std::__1::__tree_node<std::__1::__value_type<unsigned int, std::__1::set<const MergeCandidate *, V3Scoreboard<MergeCandidate, unsigned int, std::__1::less<MergeCandidate> >::CmpElems, std::__1::allocator<const MergeCandidate *> > >, void *> > >::construct<std::__1::pair<const unsigned int, std::__1::set<const MergeCandidate *, V3Scoreboard<MergeCandidate, unsigned int, std::__1::less<MergeCandidate> >::CmpElems, std::__1::allocator<const MergeCandidate *> > >, const std::__1::piecewise_construct_t &, std::__1::tuple<const unsigned int &>, std::__1::tuple<> >' requested here
     __node_traits::construct(__na, _NodeTypes::__get_ptr(__h->__value_), _VSTD::forward<_Args>(__args)...);
                    ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/__tree:2137:29: note: (skipping 1 context in backtrace; use -ftemplate-backtrace-limit=0 to see all)
         __node_holder __h = __construct_node(_VSTD::forward<_Args>(__args)...);
                             ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/map:1319:20: note: in instantiation of function template specialization 'std::__1::__tree<std::__1::__value_type<unsigned int, std::__1::set<const MergeCandidate *, V3Scoreboard<MergeCandidate, unsigned int, std::__1::less<MergeCandidate> >::CmpElems, std::__1::allocator<const MergeCandidate *> > >, std::__1::__map_value_compare<unsigned int, std::__1::__value_type<unsigned int, std::__1::set<const MergeCandidate *, V3Scoreboard<MergeCandidate, unsigned int, std::__1::less<MergeCandidate> >::CmpElems, std::__1::allocator<const MergeCandidate *> > >, std::__1::less<unsigned int>, true>, std::__1::allocator<std::__1::__value_type<unsigned int, std::__1::set<const MergeCandidate *, V3Scoreboard<MergeCandidate, unsigned int, std::__1::less<MergeCandidate> >::CmpElems, std::__1::allocator<const MergeCandidate *> > > > >::__emplace_unique_key_args<unsigned int, const std::__1::piecewise_construct_t &, std::__1::tuple<const unsigned int &>, std::__1::tuple<> >' requested here
     return __tree_.__emplace_unique_key_args(__k,
                    ^
bazel-out/darwin-fastbuild/bin/_virtual_includes/verilator_libV3/V3Scoreboard.h:244:32: note: in instantiation of member function 'std::__1::map<unsigned int, std::__1::set<const MergeCandidate *, V3Scoreboard<MergeCandidate, unsigned int, std::__1::less<MergeCandidate> >::CmpElems, std::__1::allocator<const MergeCandidate *> >, std::__1::less<unsigned int>, std::__1::allocator<std::__1::pair<const unsigned int, std::__1::set<const MergeCandidate *, V3Scoreboard<MergeCandidate, unsigned int, std::__1::less<MergeCandidate> >::CmpElems, std::__1::allocator<const MergeCandidate *> > > > >::operator[]' requested here
         KeySet& keysAtOldVal = m_vals[oldVal];
                                ^
bazel-out/darwin-fastbuild/bin/_virtual_includes/verilator_libV3/V3Scoreboard.h:330:9: note: in instantiation of member function 'SortByValueMap<const MergeCandidate *, unsigned int, V3Scoreboard<MergeCandidate, unsigned int, std::__1::less<MergeCandidate> >::CmpElems>::removeKeyFromOldVal' requested here
         removeKeyFromOldVal(k, kvit->second);
         ^
bazel-out/darwin-fastbuild/bin/_virtual_includes/verilator_libV3/V3Scoreboard.h:426:27: note: in instantiation of member function 'SortByValueMap<const MergeCandidate *, unsigned int, V3Scoreboard<MergeCandidate, unsigned int, std::__1::less<MergeCandidate> >::CmpElems>::erase' requested here
         if (0 == m_sorted.erase(elp)) {
                           ^
src/V3Partition.cpp:1093:22: note: in instantiation of member function 'V3Scoreboard<MergeCandidate, unsigned int, std::__1::less<MergeCandidate> >::removeElem' requested here
                 sbp->removeElem(tedgep);
                      ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/__tree:970:7: note: from 'diagnose_if' attribute on '__trigger_diagnostics':
       _LIBCPP_DIAGNOSE_WARNING(!__invokable<_Compare const&, _Tp const&, _Tp const&>::value,
       ^                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/__config:1095:20: note: expanded from macro '_LIBCPP_DIAGNOSE_WARNING'
     __attribute__((diagnose_if(__VA_ARGS__, "warning")))
                    ^           ~~~~~~~~~~~
1 warning generated.
</code>
@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Sep 17, 2018


Original Redmine Comment
Author Name: Kevin Kiningham (@kkiningh)
Original Date: 2018-09-17T15:47:57Z


Ah - found the problem. CmpElems::operator() was declared non const. Attached patch fixes the warning.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Sep 17, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-09-17T22:41:30Z


Great. Strange that cppcheck didn't complain about the missing const, it usually finds those.

Sounds like you're all set, but if this still gives problems I think there's an alternative fix that might work.

Pushed to git towards 4.004.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Sep 17, 2018


Original Redmine Comment
Author Name: Kevin Kiningham (@kkiningh)
Original Date: 2018-09-17T23:29:01Z


Another possible issue I noticed while working on this - it looks like gcc isn't being called with any -std=XXX even though autoconf recognizes that my compiler is gnu++14 capable. Is that the intended behavior? That means by default it uses the custom unordered_set provided with verilator rather than the system one, which might be why no one else noticed the first issue until now. I'm not super familiar with autoconf or the build system, but it looks like everything is compiled with $CPPFLAGS, but only $CXXFLAGS has the standard flag?

Also, not sure if it's related, but in configure.ac on line 237 you have "_MY_CXX_CHECK_SET(CFG_CXXFLAGS_STD_OLDEST,-std=std++03)". Should that be '-std=c++03'? At least with clang, -std=std++XX isn't a legal option.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Sep 18, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-09-18T01:49:47Z


I fixed the std++03 thing, that was a typo, though the macro it sets isn't presently used. At present these flags are only used for runtime, that should be fixed, but I'll need to do a lot of testing first as these flags seem to break a lot of stuff in glibc.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Oct 6, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-10-06T14:14:20Z


In 4.004.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.