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

Generic type information missing for items in paths #429

Closed
pragmatrix opened this issue May 27, 2023 · 5 comments · Fixed by #430 or #431
Closed

Generic type information missing for items in paths #429

pragmatrix opened this issue May 27, 2023 · 5 comments · Fixed by #430 or #431
Labels
bug Something isn't working high-prio Something considered more important than most other things.

Comments

@pragmatrix
Copy link

Hi, I was trying to use cargo public-api diffs on skia-safe, which uses generic handle types for a lot of the exported types. Here is an minimum working example that explains the problem:

lib.rs:

pub struct Handle<T>(T);

pub type HU32 = Handle<u32>;

impl HU32 {
    pub fn get_u32() -> u32 {
        0
    }
}

results in

...
pub mod public_api_test
pub struct public_api_test::Handle<T>(_)
impl public_api_test::Handle<u32>
pub fn public_api_test::Handle::get_u32() -> u32
...
pub type public_api_test::HU32 = public_api_test::Handle<u32>

but I I think it the functions should be represented with their complete type signature:

...
pub fn public_api_test::Handle<u32>::get_u32() -> u32

Without that, a diff compares all functions of the generic type instead of only the concrete type.

@Enselic
Copy link
Member

Enselic commented May 28, 2023

Thank you very much for taking the time to report this issue, and especially for your minimal reproducer. I have done some debugging, and the problem seems to be that for items in paths, generic parameters are ignored. Only the name of the item name is used if it is part of a path:

https://github.com/Enselic/cargo-public-api/blob/c1aac48db7796eee2fae308227370331b76fe7f6/public-api/src/render.rs#L249-L254

Clearly a somewhat big bug. I'm surprised it has not been noticed before. Instead of output.push(token_fn(name.to_string())) we probably need to call a higher-level rendering function like fn token_stream(), but refactor the code to avoid recursion.

@Enselic Enselic changed the title Functions implemented on concrete generic types are rendered without generic type information. Generic type information missing for items in paths May 28, 2023
@Enselic Enselic added bug Something isn't working high-prio Something considered more important than most other things. labels May 28, 2023
@Enselic
Copy link
Member

Enselic commented May 31, 2023

Oops, GitHub thought #430 was a fix. It wasn't...

@Enselic
Copy link
Member

Enselic commented May 31, 2023

PR with fix: #431

@pragmatrix If you want to test the fix already now on your real code you can do like this:

  1. Check out my branch with the fix
  2. Run the below command (but change Cargo.toml path of course)
$ cargo run -- --manifest-path ~/src/lib/Cargo.toml --omit blanket-impls,auto-trait-impls,auto-derived-impls
pub mod lib
pub struct lib::Handle<T>(_)
impl lib::Handle<u32>
pub fn lib::Handle<u32>::get_u32() -> u32
pub type lib::HU32 = lib::Handle<u32>

@Enselic
Copy link
Member

Enselic commented Jun 3, 2023

Fixed in https://crates.io/crates/cargo-public-api/0.31.0

@pragmatrix
Copy link
Author

Thank you a lot! I have tried version 0.31.0 and the results were great. I am now able to diff the API surface of skia-safe as a pre-release check and immediately found some inconsistencies in the current master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high-prio Something considered more important than most other things.
Projects
None yet
2 participants