dtolnay / inventory

Typed distributed plugin registration
Apache License 2.0
948 stars 43 forks source link

`const` requirement breaks some use cases #44

Closed mathstuf closed 2 years ago

mathstuf commented 2 years ago

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 commented 2 years ago

@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 https://github.com/cucumber-rs/cucumber/pull/158#issuecomment-969056199 for more info.

mathstuf commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Thanks! I've just merged it.

Diggsey commented 2 years ago

@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 commented 2 years ago

See this commit for the rationale: b853350a3800e38d2cb9950355b80bc8b8d3959c

mathstuf commented 2 years ago

Sorry, the PR has the explanation: #43

Diggsey commented 2 years ago

I see.