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

Begin the dissolution of libextra #11787

Merged
merged 2 commits into from Jan 27, 2014
Merged

Conversation

alexcrichton
Copy link
Member

It was decided a long, long time ago that libextra should not exist, but rather its modules should be split out into smaller independent libraries maintained outside of the compiler itself. The theory was to use rustpkg to manage dependencies in order to move everything out of the compiler, but maintain an ease of usability.

Sadly, the work on rustpkg isn't making progress as quickly as expected, but the need for dissolving libextra is becoming more and more pressing. Because of this, we've thought that a good interim solution would be to simply package more libraries with the rust distribution itself. Instead of dissolving libextra into libraries outside of the mozilla/rust repo, we can dissolve libraries into the mozilla/rust repo for now.

Work on this has been excruciatingly painful in the past because the makefiles are completely opaque to all but a few. Adding a new library involved adding about 100 lines spread out across 8 files (incredibly error prone). The first commit of this pull request targets this pain point. It does not rewrite the build system, but rather refactors large portions of it. Afterwards, adding a new library is as simple as modifying 2 lines (easy, right?). The build system automatically keeps track of dependencies between crates (rust and native), promotes binaries between stages, tracks dependencies of installed tools, etc, etc.

With this newfound buildsystem power, I chose the extra::flate module as the first candidate for removal from libextra. While a small module, this module is relative complex in that is has a C dependency and the compiler requires it (messing with the dependency graph a bit). Albeit I modified more than 2 lines of makefiles to accomodate libflate (the native dependency required 2 extra lines of modifications), but the removal process was easy to do and straightforward.


Testing-wise, I've cross-compiled, run tests, built some docs, installed, uninstalled, etc. I'm still working out a few kinks, and I'm sure that there's gonna be built system issues after this, but it should be working well for basic use!

cc #8784

@huonw
Copy link
Member

huonw commented Jan 25, 2014

flate needs to be listed in some documentation index somewhere, or we lose it "completely" (except for already those in the know about it).

@alexcrichton
Copy link
Member Author

That is true, I think that rustdoc is going to need a broader solution for integrating the documentation of multiple crates together. For now, we can try to update doc/index.md, but there's no verification or auto-generation of this, so it's highly likely to fall out of date quickly.

The good news is that this will continue to generate, test, and upload documentation, you'll just have to know where to go to find it (for now)

@derekchiang
Copy link
Contributor

Just to be clear, if after this PR lands I want to use flate (or any other module in libextra), I will need to do something like:

extern mod flate;
use flate::something;

Correct?

@alexcrichton
Copy link
Member Author

That's the idea!

@derekchiang
Copy link
Contributor

For future visitors to this page, could you explain why people wanted to dissolve libextra?

@alexcrichton
Copy link
Member Author

The current pattern of the rust compiler is to have very large libraries with lots and lots of modules inside them. This has become less than desirable for a few reasons

  • Large crates lead to higher compile-times.
  • Large crates fail to have a clear separation of functionality between modules (because modules are cyclical)
  • It's tough to easily get at "just the functionality you want" from a large crate.

Essentially, whenever you link to a crate, you should be planning on using the majority of the crate. Currently, this is not at all the case of libextra where each module is very different in functionality than the next. All of the contents of libextra are quite valuable, but they're arguably more valuable on their own. Functionality like the time module could be vastly expanded into very high quality bindings to everything related to time, but not all this functionality belongs in a "conglomerate crate".

We would like to start fostering a pattern of smaller, more focused crates than we've been doing in the past, starting with libextra being broken up. These smaller crates will allow for greater individual expansion in the future instead of maintaining one monolithic crate.

Essentially, we want to break up libextra in order to foster futher innovation of crates. Development times will improve (smaller crates), more functionality can come in (not a giant crate to worry about), and existing functionality can be refined more quickly (smaller focus when dealing with a single nugget of functionality).

I may be leaving something out, but that's the general idea in my mind at least.

@thestinger
Copy link
Contributor

Something to keep in mind is that generic functions/containers don't slow down noticeably compile-time if they aren't used. The compile-time depends on the number of instantiations, which is going to go up if there are many crates reusing the same generic code.

@huonw
Copy link
Member

huonw commented Jan 25, 2014

Development times will improve (smaller crates)

Also, crates that the compiler doesn't use will be exponentially improved, since they will not require a full bootstrap when changed.

@liigo
Copy link
Contributor

liigo commented Jan 25, 2014

before this dissolution:

extern mod extra;
use extra::a::x;
use extra::b::x;
use extra::c::x;
use extra::d::x;

after this dissolution:

extern mod a;
extern mod b;
extern mod c;
extern mod d;
use a::x;
use b::x;
use c::x;
use d::x;

You see, more lines (extern mod ...) need to write.

Before module system being improved (Issue #11745), I wouldn't like to see landing this PR personally.

@adrientetar
Copy link
Contributor

@liigo You're missing the point.

@Dretch
Copy link
Contributor

Dretch commented Jan 25, 2014

@liigo

I see your point, but i think it is worth mentioning that you will only need the extra extern mods in one place, not in every file in your crate.

@brson
Copy link
Contributor

brson commented Jan 26, 2014

Thank you so much for starting this. You are my hero.

@brson
Copy link
Contributor

brson commented Jan 26, 2014

@ligo I think that should not be the case since promoting extra mods to crates will raise the nesting up a level, i.e instead of:

extern mod extra;
use extra::flate;

You'll write

extern mod flate;

In either case, afterward you are just using flate::foo.

In submodules where you previously wrote use extra::flate, you can now write either use flate or just repeat extern mod flate, for the same number of lines.

@brson
Copy link
Contributor

brson commented Jan 26, 2014

My own perspective on why we're doing this:

It's long been a consensus that smaller libraries are better for Rust's compilation model, and we have a strong desire to not have a large, monolithic, and in some ways inflexible standard library (like Java). So we want to have a strong ecosystem of easily accessible and featureful crates, some of which are officially supported and part of the 'standard rust distribution'.

So the standard library is intended to be small and focused on core language and runtime infrastructure, but the standard libraries should be expansive.

We're having a hard time getting to this stage because of the delays in finishing rustpkg. Rust gets a lot of pull requests for interesting library functionality, and we want to encourage that while also resisting piling everything into std, so as an interim solution to support the continued expansion of Rust's libraries we need to get libextra broken up so that the library organization begins to look like our our eventual goals, and we get experince with the many-crates model.

@liigo
Copy link
Contributor

liigo commented Jan 26, 2014

Rust's design is oriented toward concerns of “programming in the large”. (from rust-lang.org) Is it right?
In a large project, i use not only one crate flate, there are many more: arc, btree, json, ... (they're all belongs to extra currently). @brson
Before:

extern mod extra;
use extra::flate::*;
use extra::arc::*;
use extra::btree::*;
use extra::json::*;

After extra's dissolution:

extern mod flate;  //!! More `extern mod xxx` need to write
extern mod arc;    //!! More crates, more lines.
extern mod btree;
extern mod json;
use flate::*;
use arc::*;
use btree::*;
use json::*;

@huonw
Copy link
Member

huonw commented Jan 26, 2014

The conventional style is for

extern mod extra;
use extra::json;
use extra::flate;

fn main() { flate::foo(); json::bar(); }

Under this scheme

extern mod json;
extern mod flate;

fn main() { flate::foo(); json::bar(); }

@brson
Copy link
Contributor

brson commented Jan 26, 2014

@ligo Yes, Rust is concerned with 'programming in the large', and yes, your example is correct.

Do you have a suggestion on how to reduce the amount of syntax here?

@brson
Copy link
Contributor

brson commented Jan 26, 2014

@ligo I'd suggest that this problem is relatively minor at scale. Yes, for small projects where there are no submodules, you may have some extra use statements, but this doesn't change the number of lines of code necessary in submodules.

@brson
Copy link
Contributor

brson commented Jan 26, 2014

Great diffstats on this: 544 additions and 1,039 deletions.

@liigo
Copy link
Contributor

liigo commented Jan 26, 2014

@brson Yes, I have: make the extern mod extra; optional.

When programmers write use extra::arc::Arc, rustc always knows he want to use crate extra, then why he still have to write extern mod extra;? It should be optional, unless compiler got ambiguity.

I've leave comments to #11745 yesterday. This is liigo not ligo, thank you.

@brson
Copy link
Contributor

brson commented Jan 26, 2014

I like that this apparently replaces the .dummy files with stamp. - it's a little less embarrassing.

@brson
Copy link
Contributor

brson commented Jan 26, 2014

@liigo Perhaps you can open a new issue on just that idea. It seems worth exploring, but I don't want it to get buried in #11745.

@liigo
Copy link
Contributor

liigo commented Jan 26, 2014

brson: OK, I will.

@huonw : for rust-style source code, you need use type_or_functon, not use submod. Maybe i'm wong?
mozilla/rust repo always use type_or_functon.

@huonw
Copy link
Member

huonw commented Jan 26, 2014

Yes, use submod is perfectly ok.

@liigo
Copy link
Contributor

liigo commented Jan 26, 2014

mozilla/rust repo always use type_or_functon. i think it's right style direction.

@huonw
Copy link
Member

huonw commented Jan 26, 2014

No, it really doesn't. Random example: everything except File, MemWriter, Decodable and Encodable are modules.

@liigo
Copy link
Contributor

liigo commented Jan 26, 2014

It is minority exceptional case at all.

@brson
Copy link
Contributor

brson commented Jan 26, 2014

Well, this is just amazing.

It's impossible to guess whether this is correct or not since the makefiles are so blindingly impenetrable, so we're just going to have to push it through and see what happens, but this is a massive improvement to the design of the build system.

@huonw
Copy link
Member

huonw commented Jan 26, 2014

@liigo I'm sorry, but it's not. Even the style guide says "Prefer to fully import types while module-qualifying functions" (my emphasis).

Some basic statistics: the first is counting the number of occurrences of use std::<name>; (importing a module) the second is use std::<name>:: (i.e. probably importing a function) and the last is use std::<name>::<lowercase letter> (same as the second, but filtering out imports like use std::cell::Cell; which are types)

$ git grep 'use std::[^:]*;' | wc -l
936
$ git grep 'use std::[^:]*::' | wc -l
1096
$ git grep 'use std::[^:]*::[^A-Z]'  | wc -l
650

So, one certainly cannot say that using a module is rare. Also note that the second number includes imports like use std::io::fs; (a nested submodule) and lines like use std::cell::Cell (types), so it is overcounting the number of function import lines. The third number is an approximate count excluding the types, but it still includes the nested module imports and imports like use std::io::{File, BufferedReader}, and so is also an overestimate of the actual function import lines.

(In any case, this discussion is offtopic for this PR.)

@liigo
Copy link
Contributor

liigo commented Jan 26, 2014

@huonw : Out of topic. I opens a new issue #11811.

Before this patch, if you wanted to add a crate to the build system you had to
change about 100 lines across 8 separate makefiles. This is highly error prone
and opaque to all but a few. This refactoring is targeted at consolidating this
effort so adding a new crate adds one line in one file in a way that everyone
can understand it.
This is hopefully the beginning of the long-awaited dissolution of libextra.
Using the newly created build infrastructure for building libraries, I decided
to move the first module out of libextra.

While not being a particularly meaty module in and of itself, the flate module
is required by rustc and additionally has a native C dependency. I was able to
very easily split out the C dependency from rustrt, update librustc, and
magically everything gets installed to the right locations and built
automatically.

This is meant to be a proof-of-concept commit to how easy it is to remove
modules from libextra now. I didn't put any effort into modernizing the
interface of libflate or updating it other than to remove the one glob import it
had.
bors added a commit that referenced this pull request Jan 27, 2014
It was decided a long, long time ago that libextra should not exist, but rather its modules should be split out into smaller independent libraries maintained outside of the compiler itself. The theory was to use `rustpkg` to manage dependencies in order to move everything out of the compiler, but maintain an ease of usability.

Sadly, the work on `rustpkg` isn't making progress as quickly as expected, but the need for dissolving libextra is becoming more and more pressing. Because of this, we've thought that a good interim solution would be to simply package more libraries with the rust distribution itself. Instead of dissolving libextra into libraries outside of the mozilla/rust repo, we can dissolve libraries into the mozilla/rust repo for now.

Work on this has been excruciatingly painful in the past because the makefiles are completely opaque to all but a few. Adding a new library involved adding about 100 lines spread out across 8 files (incredibly error prone). The first commit of this pull request targets this pain point. It does not rewrite the build system, but rather refactors large portions of it. Afterwards, adding a new library is as simple as modifying 2 lines (easy, right?). The build system automatically keeps track of dependencies between crates (rust *and* native), promotes binaries between stages, tracks dependencies of installed tools, etc, etc.

With this newfound buildsystem power, I chose the `extra::flate` module as the first candidate for removal from libextra. While a small module, this module is relative complex in that is has a C dependency and the compiler requires it (messing with the dependency graph a bit). Albeit I modified more than 2 lines of makefiles to accomodate libflate (the native dependency required 2 extra lines of modifications), but the removal process was easy to do and straightforward.

---

Testing-wise, I've cross-compiled, run tests, built some docs, installed, uninstalled, etc. I'm still working out a few kinks, and I'm sure that there's gonna be built system issues after this, but it should be working well for basic use!

cc #8784
@bors bors closed this Jan 27, 2014
@bors bors merged commit cdfdc1e into rust-lang:master Jan 27, 2014
@alexcrichton alexcrichton deleted the refactor branch January 27, 2014 05:54
dmanescu added a commit to dmanescu/rust that referenced this pull request Jan 29, 2014
In line with the dissolution of libextra - rust-lang#8784 - moves arena to its own library libarena.
Changes based on PR rust-lang#11787. Updates .gitignore to ignore doc/arena.
dmanescu added a commit to dmanescu/rust that referenced this pull request Jan 29, 2014
In line with the dissolution of libextra - moves glob to its own library libglob.
Changes based on PR rust-lang#11787. Updates .gitignore to ignore doc/glob.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Nov 16, 2023
Fixes to `manual_let_else`'s divergence check

A few changes to the divergence check in `manual_let_else` and moves it the implementation to `clippy_utils` since it's generally useful:
* Handle internal `break` and `continue` expressions.
    e.g. The first loop is divergent, but the second is not.
    ```rust
    {
        loop {
            break 'outer;
        };
    }
    {
        loop {
            break;
        };
    }
    ```
* Match rust's definition of divergence which is defined via the type system.
    e.g. The following is not considered divergent by rustc as the inner block has a result type of `()`:
    ```rust
    {
        'a: {
            panic!();
            break 'a;
        };
    }
    ```
* Handle when adding a single semicolon would make the expression divergent.
    e.g. The following would be a divergent if a semicolon were added after the `if` expression:
    ```rust
    { if panic!() { 0 } else { 1 } }
    ```

changelog: None
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.

None yet

9 participants