Skip to content

add ptr getter method #37

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

Merged
merged 2 commits into from
Mar 26, 2025
Merged

Conversation

robamu
Copy link
Contributor

@robamu robamu commented Mar 25, 2025

I think I used this to verify the correctness of the pointer offset for a generic HAL API.

@robamu robamu changed the title add ptr getter methods add ptr getter method Mar 25, 2025
@robamu
Copy link
Contributor Author

robamu commented Mar 25, 2025

Oh, a MSRV problem, and you suggested a tool that solves this I think...

@robamu
Copy link
Contributor Author

robamu commented Mar 25, 2025

This is odd. Apparently, on older Rust compiler, I am leaking a private type. I just wonder why this is not an issue on newer versions of Rustc

@robamu
Copy link
Contributor Author

robamu commented Mar 25, 2025

Okay, this has a caveat: The generated code now required the rustversion dependency. This can be documented, similarly to how svd2rust documents the required dependencies of the generated code. This would allow the derive mmio crate to add new features with a higher MSRV without actually having to bump the MSRV. What do you think?

@robamu robamu force-pushed the ptr-getter-method branch from f83b0c6 to 5865604 Compare March 25, 2025 17:33
@robamu robamu force-pushed the ptr-getter-method branch from 5865604 to cd716e4 Compare March 25, 2025 21:10
@jonathanpallant
Copy link
Collaborator

Oh, is this because it's a pub function that returns a type that might not be pub?

@jonathanpallant
Copy link
Collaborator

jonathanpallant commented Mar 26, 2025

Maybe this API should be hidden behind an attribute, people can generate code that doesn't use rustversion if they prefer?

I'm still confused why it was OK in 1.74.1 but not in 1.74 in 1.74 but not in 1.73.

Maybe related to https://blog.rust-lang.org/2023/11/16/Rust-1.74.0.html?

@jonathanpallant
Copy link
Collaborator

Lukas suggests:

let vis = &input.vis;
...
quote! {

            /// Retrieve the base pointer for this MMIO handle.
            #[inline]
            $vis const unsafe fn ptr(&self) -> *mut #ident {
                self.ptr
            }
}

@robamu
Copy link
Contributor Author

robamu commented Mar 26, 2025

I see you already created an issue in rust-lang/rust#138982 which might explain this

@jonathanpallant
Copy link
Collaborator

Well they say "yes, we changed how the errors are reported".

So we probably need to control whether the function is public based on whether the return type is public.

@robamu
Copy link
Contributor Author

robamu commented Mar 26, 2025

The suggestion by Lukas worked.. I think this might be the best fix. About the rustversion dependency: With this,
the field pointer getters (#25 (comment)) could be made const again, hidden behind a #[rustversion::attr(since(1.83), const)] . But I think this is a separate PR now.

@jonathanpallant jonathanpallant merged commit 9e59332 into knurling-rs:main Mar 26, 2025
15 checks passed
@jonathanpallant
Copy link
Collaborator

Thank you!

@robamu
Copy link
Contributor Author

robamu commented Mar 26, 2025

Oh, I think an unneded dependency slipped in

@robamu robamu deleted the ptr-getter-method branch March 26, 2025 16:14
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.

2 participants