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

Add static selectors #104

Merged
merged 30 commits into from
Jun 23, 2022
Merged

Add static selectors #104

merged 30 commits into from
Jun 23, 2022

Conversation

madsmtm
Copy link
Owner

@madsmtm madsmtm commented Jan 2, 2022

My work on fixing upstream SSheldon/rust-objc#49.

A lot of work has gone into this, by many different people, see in particular the following:

This implementation is a combination of the following (git co-author attribution given where relevant):

I've cut down heavily on procedural macro usage, because I think the code is much more readable without (ideally we'll find a way to get rid of them entirely).

TODO:

  • Set up ObjFW CI (ObjFW support #117) Postponed.
  • More test cases in CI, including at least different linking tests, testing in generic functions, and perhaps also UI tests (Use trybuild for testing compilation failures #35) and LLVM/Assembly/... output
  • Set up WinObjC CI Deferred
  • Make this work on GNUStep Deferred
  • Read LLVM, LLD and clang docs and sources
  • Read objc4 sources so see how the loading happens
  • Maybe make necessary changes to Rust to fix the linked issues with generics, incremental compilation and codegen units? Deferred, not really sure how it would be accomplished
  • Should we verify asciiness and such of the selectors like objrs does? Deferred, documented as ill-supported for now.

See also:

Comment on lines 106 to 110
// TODO: `::core` here could be replaced with some more
// sophisticated logic so we don't rely on downstream users having
// this setup.
Copy link
Contributor

Choose a reason for hiding this comment

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

You can re-export core in your crate and then access it from this macro as e.g. $crate::_core.

Copy link
Owner Author

@madsmtm madsmtm Jan 10, 2022

Choose a reason for hiding this comment

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

Yeah I know, I was just carrying over a TODO from previous authors. Thanks for the review though!

@madsmtm madsmtm mentioned this pull request Jan 14, 2022
@madsmtm madsmtm mentioned this pull request Mar 1, 2022
@madsmtm
Copy link
Owner Author

madsmtm commented Mar 29, 2022

The defmt crate also hashes source code locations and puts them in export_name - so that approach is at least used elsewhere.

@madsmtm madsmtm marked this pull request as ready for review June 23, 2022 18:47
@madsmtm
Copy link
Owner Author

madsmtm commented Jun 23, 2022

I think I'm gonna merge this as-is now, allowing users to enable and test this with the "unstable-static-sel" feature flag. It's still a bit rough on the edges, and involves a great deal of hacks and underspecified parts of the compiler, but I'm fairly certain it works.

The primary blocker for adopting this fully (in its most optimal form "unstable-static-sel-inlined", which is the whole point) is the Rust issue rust-lang/rust#53929 on how private statics aren't in the same codegen unit as the code that references them, which I don't really see a way forward on.

Also, we would probably have to have some parts of how the compiler works be more fully specified before we could really ensure that this is stable.

@madsmtm madsmtm merged commit 61b0a97 into master Jun 23, 2022
@madsmtm madsmtm deleted the static-sel branch June 23, 2022 19:06
@madsmtm madsmtm added the A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates label Jun 23, 2022
@madsmtm madsmtm mentioned this pull request Jul 5, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants