Skip to content

refactor: remove OnceLock #9992

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.

Sign up for GitHub

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

Draft
JonasKruckenberg wants to merge 7 commits into
base: main
Choose a base branch
from
+536 −209
Draft

refactor: remove OnceLock #9992

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
34933d0
feat: make `MachineEnv`s static and remove `OnceLock`s
JonasKruckenberg Jan 13, 2025
416a5f6
Update abi.rs
JonasKruckenberg Jan 13, 2025
9a1ef9c
fixes
JonasKruckenberg Jan 13, 2025
846297f
Update abi.rs
JonasKruckenberg Jan 13, 2025
beeb370
fixes
JonasKruckenberg Jan 13, 2025
080f09d
Update Cargo.toml
JonasKruckenberg Jan 13, 2025
e091faf
Update Cargo.toml
JonasKruckenberg Jan 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fixes
  • Loading branch information
@JonasKruckenberg
JonasKruckenberg committed Jan 13, 2025
commit beeb370e6ce066c8165816d4f94f788f90e115ba
2 changes: 2 additions & 0 deletions cranelift/codegen/src/machinst/reg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,13 @@ pub fn first_user_vreg_index() -> usize {
pub struct Reg(VReg);

impl Reg {
/// Create a register from a physical register
// FIXME(const-hack) remove in favor of `From` impl
Copy link
Member

@cfallin cfallin Jan 13, 2025

Choose a reason for hiding this comment

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

Can you say more here -- I'm not that familiar with the current state of constification of everything; is there some feature this is waiting on to stabilize or...?

Copy link
Contributor Author

@JonasKruckenberg JonasKruckenberg Jan 13, 2025

Choose a reason for hiding this comment

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

well, the feature that would be waiting for is const associated functions in traits (therefore allowing From::from to be const) but that afaik is in "never type land". I can remove the note as it doesn't make much sense to hold our breath on it

Copy link
Member

@cfallin cfallin Jan 13, 2025

Choose a reason for hiding this comment

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

Yeah, I think that's best -- usually when we have a FIXME we want it to be actionable, or at least give sufficient context so that we know what we're waiting on (i.e.: if someone else walked up to this code tomorrow, would they know what they need in order to fix it?). It's fine to evaluate questions like this once new Rust features land in the future.

Copy link
Contributor Author

@JonasKruckenberg JonasKruckenberg Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

the issue with constness is that theres no good way to detect constness workarounds after the fact (ie reevaluating code like this is tricky since it appears as just somewhat odd but regular code) which is the whole reason for the const-hack comments but usually you'd tie them to specific RFCs so yeah i agree getting rid of it is fine

Copy link

@hanna-kruppe hanna-kruppe Jan 14, 2025

Choose a reason for hiding this comment

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

Relevant RFC fresh off the press: rust-lang/rfcs#3762

pub const fn from_preg(preg: PReg) -> Reg {
Reg(RealReg(preg).to_vreg())
}

/// Create a register from a virtual register
pub const fn from_vreg(vreg: VReg) -> Reg {
Reg(vreg)
}
Expand Down