Fix Int overflow on 32-bit platforms (wasm32, watchOS)

[mansbernhardt]

The bug

TimecodeFrameRate.maxTotalSubFrames(in:base:) computes its product directly in Int:

maxTotalFrames(in: extent) * base.rawValue

With extent == .max100Days that product exceeds Int32.max for every frame rate. The smallest case, 23.976 fps at 80 subframes, is already

2_073_600 × 100 × 80 = 16_588_800_000     vs Int.max = 2_147_483_647

so the multiplication traps on overflow on any 32-bit platform — wasm32, and watchOS armv7k / arm64_32.

Because the bound is recomputed inside every wrapping add (sfcNew.clamped(to: 0 ... maxSubFrameCountExpressible)), this makes all arithmetic on a .max100Days timecode trap on those platforms, no matter how small the operands are.

Repro (wasm32)

var lhs = try Timecode(.realTime(seconds: 1.0), at: .fps59_94)
var rhs = try Timecode(.realTime(seconds: 192.0), at: .fps59_94)
lhs.properties.upperLimit = .max100Days
rhs.properties.upperLimit = .max100Days
_ = try lhs.adding(rhs, by: .wrapping)      // ← unreachable

Observed in a browser, wasm32 debug build:

Int is 32-bit, max=2147483647
maxTotalFrames(24h)        = 5184000
maxTotalFrames(100d)       = 518400000
maxTotalSubFrames(24h,80)  = 414720000     ← fits
maxTotalSubFrames(100d,80) → TRAP
limit max24Hours — adding ok 00:03:12:48   ← same operands
limit max100Days — adding TRAP             ← same operands

Construction, comparison, max(by:) and .realTimeValue all work; only arithmetic under .max100Days traps.

Worth noting this is easy to hit without ever choosing .max100Days deliberately: our wrapper type sets it on every Timecode it constructs, so every timecode operation trapped once we started building for wasm32.

The fix

Compute in Int64, saturate on return:

let product = Int64(maxTotalFrames(in: extent)) * Int64(base.rawValue)
return Int(clamping: product)

• No behaviour change on 64-bit. The product peaks at ~82.9e9 (120 fps, 100 days, 100 subframes), ~8 orders of magnitude below Int64.max, so the clamp never engages. The existing exact-value assertions in TimecodeFrameRate_Properties_Tests.properties() still hold.
• Correct on 32-bit. This value is only ever used as an upper bound — a clamped(to:) range, or a > comparison against a subFrameCount. A subFrameCount that large is itself unrepresentable in a 32-bit Int, so saturating at Int.max still bounds the entire representable domain.
• Signature unchanged, so it is not source-breaking.

Tests

Two regression tests in TimecodeFrameRate Properties Tests.swift:

• maxTotalSubFramesDoesNotOverflowOn32Bit() — every frame rate × every subframe base at .max100Days; asserts the exact product on 64-bit and Int.max on 32-bit, and that maxSubFrameCountExpressible stays consistent.
• max100DaysArithmeticDoesNotTrap() — the wrapping add above.

Full suite green locally: 506 tests in 55 suites passed.

One suggestion, happy to do it separately

The wasm CI jobs added in #87 (mine) run swift build only. This defect compiles perfectly and traps at runtime, so a build-only job structurally cannot catch it — and the tests above would have, had the suite run under wasm32. If you'd like, I can follow up with a PR that runs swift test on the wasm jobs via wasmtime or Node.

[orchetect]

Yes, valid point. The library was built primarily targeting 64-bit platforms but after the cross-platform effort invariably there are still 32-bit targets in this day and age. WASM64 is in the works but likely not viable any time soon.

Forgive my possible naïveté, but Int64 is available in Swift on 32-bit platforms (and cross-compiles successfully on the Swift WASM SDK). Can we not just respell Int as Int64 where necessary without any platform-conditional logic changes?

[orchetect]

wasm CI jobs added [...] run swift build only.
If you'd like, I can follow up with a PR that runs swift test on the wasm jobs via wasmtime or Node.

That would actually be fantastic if you would like to. Preferably branch off main and punt it over as a new PR even if the actual tests fail. I've only recently added Android and now WASM build jobs to repository CI pipelines, but just haven't had time to look into how to get actual unit tests happening on CI.

[mansbernhardt]

Not naïve at all — you're right, and it's the better fix. I've force-pushed it onto this branch (the saturating version is gone; shout if you'd rather have had it as a separate PR and I'll restore).

No platform-conditional logic anywhere. The internal domain widens cleanly because the sfcNew locals infer their type:

• internal: FrameCount.subFrameCount, framesToSubFrames, subFramesToFrames, FrameCount.init(subFrameCount:base:)
• public: TimecodeFrameRate.maxTotalSubFrames(in:base:), maxSubFrameCountExpressible(in:base:), Timecode.maxSubFrameCountExpressible — three signatures, Int → Int64. That is source-breaking for anyone binding the result to an Int, so say the word if you'd rather stage it behind a deprecated overload.

Deliberately not widened: maxTotalFrames, which peaks at 1_036_800_000 (120 fps over 100 days) and fits a 32-bit Int — along with the frames/subFrames components it bounds. Only the count needs 64 bits.

One narrowing remains, commented at the site: Timecode.rationalValue converts back to Int because Fraction is Int-based, so on a 32-bit platform a timecode beyond ~Int32.max subframes has no representable rational value. That is a pre-existing limit of Fraction rather than of the count. Happy to widen Fraction in a separate PR if you want it, but I did not want to expand this one's blast radius uninvited.

Also correcting something I wrote in the original description: I said the product exceeds Int32.max "for every frame rate". That holds at the 80- and 100-subframe bases, but at .quarterFrames the lower rates still fit. The test asserts across every rate/base pair rather than spot-checking, which is how I noticed.

Verified: full suite 506 tests in 55 suites on macOS, and 551 tests in 128 suites on wasm32 under wasmtime — the latter by pinning this branch into my own project, which was the only way I could actually execute your library's code on wasm. Which leads into the CI follow-up you asked for; PR coming, and it explains why running your suite there is not yet a two-line job.

Unlike the saturating version, this makes .max100Days genuinely usable on 32-bit rather than merely non-trapping.

[orchetect]

This holds true at present time, but we can't assume 120fps will remain the highest frame rate provided by the library, nor should we assume that 100 days the greatest maximum upper bound that will be implemented. At 100 days, 120fps occupies 30 bits + 1 for the sign bit. 240fps occupies 31 bits + the sign bit. 480fps overflows a 32-bit signed Int.

Unit tests will invariably trip at any future point if greater frame rates or upper bounds are supported of course but it may be worth considering at this stage.

[orchetect]

public: Int → Int64. That is source-breaking for anyone binding the result to an Int

My feeling that there should be consistency with consumed and emitted types concerning total frame counts and total subframe counts across the library's public API surface. Inconsistency may be confusing to the consumer if a frame count is typed as Int in one location but Int64 somewhere else. I realize this increases the necessary deprecation overloads but I think it is worth doing. I haven't had a close look at all possible sites concerned, but thought I would mention it.

You may have found them, but for cleanness and conciseness, deprecations all belong in a respective target's /API Evolution folder, where some can be found already. These files are named based upon the release version in which the deprecation appears, so these could be for release 3.1.4. That version can always be updated prior to release if needed of course.

Fraction is Int

In keeping with the concern for consistency, Fraction should likely be migrated to Int64 for its public API surface with deprecations. If you want to address that in this PR it would probably make more sense since it is closely related and reliant on changes made in this PR.

[mansbernhardt]

Agreed on consistency — a frame count typed Int in one place and Int64 in another is worse than either alone, and I will follow the /API Evolution convention with a SwiftTimecodeCore-API-3.1.4.swift for the deprecations. Thanks for pointing at that; I had not spotted it.

One finding worth folding into the scope before anyone starts, which came out of the WASM CI work in #89: there is a third total-count domain in the same family — audio samples. Timecode Samples Tests.swift carries literals like 4_147_200_000 stored into Int. That is 48 kHz × 24 hours, so unlike the frame-count ceiling it overflows a 32-bit Int at the library's ordinary limits, with no hypothetical future frame rate required. If totals are being made consistent, samples probably belong in that set alongside frames, subframes and Fraction.

That makes the job meaningfully larger than this PR, which is why I want to ask rather than assume: would you prefer to merge this one as approved and take the consistency work as a follow-up PR, or hold this one and do it all together?

I am happy either way and will do the work regardless — it is your API and your call on how to stage it. My only reason for raising it is that this PR is already approved and fixes a hard trap on 32-bit, so there may be value in it landing on its own rather than waiting behind a larger refactor. If you would rather have one coherent change, say so and I will fold it all in here, Fraction included.

---

Unrelated, in case it is useful: Tests (macOS), Tests (macOS - Swift 6.2) and Tests (macCatalyst) are all being cancelled at ~30 minutes here — and on main as well; I checked runs from 08-08, 08-09 and 08-10, which all show the same ~30m cutoff. It looks like a job timeout rather than anything in this diff, but it does leave this PR sitting at UNSTABLE despite your approval, so I thought it worth mentioning.

[orchetect]

there is a third total-count domain in the same family — audio samples
samples probably belong in that set

Yes, good point.

[macOS CI tests] are all being cancelled at ~30 minutes

Well aware. GitHub CI has been very unreliable and often the runners and Actions backend cause random test failures and cancellations. It's a constant game of plugging leaks in the dam because the runners are a moving target and their composition never stays static for long.

[orchetect]

would you prefer to merge this one as approved and take the consistency work as a follow-up PR, or hold this one and do it all together?

I think we can add it to this PR, as it's closely related in scope. The commit history is enough to allow in-situ rollbacks if needed.

[orchetect]

[macOS CI tests] are all being cancelled at ~30 minutes

FYI: I was right (#90). The runners are broken. There's nothing wrong with the package or the CI job itself. GitHub wastes so much of my time chasing false positives it's beyond belief.

[mansbernhardt]

Consistency work pushed — frames, subframes, samples and Fraction are now all Int64. Full suite green: 506 tests in 55 suites, including the CMTime bridge.

The rule I applied throughout: totals are Int64, per-component values stay Int.

Widened — FrameCount.Value's .frames/.split/.splitUnitInterval payloads, wholeFrames, maxTotalFrames, maxTotalFramesExpressible, maxTotalSubFrames, maxSubFrameCountExpressible, Timecode.Stride, .frames(_:) and .samples(_:) constructors, samplesValue(sampleRate:), and Fraction.numerator/denominator with its internal arithmetic.

Left as Int — Components' h/m/s/f, FrameCount.subFrames, and FeetAndFrames (24h at 24fps is only ~129,600 feet). These are components, not totals.

A nice side effect: widening Fraction let me delete the narrowing that the first version of this PR had to leave in rationalValue, where a 64-bit subframe count was being squeezed back into an Int.

Two things needing your call

1. Int companion overloads, not deprecated. .samples(_:), .frames(_:) and Fraction.init keep Int alongside Int64. This is not redundancy — Int is Swift's default integer-literal type, so with only Int64 and Double overloads present, .samples(48000 * 2, sampleRate: 48000) becomes ambiguous. I left them undeprecated deliberately: marking them deprecated would emit warnings on ordinary literal use, which seems worse than the inconsistency. Happy to flip them if you disagree.

2. No /API Evolution/SwiftTimecodeCore-API-3.1.4.swift yet, and I want to check the approach before writing one. The deprecation strategy only half-applies here:

• Parameter type changes are already covered by the Int companions above — a caller passing an Int keeps compiling.
• Return type changes cannot be deprecated this way at all. samplesValue(sampleRate:), wholeFrames, maxTotalFrames, Stride, and the Fraction properties changed what they return, and Swift will not let a deprecated shim differ only by return type without making call sites ambiguous. So these are simply source-breaking for anyone binding the result to an Int.

If you want migration cover for those, it needs differently-named accessors — something like samplesIntValue(sampleRate:) marked deprecated — rather than overloads. That is a naming decision I would rather you made than have me invent. Tell me which symbols you want covered and how you would like them named, and I will add the 3.1.4 file.

Worth noting the tests caught real 32-bit problems on the way through, not just type churn: several sample-count literals in Timecode Samples Tests.swift overflow a 32-bit Int outright.

[orchetect]

Thanks very much.

Swift will not let a deprecated shim differ only by return type without making call sites ambiguous

Not strictly the case.

func foo() -> Int { 0 }

@_disfavoredOverload
func foo() -> Int64 { 1 }

let x = foo() // infers Int, returns `0`
let y: Int64 = foo() // explicitly Int64, returns `1`

The least breaking solution for the consumer would be to keep Int as the preferred overload where possible. It may be feasible to not have any deprecations but instead offer overloads for all type consuming and emitting sites. In which case, they would not go in an API evolution file but go next to their sister method/properties. The side effect may be that 32-bit platforms may need to imperatively constrain types to 64-bit concrete types where overflows may be likely.

If there is a solution that can create the least disruption for the vast amount of consumers who are all working exclusively on 64-bit platforms that would be ideal. It doesn't make a ton of sense making disruptive changes to serve the needs of a tiny fraction of the consumer base.

[mansbernhardt]

I tested @_disfavoredOverload before replying — you're right, it behaves exactly as you describe. A bare call infers Int, an annotated one picks Int64, and internal call sites can force Int64 where they need exactness. My claim that Swift wouldn't allow it was wrong.

But testing it turned up something that changes the shape of the decision, so rather than reply I went and measured both options.

@_disfavoredOverload only covers functions

Properties can't be overloaded by type — computed or stored:

struct F {
    var numerator: Int { 1 }
    @_disfavoredOverload
    var numerator: Int64 { 2 }   // error: invalid redeclaration of 'numerator'
}

So "overloads for all type consuming and emitting sites" can cover the functions and initializers, but not Fraction.numerator/denominator, FrameCount.wholeFrames, the FrameCount.Value enum payloads, or the Stride typealias. Each of those has to pick a single type, and picking Int64 is a source break with no overload escape hatch.

Which made me try the opposite extreme

If consistency can't be achieved without breaking changes somewhere, it's worth knowing what the bug actually costs to fix on its own. Turns out: nothing.

Keeping every public signature exactly as it is, computing the bounds in Int64 internally, and having the public Int accessors clamp rather than trap:

	consistency refactor (currently on this PR)	minimal fix
public API changes	frames, subframes, samples, Fraction, Stride	none
test files changed	3	0
native suite	506 pass	504 pass
wasm32	not yet run	554 tests / 128 suites pass
fixes the 32-bit trap	yes	yes

Branch: mansbernhardt:experiment/minimal.

The clamp is safe for the same reason the first version of this PR was: these values are only ever used as an upper bound, and a subframe count that large is itself unrepresentable in a 32-bit Int, so clamping still bounds the entire representable domain. On 64-bit it never engages.

Suggestion

Land the minimal fix to close the 32-bit trap with zero disruption to the 64-bit majority, and treat API consistency as its own deliberate change later — because consistency now unavoidably means choosing types for properties that can't be overloaded, which is an API decision rather than a mechanical refactor, and it deserves to be made on its own terms rather than as a side effect of a bug fix.

That said, this is your library and you've already said you'd like the consistency work here. The full refactor is pushed and green if you'd prefer it — just say which and I'll set the PR to match. I'd rather give you the measurements than argue for one.

[orchetect]

Thanks for doing the exploratory work on this.

Properties can't be overloaded by type

If the properties were converted to functions (var foo: Int → func foo() → Int) it could be possible, and could have computed property deprecation proxies for them. But that's just contributing further to an already less than ideal over-arching solution.

having the public Int accessors clamp
Land the minimal fix to close the 32-bit trap with zero disruption to the 64-bit majority

The simplicity of this approach without API changes makes the most sense at this point in time. My only hesitation is having values silently clamp instead of returning actual true values, if that behavior is not obvious at the callsite for consumers.

As just one example, audio samples @ 48KHz overflows Int32 at just 13 hours.

treat API consistency as its own deliberate change later
deserves to be made on its own terms rather than as a side effect of a bug fix

Big-picture, yes - you're right. If we adopt Int64 across public API consistently where appropriate, we can also do it cleanly without deprecations or overloads if it is considered a major version bump.

There is one other possibility I might entertain at this junction before we ratify a solution. It wouldn't be entirely out of form to conditionally substitute Int64 using compiler fences only on 32-bit platforms. Essentially nothing changes for 64-bit platforms which continue to use Int, while 32-bit platforms will use Int64 where needed. If consumers have a mixed environment where they are compiling for both 64-bit and 32-bit platforms they will have to implement similar fences or just wrap all concerned values in Int64() at API boundaries so it cross-compiles. This gives a somewhat middle-ground solution where all consumers are getting true values in concrete types that make it clear to them how they should be handled.

[mansbernhardt]

Built and measured your fence idea rather than replying — it works, and it answers your clamping objection cleanly.

Three options, all green

	consistency refactor (on this PR now)	clamping	platform fence
64-bit public API	changed	unchanged	unchanged
32-bit values	true	clamped	true
test files changed	3	0	0
native suite	506 pass	504 pass	504 pass
wasm32 suite	not run	554 pass	554 pass
fixes the trap	yes	yes	yes

Branch: mansbernhardt:experiment/fence (clamping variant is experiment/minimal).

The shape is a single alias rather than fences at each signature, which keeps the duplication down:

#if _pointerBitWidth(_64)
public typealias TimecodeTotalCount = Int
#elseif _pointerBitWidth(_32)
public typealias TimecodeTotalCount = Int64
#else
#error("Unsupported pointer width — TimecodeTotalCount needs a mapping for this platform.")
#endif

Applied to total subframe counts and total sample counts — the two domains that provably overflow — plus the internal arithmetic between them. Components' h/m/s/f, FrameCount.subFrames and FeetAndFrames stay Int; they're components, not totals. Naming is a placeholder, rename as you like.

On a 64-bit build the alias is Int, so the diff is invisible to existing consumers: 504 tests pass with zero test-file changes.

One cost worth knowing before you pick it

Internal code has to be written alias-aware, and a 64-bit build will not catch mistakes. Because the alias is Int there, an ordinary Int(…) cast compiles perfectly and only fails when someone builds for 32-bit. I hit 66 such errors on the wasm32 build that were completely invisible natively.

That's an ongoing maintenance tax rather than a one-off, and it makes the WASM CI job in #89 load-bearing rather than nice-to-have — without a 32-bit build in CI, this class of breakage lands silently.

One thing I could not carry

Timecode.rationalValue still narrows, because Fraction is Int-based. On a 32-bit platform a timecode beyond ~Int32.max subframes has no representable rational value. Widening Fraction belongs with the broader consistency work rather than with a bug fix, so I left it and commented the site.

Happy to set this PR to whichever of the three you prefer — say the word and it's one push. If it were mine I'd take the fence: it fixes the trap, gives true values everywhere, and costs 64-bit consumers nothing. But the maintenance tax above is real and you're the one who'll carry it.

[orchetect]

I'd probably move toward something more generic for an alias name like PlatformInt if we go that direction, as it's being used for a variety of units. But either way, it does add another layer of possible confusion for consumers when they see something other than standard integer types in signatures.

It's possible to swap in the alias for Fraction. If you swap it in for its two stored properties then squash compiler errors until it's swapped out downstream where needed, it works.

However, the more I dig into this the more it becomes evident there is no trivial way to do it cleanly without some form of compromise for the consumer.

I'm increasingly leaning toward a new major version release where the entire codebase would adopt specific bitwidth types for public API consistently at every overflow pinch point, measured not just against current upper bounds but taking into consideration the potential for larger frame rates in future.

[mansbernhardt]

On the pin, since it's the one practical thing outstanding on our side: we currently track this fork branch by revision: in our own package. That works fine, but it means carrying an unreleased dependency.

Would you consider landing either the clamping or the fence variant as a 3.1.x patch in the meantime? Both are zero-public-API-change on 64-bit and green (504 native, 554 on wasm32), so neither pre-empts nor constrains the major version you're planning — they'd just close the 32-bit trap for anyone hitting it today, and let us move back onto a released tag.

Entirely your call, and no urgency from our side; the pin is stable. Happy to wait for the major version if you'd rather do it once, properly.

On the alias name — agreed that PlatformInt is better than what I used if the fence approach survives into the major version; it's used for several different units and the name shouldn't imply one.