Feature/concurrent node containers #271
Conversation
|
An automated preview of the documentation is available at https://271.unordered.prtest2.cppalliance.org/libs/unordered/doc/html/unordered.html |
1 similar comment
|
An automated preview of the documentation is available at https://271.unordered.prtest2.cppalliance.org/libs/unordered/doc/html/unordered.html |
| @@ -1458,6 +1458,11 @@ table_core:empty_value<Hash,0>,empty_value<Pred,1>,empty_value<Allocator,2> | |||
| using stats=table_core_stats; | |||
| #endif | |||
|
|
|||
| #if defined(BOOST_GCC) | |||
| #pragma GCC diagnostic push | |||
| #pragma GCC diagnostic ignored "-Wmaybe-uninitialized" | |||
There was a problem hiding this comment.
A maybe-uninitialized warning? What did the warning say, out of curiosity?
Historically, this might mean we need a launder() call somewhere.
There was a problem hiding this comment.
It is with this type of constructor:
table_core(
std::size_t n=default_bucket_count,const Hash& h_=Hash(),
const Pred& pred_=Pred(),const Allocator& al_=Allocator()):
hash_base{empty_init,h_},pred_base{empty_init,pred_},
allocator_base{empty_init,al_},arrays(new_arrays(n)),
size_ctrl{initial_max_load(),0}
{}As it happens, arrays (in arrays(new_arrays(n))) is a non-static member function that uses the internal allocator, but not other data members that have not been initialized yet (namely size_ctrl), but the compiler's analysis is not as fine-grained as that, seemingly. The situation would be similar to this example:
struct X
{
// calculate_b() called before X is fully initialized, but it's OK as b and c do not
// form part of the calculation.
X(int a_): a(a_), b(calculate_b()), c(0){}
int calculate_b() { return 2*a; }
int a;
int b;
int c;
};What I find surprising is that this warning didn't trigger before, maybe it only happens for complex, non-inlined scenarios.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #271 +/- ##
===========================================
- Coverage 98.07% 98.02% -0.05%
===========================================
Files 152 157 +5
Lines 20636 21369 +733
===========================================
+ Hits 20238 20948 +710
- Misses 398 421 +23
... and 6 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
An automated preview of the documentation is available at https://271.unordered.prtest2.cppalliance.org/libs/unordered/doc/html/unordered.html |
|
An automated preview of the documentation is available at https://271.unordered.prtest2.cppalliance.org/libs/unordered/doc/html/unordered.html |
|
An automated preview of the documentation is available at https://271.unordered.prtest2.cppalliance.org/libs/unordered/doc/html/unordered.html |
|
An automated preview of the documentation is available at https://271.unordered.prtest2.cppalliance.org/libs/unordered/doc/html/unordered.html |
|
An automated preview of the documentation is available at https://271.unordered.prtest2.cppalliance.org/libs/unordered/doc/html/unordered.html |
|
An automated preview of the documentation is available at https://271.unordered.prtest2.cppalliance.org/libs/unordered/doc/html/unordered.html |
|
An automated preview of the documentation is available at https://271.unordered.prtest2.cppalliance.org/libs/unordered/doc/html/unordered.html |
|
An automated preview of the documentation is available at https://271.unordered.prtest2.cppalliance.org/libs/unordered/doc/html/unordered.html |
|
An automated preview of the documentation is available at https://271.unordered.prtest2.cppalliance.org/libs/unordered/doc/html/unordered.html |
… 100% of the time)
|
An automated preview of the documentation is available at https://271.unordered.prtest2.cppalliance.org/libs/unordered/doc/html/unordered.html |
| /* GCC lifetime analysis incorrectly warns about uninitialized | ||
| * allocator object under some circumstances. | ||
| */ | ||
| if(empty())__builtin_unreachable(); |
There was a problem hiding this comment.
This seems fishy to me. Is calling get_allocator undefined behavior when empty()? Is the removal of the noexcept on purpose?
There was a problem hiding this comment.
get_allocatoron an empty node handle is UB, see here.- I removed
noexcepton purpose cause the standard does not have it (presumably because of the Lakos rule on narrow-contract functions). This removal is not materially connected to the__builtin_unreachableaddition, really. - The
__builtin_unreachableaddition is due to GCC incorrectly, and not systematically, warning about potentially uninitialized node handles here and here: adding__builtin_unreachablehelped but not always, so I was forced to also add this. I don't see any problem with foolproofingnode_handle_base::get_allocatoras I've done, but if it looks smelly/ugly I can just remove it, of course --the GCC pragma innode_handle_allocator_tests.cpphas us covered.
|
An automated preview of the documentation is available at https://271.unordered.prtest2.cppalliance.org/libs/unordered/doc/html/unordered.html |
|
I caused the merge conflict with my other PR, so @joaquintides gave me permission to resolve it here. |
|
An automated preview of the documentation is available at https://271.unordered.prtest2.cppalliance.org/libs/unordered/doc/html/unordered.html |
Adds
boost::concurrent_node_(map|set). The following is not part of this PR and will be done later:(insert|emplace)_and_visitoperations. This is still under discussion (conversation on the #boost-unordered Slack group).