Test what a deletion sweep found nothing was watching - #197
Merged
Conversation
Issue #194 asked for the other half of the mutation work: --operators deletions removes whole statements rather than changing one token, and had never been run over the whole header. 665 mutants, 75 of them survived. A surviving deletion says one of two things, and both turned up here. Most of it was untested behaviour. The table carries four things besides its elements -- the hasher, the key equality, the max load factor and the size it calls full -- and each is copied, moved or swapped by hand, in four places. Every one of those statements could be deleted with the suite still green, because every test uses std::hash and std::equal_to, where a table that keeps its own hasher and one that takes the other's are indistinguishable. A hasher carrying an inert tag makes them tell apart, which is what test/app/hashers.h grows here. The same shape, one level down: four at() overloads of which the tests reached the two non-const ones, try_emplace with a hint reached only for the map's own key type, the iterator-to-const_iterator converting *assignment* (as opposed to the construction, which is everywhere), and segmented_vector's free swap. Those now come back `compiler` rather than `survived` -- deleting a return from a function that is actually instantiated does not build, which is a firmer kill than a failing assertion. Two were real gaps in code that looks exercised. extract(iterator) walks the probe sequence from the key's home bucket, and the statement advancing that walk was removable: every element any test extracted sat in its home bucket, so the loop never went round once. And max_load_factor() recomputes the size at which the table is full, which no test noticed because they all set it on a table with no buckets yet, where there is nothing to recompute. The seventeen scalar hash specializations were unchecked as a block -- hash_char_types.cpp held one empty test case and a "TODO make hash generic?". Losing one is invisible without looking: hash<T> falls back to the primary template, which forwards to std::hash and is not avalanching. Asserting both halves turns all seventeen into compile errors. The rest was the second kind: statements that do nothing. wyhash zeroes a and b for the short-input cases, and both are already zero from their declarations -- the empty-input branch exists only to assign zero to two variables that hold it. The golden hash values cover length 0, so this is provably the same function rather than merely an untested one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Neither of these is new, and neither is caused by the tests this branch adds -- adding two files changed which chunk the existing ones land in, and that was enough to put these two pairs together for the first time. This is precisely what the unity leg is for: a collision that was already there and invisible. set_or_map_types.cpp declared five names at file scope, four of them as generic as names get (map1_t, map2_t, set1_t, set2_t). unordered_set.cpp declares its own local set1_t, so once the two shared a translation unit the local one shadowed the file-scope one -- and -Wshadow=global reported it against unordered_set.cpp, which had done nothing wrong. An anonymous namespace would not have helped, because what is shadowed is a scope and not a linkage, so the whole file moves into a namespace of its own instead. segmented_vector_allocators.cpp had its own require_holds, and the vectors it passes carry a test::id_allocator -- so ADL also finds test::require_holds in app/map_fixtures.h, which asks the same question of a map. Two equally good candidates is an ambiguous call. Renamed to require_vector_holds, which is what it actually asks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #194.
--operators deletionsremoves whole statements rather than changing one token, and had never been run over the whole header. This is that run, the triage, and the outcome for each kind.The run
compilercaughthangoomsurvived665 mutants in 843s. Every fix below was confirmed by re-running the specific mutants, not assumed.
What survived, and why
#194 predicted two kinds, and both turned up — plus a third the issue did not anticipate.
1. Untested behaviour — the state a table carries (13)
A table carries four things besides its elements: the hasher, the key equality, the max load factor, and the size at which it calls itself full. Each is copied, moved or swapped by hand, one statement per member, in four separate places (copy assignment, both branches of move assignment, and swap). Every one of those statements was deletable with the suite green.
The reason is worth keeping: every existing test uses
std::hashandstd::equal_to, which are stateless — so a table that keeps its own hasher and one that takes the other's are indistinguishable.test/app/hashers.hgains a hasher and an equality carrying an inert tag, which is the smallest thing that makes the question askable. The tag takes no part in hashing, so swapping tables that carry different tags cannot change what either can find.The full-threshold has no getter at all; it shows only in when the table rehashes, so it is pinned by
bucket_count()holding still while the table demonstrably still has room.2. Untested behaviour — overloads nothing ever called (5+17)
Two of the four
at()overloads (const × transparent), transparenttry_emplacewith a hint, the iterator→const_iterator converting assignment (the construction is used everywhere; the assignment was not), andsegmented_vector's freeswap.These now come back
compilerrather thancaught— deleting areturnfrom a function that is genuinely instantiated does not build. That is a firmer kill than a failing assertion, and it is also the proof that they were previously uninstantiated: a deletion can only compile if nothing asks for the template.Same shape for the 17 scalar
hash<T>specializations, which were unchecked as a block:hash_char_types.cppheld one empty test case and a// TODO(martinus) make hash generic?. Losing one is invisible without looking —hash<T>silently falls back to the primary template, which forwards tostd::hashand is not avalanching. Asserting both halves turns all seventeen into compile errors. The file is renamedhash_scalar_types.cpp, since it coversboolthroughunsigned long long, and that TODO is now implemented.3. Real gaps in code that looked exercised (2)
extract(iterator)walks the probe sequence from the key's home bucket, and the statement advancing that walk was removable. That can only mean the loop never went round once: every element every test extracted was already in its home bucket. A hash that sends every key to the same bucket makes the walk unavoidable. The mutant is nowhang— without the advance it spins forever.max_load_factor()recomputes the full-threshold, and nothing noticed it missing, because every test set the factor on a table with no buckets yet, where there is nothing to recompute. Setting it on a populated table has to make the next insert grow.4. Statements that do nothing (3)
wyhash zeroes
aandbfor the short-input cases, and both are already zero fromstd::uint64_t a{}/b{}— the empty-input branch existed only to assign zero to two variables already holding it.This is provably the same function rather than merely an untested one: the golden values pin length 0 explicitly (
""→0x42bc986dc5eec4d3) and the loop over lengths 0–200 covers the branch. So the branch is exercised; the statements simply have no effect. That is the distinction #194 asked for, and it is the reason this is a removal rather than a test.What is left
37 survivors remain, filed as a follow-up issue with the triage attached. They are a different population from the 38 fixed here — mostly exception-recovery paths,
if constexprbranches no tested type instantiates, and a handful that are provably unkillable:~table() = default(identical to the implicit one), the twoPREFETCHcalls (a pure performance hint), andoperator=='s&a == &bfast path (the general path returns the same answer). Those three are not test gaps and should not be chased; the sweep will report them every time.Verification
590 test cases pass (up from 539), clang-format clean, and each group re-checked against the mutants it targets.
🤖 Generated with Claude Code