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

--crate-type metadata overrides binary-ness of crates #38273

Closed
nrc opened this issue Dec 9, 2016 · 11 comments
Closed

--crate-type metadata overrides binary-ness of crates #38273

nrc opened this issue Dec 9, 2016 · 11 comments
Assignees

Comments

@nrc
Copy link
Member

nrc commented Dec 9, 2016

If you make a crate metadata, then it treats the crate as a library in all cases. This causes problems because the compiler will think that any non-public code is dead and issue warnings.

I'm not exactly sure the fix here - I don't want to always silence these warnings because they are valid for library crates. OTOH, I don't really want metadata-bin as another crate type, that might be the best thing to do though.

cc @alexcrichton

@alexcrichton
Copy link
Member

Hm this is indeed unfortunate! I'd probably be a fan of looking for a main function to mark it as not dead and otherwise just carrying on as usual.

@nrc
Copy link
Member Author

nrc commented Dec 16, 2016

I don't like the idea of looking for main - what if the user wants it to be a binary and mis-types main? Then we get the wrong errors. We'd also have to take into account #[start] and the other 101 ways to have a main function too.

A few ideas:

  • we change metadata to metadata-lib and metadata-bin
  • we add --emit=nope which cargo would use for the last crate and does not emit metadata, but also treats the crate as a bin
  • we allow multiple crate types, so the compiler could special case metadata,bin (where there is no *lib crate type) to treat the crate as a metadata and binary.
  • crate-type=metadata always assumes a binary wrt dead code (I don't like this because it misses some errors).

cc @rust-lang/compiler @rust-lang/tools

@michaelwoerister
Copy link
Member

We'd also have to take into account #[start] and the other 101 ways to have a main function too.

But the compiler already has a pass for gathering this information, so that would not be an obstacle.

@alexcrichton
Copy link
Member

I'd like to avoid adding two crate types for this at all costs if possible (plus it's already landed so it'd be difficult to change).

This basically only deals with lint warnings, so I think @michaelwoerister has a good point that we can just, if compiling metadata, consider the start function, if any, used. I agree that it shouldn't be too difficult to implement in the compiler as well.

@nrc
Copy link
Member Author

nrc commented Dec 18, 2016

@alexcrichton what do you think of this idea, I think it is my favourite, although the semantics are a little bit non-obvious:

the compiler could special case metadata,bin (where there is no *lib crate type) to treat the crate as a metadata and binary.

@alexcrichton
Copy link
Member

@nrc to clarify, that's for special casing when the compiler has two crate types requested, one for metadata and one for a binary?

If that's true, wouldn't that not solve the cargo check case because Cargo compiles with only --crate-type metadata?

@nikomatsakis
Copy link
Contributor

This basically only deals with lint warnings, so I think @michaelwoerister has a good point that we can just, if compiling metadata, consider the start function, if any, used. I agree that it shouldn't be too difficult to implement in the compiler as well.

I agree it's easy to implement, but I'm not sure it's the right thing. If I understand the context correctly, @nrc is using this in the RLS. This means that if you are building a library and you have dead-code relating to a main function, then when using the IDE, you would not get the same warnings that you get when you run cargo from the command line, right? Isn't that confusing? It seems we should strive for fidelity here.

@alexcrichton
Copy link
Member

@nikomatsakis that is indeed a very good point. The only solution I see to that is indeed having a metadata-lib and a metadata-bin crate type.

Or just considering fn main used for lib crate types.

@brson
Copy link
Contributor

brson commented Dec 21, 2016

Please triage and add a P-tag. This seems important since it involves a recently-added stable interface.

@nrc
Copy link
Member Author

nrc commented Dec 22, 2016

I vote for p-high given it affects stability and we'll need to uplift to beta. I'm working on a fix.

@alexcrichton
Copy link
Member

I believe this is no longer an issue since we moved to --emit metadata

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

No branches or pull requests

5 participants