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

Allow extern crate to take a list of crates #1875

Closed
wants to merge 1 commit into from

Conversation

sbeckeriv
Copy link

@sbeckeriv sbeckeriv commented Jan 29, 2017

Rendered

Dearest Reviewer,

I hope all is well. I have been enjoying rust immensely. Recently I was
working on converting an old ruby project to rust. I was adding my 20ish
crate to my project and after scrolling down I thought, "Do I need all the
extern crate lines?".

I set out by making a macro, my first, which was easy and fun. I posted
my macro to reddit wondering if I had missed something. After many
great comments someone suggested an rfc with a wink
face1.

I figured why not. I wrote up this start and then went over to
internals.rust-lang.org.
Here I received advice on syntax and missing cases to my rfc. Excellent
feedback again!

I hope its considered to allow one line of extern crate to accept many
crates. I understand completely if it is not.

Thanks again
Becker

@andylokandy
Copy link

@sbeckeriv I think the macro should be sufficient, and it should be provided by a lib.
Because I rarely have more than six depending crates, and this style could make git diff unnecessarily vague.

@sbeckeriv
Copy link
Author

@goandylok I understand its not very useful for projects with small amounts of crates. My recent small project had 16 externs, cargo currently has 20, librustc_driver has 21 and some servo libs have over 50. I would be interested in more data but I am unsure how I would gather it.

I listed the diff issue in the drawbacks. There have been suggestions on how to make it better in git diff but not everyone uses git. Using the new block syntax you could put each crate on its own line. This doesn't make the list compact but it does remove repeated extern creates.

Thanks
Becker

@Diggsey
Copy link
Contributor

Diggsey commented Jan 30, 2017

This seems quite sensible and sufficiently consistent with use to not be surprising to users.

@nrc nrc added the T-lang Relevant to the language team, which will review and decide on the RFC. label Jan 30, 2017
@nrc
Copy link
Member

nrc commented Jan 30, 2017

Seems like a win to me, albeit a minor one (I can't say multiple extern crates have bothered me, and editors should be able to hide them). I think having a macro crate to implement this is a good solution and it is not necessary to support this in the language (I wouldn't object to adding it though).

@nrc nrc changed the title RFC: Boil Down Externs Allow extern crate to take a list of crates Jan 30, 2017
@est31
Copy link
Member

est31 commented Jan 30, 2017

👍 I like this change. Its minor but still an improvement, and much better than suggestions by other people who suggested to remove extern crate entirely when cargo is being used (wouldn't like that to happen).

@strega-nil
Copy link

strega-nil commented Jan 31, 2017

@Diggsey It's also not consistent with mod. I think of extern crate as much closer to mod than use.

I dislike this change. It's confusing, it drops readability, diffability, and grepability, and I don't think it gives enough of a benefit. You write extern crate once for each dependency, throughout your entire project. I don't think it's a big deal to actually write out extern crate.

use groups like things together, with list imports; extern crate would just bring together crates that have nothing in common.

Basically, I don't think it's a good change. I wouldn't want mod { ... } to create multiple mods from multiple files, either.

@sbeckeriv
Copy link
Author

sbeckeriv commented Jan 31, 2017

@ubsan I left grepability out of the list of drawbacks because extern create is mostly used in a single file. Searching should be a little easier since the crate names will be in a compact list.

Crates can be related. The list format could be used to group them together. I will try to add this idea to the RFC.

Thanks
Becker

@ahicks92
Copy link

I don't like this. If we're going to try to make the crate system easier to use, we should just move to the implicit extern idea.

There should only be one place in which information is placed: if you are depending on a crate in cargo.toml and not then doing extern crate then you shouldn't be depending on the crate in the first place. Ergo, having Cargo just do it seems perfectly viable to me.

I don't dislike this because of syntax, I dislike it because it feels like a stopgap measure until we do do the automatic extern crate thing, which I consider inevitable.

@withoutboats
Copy link
Contributor

withoutboats commented Feb 1, 2017

As @ubsan noted, the analog to extern crate is not use but mod, which does not support this kind of syntax. Though mod has a reason it can't allow this syntax which extern crate does not, the benefit of allowing this syntax doesn't seem to outweigh the inconsistency to me.

And, like @camlorn, I think there's a good change we will make adding the extern crate optional in the near future (and I'd be happy to mentor anyone interested in writing that RFC).

@strega-nil
Copy link

Note that I will also disagree with implicit extern. I don't think that it's useful enough either.

@ahicks92
Copy link

ahicks92 commented Feb 1, 2017

@ubsan
Why? Would it be better to go the other way, and get cargo.toml dependencies from extern? I'm not sure what the possible objection here is.

I don't think we will do it because it is a significant improvement for Rust programmers: putting something that explicitly names the crate in two places is annoying, but way less annoying than the least annoying bits of C++. However, it is a learnability point, putting the same info in two places is an anti-pattern, and it should not be hard to specify or implement.

Maybe this is a bit off topic.

@sbeckeriv
Copy link
Author

I see someone was assigned and then unassigned. I updated the rfc drawbacks with the concerns around future crate improvements and the differing opinions around use/mod/extern.

The conversation here seems to have died off. It appears to be a small improvement. Do concerns around formatting and its constancy related to mod/use out weigh the small improvement?

Thanks everyone for your input!
Becker

@liigo
Copy link
Contributor

liigo commented Apr 15, 2017

the { } is redundant in crate list (ie. extern crate {diesel, diesel_codegen, serde, serder_json}; in this rfc), just write it as:

extern crate diesel, diesel_codegen, serde, serder_json;

It is consistent with future use/mod items:

use io, ptr, mem::{self, transmute};
mod a, b, c;

@nathanjent
Copy link

There are extern blocks to list FFI functions.

extern { 
  fn foo(x: i32, ...); 
}

Maybe have something similar for crates.

extern {
  crate bar;
  #[macro_use]
  crate baz; 
}

@sbeckeriv sbeckeriv force-pushed the master branch 2 times, most recently from 5e342b4 to 4afb6de Compare April 27, 2017 23:29
Dearest Reviewer,

I hope all is well. I have been enjoying rust immensely. Recently I was
working on converting an old ruby project to rust. I was adding my 20ish
crate to my project and after scrolling down I thought, "Do I need all the
`extern crate` lines?".

I set out by making a macro, my first, which was easy and fun. I posted
my macro to reddit wondering if I had missed something. After many
great comments someone suggested an rfc with a wink
face[1](https://www.reddit.com/r/rust/comments/5q7ssv/long_list_of_externs/).

I figured why not. I wrote up this start and then went over to
[internals.rust-lang.org](https://internals.rust-lang.org/t/pre-rfc-boil-down-externs/4676).
Here I received advice on syntax and missing cases to my rfc. Excellent
feedback again!

I hope its considered to allow one line of `extern crate` to accept many
crates. I understand completely if it is not.

Thanks again
Becker

Updated with alt-syntax
@sbeckeriv
Copy link
Author

@liigo I added the suggestion to the rfc. Thanks!

@aturon
Copy link
Member

aturon commented Apr 29, 2017

@withoutboats ping on this RFC. I suspect that, given the questions in the air about where we want to take the module system within the ergonomics initiative, we should probably postpone this for the time being. What do you think?

@withoutboats
Copy link
Contributor

@rfcbot fcp postpone

I agree; for now a macro seems sufficient.

@rfcbot
Copy link
Collaborator

rfcbot commented Apr 29, 2017

Team member @withoutboats has proposed to postpone this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot
Copy link
Collaborator

rfcbot commented Jun 15, 2017

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label Jun 15, 2017
@rfcbot
Copy link
Collaborator

rfcbot commented Jun 25, 2017

The final comment period is now complete.

@aturon
Copy link
Member

aturon commented Jul 3, 2017

Closing as postponed. Thanks @sbeckeriv! We'll look forward to a more general RFC on extern crate in the near future (if anyone's interested in taking that on, I'm happy to mentor).

@aturon aturon closed this Jul 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. 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.