mirror of
https://github.com/carbon-language/carbon-lang.git
synced 2026-09-24 19:20:13 +01:00
Fix uniform identifier generation for lengths over 64 (#7459)
`GetIdentifiersImpl` unconditionally sliced the 64-entry `IdentifierLengthCounts` table even for uniform distributions, which the API documents as having no `max_length` limit. Requesting a uniform distribution with `max_length > 64` therefore tripped an out-of-bounds slice assertion. Only compute the table slice on the non-uniform path, where `max_length <= 64` is already enforced. Add `IdentifierByteSumStableAcrossSeeds`, which exercises this path (a uniform request up to length 200) and checks the core invariant that the total identifier byte count is independent of the random seed across a spread of parameters. Assisted-by: Claude Code --------- Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
This commit is contained in:
co-authored by
josh11b
parent
a2890716ba
commit
383cfbb023
@@ -654,11 +654,15 @@ auto SourceGen::GetIdentifiersImpl(int number, int min_length, int max_length,
|
||||
idents.reserve(number);
|
||||
|
||||
// First, compute the total weight of the distribution so we know how many
|
||||
// identifiers we'll get each time we collect from it.
|
||||
// identifiers we'll get each time we collect from it. For a uniform
|
||||
// distribution every length has weight one, so the sum is simply the number
|
||||
// of lengths; this also avoids indexing the bounded `IdentifierLengthCounts`
|
||||
// table, which only covers lengths up to 64 and which uniform callers are
|
||||
// allowed to exceed.
|
||||
int num_lengths = max_length - min_length + 1;
|
||||
auto length_counts =
|
||||
llvm::ArrayRef(IdentifierLengthCounts).slice(min_length - 1, num_lengths);
|
||||
int count_sum = uniform ? num_lengths : Sum(length_counts);
|
||||
int count_sum = uniform ? num_lengths
|
||||
: Sum(llvm::ArrayRef(IdentifierLengthCounts)
|
||||
.slice(min_length - 1, num_lengths));
|
||||
CARBON_CHECK(count_sum >= 1);
|
||||
|
||||
int number_rem = number % count_sum;
|
||||
|
||||
@@ -7,9 +7,11 @@
|
||||
#include <gmock/gmock.h>
|
||||
#include <gtest/gtest.h>
|
||||
|
||||
#include <optional>
|
||||
#include <string>
|
||||
|
||||
#include "common/set.h"
|
||||
#include "llvm/Support/FormatVariadic.h"
|
||||
#include "testing/base/global_exe_path.h"
|
||||
#include "toolchain/base/install_paths_test_helpers.h"
|
||||
#include "toolchain/driver/driver.h"
|
||||
@@ -87,6 +89,89 @@ TEST(SourceGenTest, Identifiers) {
|
||||
EXPECT_THAT(idents, Each(SizeIs(AllOf(Ge(10), Le(20)))));
|
||||
}
|
||||
|
||||
// For fixed parameters, the total number of bytes across the returned
|
||||
// identifiers must not depend on the random seed, even though the specific
|
||||
// identifiers do. This checks that across a range of parameters and across many
|
||||
// freshly-seeded generators (each `SourceGen` gets an independent random seed).
|
||||
TEST(SourceGenTest, IdentifierByteSumStableAcrossSeeds) {
|
||||
struct Config {
|
||||
int number;
|
||||
int min_length;
|
||||
int max_length;
|
||||
bool uniform;
|
||||
bool unique;
|
||||
};
|
||||
// A spread of parameters including: the default range, narrow ranges, the
|
||||
// single-length extreme, uniform distributions, and a uniform range with a
|
||||
// `max_length` well beyond the 64 limit that only the uniform path allows.
|
||||
Config configs[] = {
|
||||
{.number = 1000, .min_length = 1, .max_length = 64, .uniform = false},
|
||||
{.number = 1000, .min_length = 4, .max_length = 64, .uniform = false},
|
||||
{.number = 999, .min_length = 1, .max_length = 64, .uniform = false},
|
||||
{.number = 1000, .min_length = 10, .max_length = 20, .uniform = false},
|
||||
{.number = 1000, .min_length = 8, .max_length = 8, .uniform = false},
|
||||
{.number = 100, .min_length = 10, .max_length = 19, .uniform = true},
|
||||
{.number = 97, .min_length = 10, .max_length = 19, .uniform = true},
|
||||
{.number = 500, .min_length = 50, .max_length = 200, .uniform = true},
|
||||
{.number = 1000,
|
||||
.min_length = 4,
|
||||
.max_length = 64,
|
||||
.uniform = false,
|
||||
.unique = true},
|
||||
{.number = 1000,
|
||||
.min_length = 4,
|
||||
.max_length = 4,
|
||||
.uniform = false,
|
||||
.unique = true},
|
||||
{.number = 200,
|
||||
.min_length = 30,
|
||||
.max_length = 120,
|
||||
.uniform = true,
|
||||
.unique = true},
|
||||
};
|
||||
|
||||
for (const Config& c : configs) {
|
||||
SCOPED_TRACE(llvm::formatv(
|
||||
"Config: number={0} min_length={1} max_length={2} uniform={3} "
|
||||
"unique={4}",
|
||||
c.number, c.min_length, c.max_length, c.uniform, c.unique));
|
||||
std::optional<ssize_t> expected_sum;
|
||||
bool any_different = false;
|
||||
std::optional<llvm::SmallVector<llvm::StringRef>> first;
|
||||
constexpr int NumSeeds = 8;
|
||||
for (int seed : llvm::seq(NumSeeds)) {
|
||||
// Each iteration constructs a fresh generator with an independent random
|
||||
// seed; the traced index identifies which iteration failed.
|
||||
SCOPED_TRACE(llvm::formatv("Seed iteration: {0}", seed));
|
||||
SourceGen gen;
|
||||
auto idents = c.unique
|
||||
? gen.GetShuffledUniqueIdentifiers(
|
||||
c.number, c.min_length, c.max_length, c.uniform)
|
||||
: gen.GetShuffledIdentifiers(c.number, c.min_length,
|
||||
c.max_length, c.uniform);
|
||||
EXPECT_THAT(idents, SizeIs(c.number));
|
||||
EXPECT_THAT(idents,
|
||||
Each(SizeIs(AllOf(Ge(c.min_length), Le(c.max_length)))));
|
||||
|
||||
ssize_t sum = SumSizes(idents);
|
||||
if (!expected_sum) {
|
||||
expected_sum = sum;
|
||||
first = idents;
|
||||
continue;
|
||||
}
|
||||
// The byte sum must be identical regardless of the seed.
|
||||
EXPECT_THAT(sum, Eq(*expected_sum));
|
||||
if (idents != *first) {
|
||||
any_different = true;
|
||||
}
|
||||
}
|
||||
// Sanity check that the generators really are producing different content,
|
||||
// so that the invariance check above is meaningful rather than trivially
|
||||
// passing on identical output.
|
||||
EXPECT_TRUE(any_different);
|
||||
}
|
||||
}
|
||||
|
||||
TEST(SourceGenTest, UniformIdentifiers) {
|
||||
SourceGen gen;
|
||||
// Check that uniform identifier length results in exact coverage of each
|
||||
|
||||
Reference in New Issue
Block a user