Skip to content

Add Library::new_with_flags #159

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

Closed
wants to merge 1 commit into from
Closed

Conversation

tgross35
Copy link

@tgross35 tgross35 commented Oct 3, 2024

Sometimes it would be convenient to keep the safer lifetime-bound API, but adjust the way the library gets loaded (e.g. to use eager binding). Add Library::new_with_flags and a LoadFlags type to provide an easier way to do this.

Sometimes it would be convenient to keep the safer lifetime-bound API,
but adjust the way the library gets loaded (e.g. to use eager binding).
Add `Library::new_with_flags` and a `LoadFlags` type to provide an
easier way to do this.
@nagisa
Copy link
Owner

nagisa commented Oct 3, 2024

Converting a os::Library into plain Library with an .into() seems pretty straightforward to me, so the safer API is within arm's reach at all times.

The fact that LoadFlags is a OS dependent typedef seems iffy -- code written in a portable seeming way isn't actually portable. More generally I consider that to be a major footgun with exposing any sort of exposition of OS-specific details outside the os::* modules and unless there is a very very good reason I would rather not break the os::* encapsulation. At least not until rust-lang/rust#41619 is implemented and libstd (and thus the ecosystem) expectations change with regards to what looks "portable".

@tgross35
Copy link
Author

tgross35 commented Oct 3, 2024

I don't know why I didn't consider just .intoing it. That's way easier, thanks!

@tgross35 tgross35 closed this Oct 3, 2024
@tgross35 tgross35 deleted the new-with-flags branch October 3, 2024 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants