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

Remove superfluous blanket impls in our logging traits #8605

Merged
merged 2 commits into from
Jan 7, 2025

Conversation

teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Jan 7, 2025

Our core logging traits have to be object-safe, which means Self must be unsized, which means all methods must take either a shared or exclusive reference.
This in turns means that any impl Trait for &something is non-sensical, since this lowers to impl Trait for &&self, which just gives extra work to the compiler for no reason since this is semantically identical to the builtin autoderef magic.

@teh-cmc teh-cmc added 🦀 Rust API Rust logging API 🚜 refactor Change the code, not the functionality exclude from changelog PRs with this won't show up in CHANGELOG.md 🪵 Log & send APIs Affects the user-facing API for all languages labels Jan 7, 2025
@teh-cmc
Copy link
Member Author

teh-cmc commented Jan 7, 2025

@rerun-bot full-check

Copy link

github-actions bot commented Jan 7, 2025

Web viewer built successfully. If applicable, you should also test it:

  • I have tested the web viewer
Result Commit Link Manifest
3cb70f1 https://rerun.io/viewer/pr/8605 +nightly +main

Note: This comment is updated whenever you push a commit.

Copy link

github-actions bot commented Jan 7, 2025

Started a full build: https://github.com/rerun-io/rerun/actions/runs/12656028307

@emilk emilk self-requested a review January 7, 2025 18:59
Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good riddance!

@emilk emilk merged commit ebfb6fb into main Jan 7, 2025
72 checks passed
@emilk emilk deleted the cmc/superfluous_blankets branch January 7, 2025 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude from changelog PRs with this won't show up in CHANGELOG.md 🪵 Log & send APIs Affects the user-facing API for all languages 🚜 refactor Change the code, not the functionality 🦀 Rust API Rust logging API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants