Skip to content

Consider migrating to Rust 2024 #1052

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

Open
lilizoey opened this issue Feb 20, 2025 · 3 comments
Open

Consider migrating to Rust 2024 #1052

lilizoey opened this issue Feb 20, 2025 · 3 comments
Labels
breaking-change Requires SemVer bump c: tooling CI, automation, tools quality-of-life No new functionality, but improves ergonomics/internals

Comments

@lilizoey
Copy link
Member

A new rust edition has been stabilized, see this blog post for more details:
https://blog.rust-lang.org/2025/02/20/Rust-1.85.0.html

We should consider if and when to migrate gdext to that edition. For that it'd be useful to read the edition guide: https://doc.rust-lang.org/edition-guide/rust-2024/index.html

This would also require an MSRV bump up to at least 1.85.

Note that some of the edition changes may impact soundness. We should be confident that this wont impact us when we do the migration.

@lilizoey lilizoey added breaking-change Requires SemVer bump quality-of-life No new functionality, but improves ergonomics/internals labels Feb 20, 2025
@Bromeon
Copy link
Member

Bromeon commented Feb 21, 2025

Sounds good, we should probably see how much is impacted, and then decide if/when to migrate. Generally, I'd like the other big PRs to be sorted out first, to avoid busywork with merge conflicts.

@lilizoey
Copy link
Member Author

lilizoey commented Mar 4, 2025

btw i put breaking change just cause it might be a breaking change. in principle changing editions shouldn't be breaking, but we might need to change some stuff in a breaking way to accommodate something.

@Bromeon
Copy link
Member

Bromeon commented Mar 23, 2025

I tried cargo fix --edition just out of curiosity. It's rather illy, it just wraps all unsafe functions with an extra unsafe {...} block around the entire body. As a result, it causes "unnecessary unsafe" warnings for existing unsafe blocks 🤡

unsafe fn xy() { unsafe { 
    // previous code
}}

There are some legitimate warnings about lifetimes in if let scopes, but they're drowned out by the unsafe BS. Would be nice to handle those problems separately.


I still don't like that unsafe_op_in_unsafe_fn doesn't do heuristics, such as "single expressions don't need an extra unsafe block". The lint/warning is probably fine as a default for complex functions, but having many trivial functions like

pub unsafe fn get_interface() -> &'static GDExtensionInterface {
    unsafe { &get_binding().interface }
}

is not adding much, and I'm 100% not starting to add // SAFETY for those.

Maybe we should address #1046 first and then re-evaluate how many trivial unsafe APIs remain. Ideally we can cut down a bit 🪓🙂

@Bromeon Bromeon added the c: tooling CI, automation, tools label Mar 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Requires SemVer bump c: tooling CI, automation, tools quality-of-life No new functionality, but improves ergonomics/internals
Projects
None yet
Development

No branches or pull requests

2 participants