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

Some renaming and minor refactoring for NativeLibraryKind #72309

Merged
merged 3 commits into from
May 22, 2020

Conversation

petrochenkov
Copy link
Contributor

No description provided.

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 17, 2020
/// Windows dynamic library (`foo.dll`) without a corresponding import library (`foo.lib`).
RawDylib,
/// A macOS-specific kind of dynamic libraries.
Framework,
/// The library kind wasn't specified, dynamic linking is currently preferred.
/// The library kind wasn't specified, `Dylib` is currently used as a default.
Copy link
Member

Choose a reason for hiding this comment

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

Hm I'm not sure I quite follow this comment -- when do we encounter Unspecified if dylib is the default? Or does this mean that (as I think) that currently when we encounter an unspecified library kind, we store it as unspecified, but the code treats it equivalently to Dylib?

If that's right, it seems like we might want to adjust this such that Dylib is instead something like Dylib { explicit: bool } perhaps? I'm not sure whether that's worthwhile though given how rarely we add things, but I'd be a bit concerned that over time we may not be actually treating unspecified entirely equivalently to unspecified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unspecified and Dylib behave identically right now, but they may behave differently if new options are added (#39998) or experiments performed (#71647).
So the intent is indeed to add things and this PR is a preparation.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 19, 2020
NativeLibrary(Kind) -> NativeLib(Kind)
NativeStatic -> StaticBundle
NativeStaticNobundle -> StaticNoBundle
NativeFramework -> Framework
NativeRawDylib -> RawDylib
NativeUnknown -> Unspecified
@petrochenkov petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 20, 2020
@matthewjasper
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented May 21, 2020

📌 Commit 8dbe4d9 has been approved by matthewjasper

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 21, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request May 22, 2020
Rollup of 6 pull requests

Successful merges:

 - rust-lang#71607 (clarify interaction of pin drop guarantee and panics)
 - rust-lang#72125 (remove broken link)
 - rust-lang#72133 (Add target thumbv7a-uwp-windows-msvc)
 - rust-lang#72304 (rustc_target: Avoid an inappropriate use of `post_link_objects`)
 - rust-lang#72309 (Some renaming and minor refactoring for `NativeLibraryKind`)
 - rust-lang#72438 (Enable ARM TME (Transactional Memory Extensions))

Failed merges:

r? @ghost
@bors bors merged commit afbbb86 into rust-lang:master May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants