Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use trait for labels instead of TypeId #5270

Merged
merged 1 commit into from
Jun 21, 2023
Merged

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Jun 21, 2023

Summary

Fixes #5263

Rust 1.72.0 changes TypeId from a u64 to a u128. This breaks Ruffs release build because it increases the size of FormatElement.

This PR removes the dependency on TypeId for LabelId and instead introduces a new trait that downstream crates should implement on an enum defining all labels for that project.

This approach has two downsides:

  • The API cannot statically enforce that a project uses a single LabelDefinition. Mixing two LabelDefinition could result in two labels being equal that should not be equal. I added a
    debug assertion that compares the names of the labels to mitigate the risk.
  • Implementing the first Label requires more boilerplate code than before

One main upside is that it allows a project to define all labels in a single place (and search for all label usages).

Test Plan

  • cargo test
  • cargo build -p ruff_python_formatter --release on Rust 1.72.0

@MichaReiser
Copy link
Member Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@MichaReiser MichaReiser added internal An internal refactor or improvement formatter Related to the formatter labels Jun 21, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 21, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.01      7.0±0.02ms     5.8 MB/sec    1.00      6.9±0.01ms     5.9 MB/sec
formatter/numpy/ctypeslib.py               1.04   1434.0±3.37µs    11.6 MB/sec    1.00   1385.1±1.99µs    12.0 MB/sec
formatter/numpy/globals.py                 1.01    134.0±0.24µs    22.0 MB/sec    1.00    133.3±1.19µs    22.1 MB/sec
formatter/pydantic/types.py                1.01      2.9±0.02ms     8.8 MB/sec    1.00      2.9±0.01ms     8.9 MB/sec
linter/all-rules/large/dataset.py          1.00     13.6±0.02ms     3.0 MB/sec    1.00     13.6±0.04ms     3.0 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.5±0.00ms     4.8 MB/sec    1.00      3.5±0.00ms     4.8 MB/sec
linter/all-rules/numpy/globals.py          1.00    368.1±1.15µs     8.0 MB/sec    1.00    367.7±1.22µs     8.0 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.0±0.01ms     4.2 MB/sec    1.00      6.0±0.02ms     4.2 MB/sec
linter/default-rules/large/dataset.py      1.01      7.0±0.02ms     5.8 MB/sec    1.00      7.0±0.01ms     5.8 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1474.0±3.81µs    11.3 MB/sec    1.00   1468.6±1.77µs    11.3 MB/sec
linter/default-rules/numpy/globals.py      1.00    154.6±0.22µs    19.1 MB/sec    1.00    154.3±0.32µs    19.1 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.2±0.01ms     8.0 MB/sec    1.00      3.2±0.00ms     8.0 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      5.7±0.04ms     7.2 MB/sec    1.00      5.7±0.06ms     7.2 MB/sec
formatter/numpy/ctypeslib.py               1.04   1157.8±9.47µs    14.4 MB/sec    1.00  1118.3±32.14µs    14.9 MB/sec
formatter/numpy/globals.py                 1.01    110.0±2.37µs    26.8 MB/sec    1.00    109.4±2.03µs    27.0 MB/sec
formatter/pydantic/types.py                1.00      2.4±0.04ms    10.7 MB/sec    1.01      2.4±0.02ms    10.6 MB/sec
linter/all-rules/large/dataset.py          1.04     11.6±0.37ms     3.5 MB/sec    1.00     11.1±0.18ms     3.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.05      3.1±0.08ms     5.3 MB/sec    1.00      3.0±0.03ms     5.6 MB/sec
linter/all-rules/numpy/globals.py          1.04   324.2±13.45µs     9.1 MB/sec    1.00    311.6±4.24µs     9.5 MB/sec
linter/all-rules/pydantic/types.py         1.04      5.2±0.18ms     4.9 MB/sec    1.00      5.0±0.06ms     5.1 MB/sec
linter/default-rules/large/dataset.py      1.00      5.9±0.18ms     6.9 MB/sec    1.00      5.9±0.10ms     7.0 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1214.2±12.30µs    13.7 MB/sec    1.00  1216.8±19.48µs    13.7 MB/sec
linter/default-rules/numpy/globals.py      1.01    130.4±2.17µs    22.6 MB/sec    1.00    128.7±1.78µs    22.9 MB/sec
linter/default-rules/pydantic/types.py     1.01      2.7±0.03ms     9.6 MB/sec    1.00      2.6±0.02ms     9.7 MB/sec

@MichaReiser MichaReiser merged commit 3d7411b into main Jun 21, 2023
@MichaReiser MichaReiser deleted the remove-typeid-from-labelid branch June 21, 2023 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter internal An internal refactor or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Release build fails in assert_eq_size! with Rust nightly ≥ 2023-06-09
2 participants