Skip to content

Conversation

@Turbo87
Copy link
Contributor

@Turbo87 Turbo87 commented Nov 8, 2025

The unit struct fields in Units enum variants were never accessed - they were only used for construction and immediately discarded in pattern matching.

Changes:

  • Convert tuple-like enum variants (e.g., steradian(steradian)) to unit-like variants (e.g., steradian)
  • Update pattern matching to use simpler syntax without wildcards
  • Update ALL_UNITS initialization to use unit-like syntax
  • Update tests to construct enum variants without passing unit structs

Benefits:

  • Simpler, clearer API
  • Smaller enum variants (no tuple wrapper)
  • Less confusion about unused fields

Breaking change: External code constructing Units enum variants must use unit-like syntax instead of tuple-like syntax.

Fixes #539

The unit struct fields in `Units` enum variants were never accessed - they
were only used for construction and immediately discarded in pattern matching.

Changes:
- Convert tuple-like enum variants (e.g., `steradian(steradian)`) to unit-like
  variants (e.g., `steradian`)
- Update pattern matching to use simpler syntax without wildcards
- Update `ALL_UNITS` initialization to use unit-like syntax
- Update tests to construct enum variants without passing unit structs

Benefits:
- Simpler, clearer API
- Smaller enum variants (no tuple wrapper)
- Less confusion about unused fields

Breaking change: External code constructing `Units` enum variants must
use unit-like syntax instead of tuple-like syntax.

Fixes iliekturtles#539
@iliekturtles iliekturtles force-pushed the remove-units-enum-fields branch from 6124bb7 to 049b05d Compare January 22, 2026 22:13
@iliekturtles
Copy link
Owner

Thanks for the PR. I rebased on top of the latest master and pushed to force Github checks to show up. The PR must have been submitted during the Github actions outage, because checks weren't originally available.

@Turbo87
Copy link
Contributor Author

Turbo87 commented Jan 22, 2026

checks weren't originally available

I think these days they only run by default if the PR author has already committed to the repo before :)

@iliekturtles
Copy link
Owner

iliekturtles commented Jan 22, 2026

Normally I get an approval button for first-time contributors, but it was missing for your PRs!

Hyperfine results, 1.03 +/- 0.02 faster compile times with the PR using rustc 1.93.0. (EDIT, re-ran on a quiet system)

$ hyperfine "touch uom/src/lib.rs; cargo build --manifest-path=uom/Cargo.toml" "touch uom-remove-units/src/lib.rs; cargo build --manifest-path=uom-remove-units/Cargo.toml"
Benchmark 1: touch uom/src/lib.rs; cargo build --manifest-path=uom/Cargo.toml
  Time (mean ± σ):      3.600 s ±  0.045 s    [User: 3.158 s, System: 0.401 s]
  Range (min … max):    3.552 s …  3.702 s    10 runs

Benchmark 2: touch uom-remove-units/src/lib.rs; cargo build --manifest-path=uom-remove-units/Cargo.toml
  Time (mean ± σ):      3.508 s ±  0.034 s    [User: 3.057 s, System: 0.387 s]
  Range (min … max):    3.459 s …  3.563 s    10 runs

Summary
  touch uom-remove-units/src/lib.rs; cargo build --manifest-path=uom-remove-units/Cargo.toml ran
    1.03 ± 0.02 times faster than touch uom/src/lib.rs; cargo build --manifest-path=uom/Cargo.toml

@iliekturtles iliekturtles merged commit 5af6803 into iliekturtles:master Jan 22, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Are unit structs necessary in Units enums?

2 participants