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

[RFC] externally definable statics #3635

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

m-ou-se
Copy link
Member

@m-ou-se m-ou-se commented May 13, 2024

@m-ou-se m-ou-se added the T-lang Relevant to the language team, which will review and decide on the RFC. label May 13, 2024
@davidhewitt
Copy link
Contributor

This strikes me a lot as a simpler form of "distributed slice" a la dtolnay's linkme.

The extensible nature of such a slice would solve more use cases over setting a single value. An easy to see motivation is for custom benchmark or test harnesses to collect cases together via proc macros. Another use case familiar to me is for PyO3 to support collecting methods from multiple impl blocks annotated with #[pymethods] to expose to Python.

Perhaps that can be mentioned as a future possibility to this RFC?

@bjorn3
Copy link
Member

bjorn3 commented May 13, 2024

There is a big difference between this RFC and distributed slices: This RFC is trivially implementable across pretty much all linkers in existence. (excluding the default implementation support) As far as linkers are concerned there isn't really a difference between crate_a referencing a symbol and crate_b implementing it (an externally defined static) and vice versa (a regular static referenced from another crate) (except for cycles with static libraries, for which some linkers require --start-group/--end-group or mentioning the static libraries twice) Distributed slices however require either platform specific mechanisms to either concatenate sections or run static constructors or they need rustc to wrap the linker such that it can generate a definition for the distributed slice based on the crate metadata of all dependencies.

Adding distributed slices as future possibility to this RFC is fine with me.

@davidhewitt
Copy link
Contributor

That is indeed how linkme currently implements them; I was hoping that a first-class language feature may be able to find an alternative implementation pathway instead of reliance on the platform & linker. Perhaps that's wishful thinking.

@kennytm
Copy link
Member

kennytm commented May 13, 2024

OK I forgot one thing about #[global_allocator]: the type of the static is not a fixed type but anything that implements GlobalAlloc. Meaning you need either one of the following to reproduce #[global_allocator]

// (1) type the static as `dyn Trait`, 
// perhaps the most faithful representation but perhaps requires unsized_locals (rust-lang/rust#48055)
extern static ALLOCATOR: dyn GlobalAlloc;
impl static ALLOCATOR: dyn GlobalAlloc = SimpleAllocator { ... };

// (2) type the static as `&dyn Trait`,
// incurring double reference though.
extern static ALLOCATOR: &dyn GlobalAlloc;
impl static ALLOCATOR: &dyn GlobalAlloc = &SimpleAllocator { ... };

// (3) type the static as `impl Trait`, 
// maybe not linker-friendly? and not sure if non-object-safe traits can really be used.
extern static ALLOCATOR: impl GlobalAlloc;
impl static ALLOCATOR: impl GlobalAlloc = SimpleAllocator { ... };

@fbstj
Copy link
Contributor

fbstj commented May 13, 2024

maybe a silly question: is this not similar to weak linkage?

@bjorn3
Copy link
Member

bjorn3 commented May 13, 2024

In the case where there is a default, yes weak linkage is a possible implementation strategy. For the case where there is no default, it is no different from a regular extern reference in C. No weak linkage need and should be involved as linking must fail if the function is not provided.

@jdonszelmann
Copy link

@davidhewitt Distributed slice, what you're talking about is actually an open issue in the testing devex team: rust-lang/testing-devex-team#3. I'm currently (slowly) working towards a proposal for that: https://internals.rust-lang.org/t/global-registration-a-kind-of-pre-rfc/20813/23. I'm afraid it will take a little, cause we'll likely do some experimentation in a T-lang experiment. That's distinct from this issue.

About this rfc: I think I liked the other one slightly better. I think that these externally definable statics are very often going to be functions, and having to use closure syntax for them all the time might be a bit awkward. I guess how Mara phrased it in the alternative RFC: that one might be a bit more ergonomic, and I like that. I like having both functions and statics be available to define externally.


# Unresolved questions

- Should we allow some form of subtyping, similarly to how traits allow trait impls to do subtyping?
Copy link
Member

Choose a reason for hiding this comment

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

I think we should, but I think it'd be fine to move this to "future work" rather than requiring this in the initial version. It's a backwards-compatible addition, and not allowing subtyping for now doesn't close the door for doing it in the future.

@Evian-Zhang
Copy link

It appears to me that the externally defined statics and functions are generalized form of "associated consts" and "required methods", generalized from trait to crate. The root library crate behaves like a trait to define those items, and the leaf binary crate behaves like a struct to implement such items. Is it better to make these two semantics have similar syntax to make it more consistent?

@m-ou-se m-ou-se changed the title [RFC] externally definable statics [RFC] externally definable statics May 22, 2024
facebook-github-bot pushed a commit to facebook/buck2 that referenced this pull request Oct 9, 2024
Summary:
`buck2_bxl` crate is heavy, and having fewer dependencies speeds up incremental compilation.

If this [RFC in Rust](rust-lang/rfcs#3635) will ever be implemented, we will reduce the amount of complexity in code like this.

Initially I wanted to do it to push attribute configuration to downstream crates from `buck2_node` for D63918028, but then decided not to do it for a while. So this diff is not required for D63918028.

Reviewed By: JakobDegen

Differential Revision: D63929039

fbshipit-source-id: 5360c5bb774a4d79471e6dad0ce2c1ba54a6b57b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants