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

const requirement breaks some use cases #44

Closed
mathstuf opened this issue Nov 27, 2021 · 8 comments
Closed

const requirement breaks some use cases #44

mathstuf opened this issue Nov 27, 2021 · 8 comments

Comments

@mathstuf
Copy link
Contributor

Unfortunately, this ends up breaking some use cases that I don't think are problematic. Maybe this needs some const fn enhancements or there's some clever way around it.

The problem I have is that fn pointers cannot be used, so I cannot pass in a monomorphized generic function pointer to the constructor of the type in the registry. Since this is a generic function and the types which are submitted to the inventory aren't known to the consumer of it (otherwise, there'd be no point of the inventory!), I'm not sure how to best hide this monomorphization step.

Crate code in question: https://gitlab.kitware.com/utils/rust-git-checks/-/blob/master/git-checks-config/src/registry.rs

@ilslv
Copy link

ilslv commented Nov 27, 2021

@mathstuf It looks like we've encountered similar problem in cucumber crate. Possible workaround may be declaring const F: fn(_) -> _: callback; and using it instead of callback itself in const fn. See cucumber-rs/cucumber#158 (comment) for more info.

@mathstuf
Copy link
Contributor Author

Hrm, that doesn't seem so easy in my case since I am not using procedural macros and instead either have to expose structure innards for literal definitions or have an API which just can't take a fn() as a parameter if made const fn. I'll keep poking at it though.

@dtolnay
Copy link
Owner

dtolnay commented Nov 27, 2021

The fn pointer restriction in const fn signatures is rarely problematic because in struct fields they're allowed, so the only necessary change is to package them into a struct. I put up https://gitlab.kitware.com/utils/rust-git-checks/-/merge_requests/179/diffs showing the change.

@mathstuf
Copy link
Contributor Author

Thanks! I've just merged it.

@Diggsey
Copy link

Diggsey commented Nov 28, 2021

@dtolnay I was using TypeId::of::<T> with the previous version of inventory, and this seems impossible now 😢

Was the change done to remove the type parameter from Registry, or was there some other reason to restrict this to consts?

@mathstuf
Copy link
Contributor Author

See this commit for the rationale: b853350

@mathstuf
Copy link
Contributor Author

Sorry, the PR has the explanation: #43

@Diggsey
Copy link

Diggsey commented Nov 28, 2021

I see.

Repository owner locked and limited conversation to collaborators Dec 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants