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

Don't promote binaries between stages #11145

Closed
brson opened this issue Dec 26, 2013 · 7 comments
Closed

Don't promote binaries between stages #11145

brson opened this issue Dec 26, 2013 · 7 comments
Labels
P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@brson
Copy link
Contributor

brson commented Dec 26, 2013

The way the build currently works makes a distinction between the 'host' binaries, which are used only to run the toolchain, and the 'target' binaries, which are targeted by the toolchain. There are separate locations on disk for each. As the build progresses between stages, binaries are promoted from the target directories to the host directories, then the host bins used to build the target bins within the next stage.

This has a few problems:

  • It doesn't make sense for cross compiles in general, since non-build-triple hosts don't need to bootstrap. This works in our only working multi-host cross-compile currently because we're just doing x86->x86_64.
  • It means that programs compiled by rustc cannot (reliably) be used as plugins to rustc itself since they are linking to different versions of libsyntax/librustc. This would make procedural macros pretty difficult.

My preference is to not intermix binaries from different stages, not have the host libraries at all, link rustc to the same target libraries as others in the same stage, use previous stages to build the entirety of the next stage.

This would have it's own bootstrapping issues, and the last time I made this change we decided not to do it because of some new difficulty to the snapshotting/bootstrapping process that I don't recall. But any build arrangement for a self-bootstrapping compiler is going to have difficulties and I think this would be the least surprising way to do the build in general.

@brson
Copy link
Contributor Author

brson commented Dec 26, 2013

Some IRC convo:

19:46 < sfackler> brson: do you know if there's any way to get stuff in test/auxiliary to link against the same version of e.g. libsyntax that that version of rustc links 
                  against?
19:46 < sfackler> just doing an "extern mod syntax" seems to link a stage too high
19:46 -!- kimundi [[email protected]] has quit [Ping timeout]
19:48 <@brson> sfackler: no, there isn't a way. because of the way the stage progression works, rustc is always linked to the previous stage
19:48 <@brson> i can't imagine a reason that would matter though? what odd thing are you trying to do?
19:49 <@brson> if you link to rustc from aux though you'll get a different rustc than the one used to do the compilation
19:49 < sfackler> external syntax extensions! https://github.com/sfackler/rust/blob/28206f335c02ee5fef1eded5ab13dd3b459db839/src/test/auxiliary/macro_crate_test.rs
19:50 -!- kimundi [[email protected]] has joined #rust-internals
19:50 <@brson> aha
19:51 < sfackler> I need to use the same libsyntax binary or the dynamic linker fails to match things up properly
19:51 <@brson> that is going to be tricky because of this issue :(
19:51 <@brson> i would like to build the stages in a different way so that they aren't intermixed like this, but there are complications
19:52 <@brson> this is something we need to resolve though
19:52 <@brson> i guess I'd prefer to just bite the bullet and *completely change the build process*
19:52 < sfackler> how does run-pass-fulldeps work? would it be possible to do something like that for auxilliary-fulldeps?
19:53 <@brson> sfackler: no, you would still be getting the wrong libs
19:53 <@brson> the rustc exe basically has its own set of libraries
19:53 <@brson> from the previous stage
19:53 <@brson> you could hack something together to manually point your library to the correct libraries
19:54 <@brson> but that's minimally useful
19:54 < sfackler> yeah
19:54 <@brson> with -L
19:54 -!- kimundi is now known as zz_kimundi
19:55 <@brson> in theory the stage3 rustc should have the same libs as its targets
19:55 <@brson> but stage3 is not used much
19:55 <@brson> and that's not always true, for unknown reasons
19:57 <@brson> i'll file an issue about this
19:58 < sfackler> cool. I'm guessing that this'll block on getting the test infrastructure to a place where it can actually have an rpass test, right?
19:59 -!- canhtak [[email protected]] has quit [Quit: canhtak]
20:00 <@brson> sfackler: does it work manually, but not in a test harness? that would surprise me
20:00 < sfackler> it works off of a full install
20:01 -!- zz_kimundi is now known as kimundi
20:01 <@brson> ok. probably because the symbols for the host bins and target bins are identical
20:01 < sfackler> and i'm guessing that  work at stage1 if I pointed it at the right libsyntax
20:01 < sfackler> *that it'll
20:01 <@brson> does the test work in stage2?
20:02 <@brson> i would expect so, since stage2 is the install stage
20:02 < sfackler> let me check
20:02 < sfackler> it does appear to work at stage2
20:03 <@brson> if it works reliably in stage2 we *might* be able to merge it, but I'm not sure how much we can rely on the stage2 symbols being the same
20:03 < sfackler> is rustc statically linked?
20:04 < sfackler> it should definitely work with the installed version of rustc since rustc and the extension crate will be linking to the exact same library
20:05 <@brson> they won't be linking to the same library. there are always at least two copies of the libs installed
20:05 < sfackler> oh, really?
20:05 <@brson> one in /usr/lib and one in /usr/lib/rustc/$triple/lib
20:05 < sfackler> ah
20:06 <@brson> i would rather the one in /usr/lib didn't exist
20:06 <@brson> it's very compilcated to change the build process though
20:06 < sfackler> yeah, it seems like if you want to link against libsyntax or librustc, you'd always want the same version as rustc's
20:08 <@brson> yes. it's tricky though. the rustc that is compiling your crate that is linking to libsyntax needs to be exactly compatible with the one that built libsyntax, 
               but it itself also needs to link to the libsyntax that it built
20:08 <@brson> bootstrapping is neat
20:09 <@brson> s/neat/hard

@alexcrichton
Copy link
Member

Nominating. If we want a true cross-compile process I think we'll need to deal with this.

@pnkfelix
Copy link
Member

pnkfelix commented Jan 9, 2014

Figuring out some aspects of this are blocked on Loadable Syntax Extensions (i.e. #11151).

Along with the cross-compilation issues. . .and we don't have to resolve it for 1.0.

So assigning to P-low.

bors added a commit that referenced this issue Jan 17, 2014
This is a first pass on support for procedural macros that aren't hardcoded into libsyntax. It is **not yet ready to merge** but I've opened a PR to have a chance to discuss some open questions and implementation issues.

Example
=======
Here's a silly example showing off the basics:

my_synext.rs
```rust
#[feature(managed_boxes, globs, macro_registrar, macro_rules)];

extern mod syntax;

use syntax::ast::{Name, token_tree};
use syntax::codemap::Span;
use syntax::ext::base::*;
use syntax::parse::token;

#[macro_export]
macro_rules! exported_macro (() => (2))

#[macro_registrar]
pub fn macro_registrar(register: |Name, SyntaxExtension|) {
    register(token::intern(&"make_a_1"),
        NormalTT(@SyntaxExpanderTT {
            expander: SyntaxExpanderTTExpanderWithoutContext(expand_make_a_1),
            span: None,
        } as @SyntaxExpanderTTTrait,
        None));
}

pub fn expand_make_a_1(cx: &mut ExtCtxt, sp: Span, tts: &[token_tree]) -> MacResult {
    if !tts.is_empty() {
        cx.span_fatal(sp, "make_a_1 takes no arguments");
    }
    MRExpr(quote_expr!(cx, 1i))
}
```

main.rs:
```rust
#[feature(phase)];

#[phase(syntax)]
extern mod my_synext;

fn main() {
    assert_eq!(1, make_a_1!());
    assert_eq!(2, exported_macro!());
}
```

Overview
=======
Crates that contain syntax extensions need to define a function with the following signature and annotation:
```rust
#[macro_registrar]
pub fn registrar(register: |ast::Name, ext::base::SyntaxExtension|) { ... }
```
that should call the `register` closure with each extension it defines. `macro_rules!` style macros can be tagged with `#[macro_export]` to be exported from the crate as well.

Crates that wish to use externally loadable syntax extensions load them by adding the `#[phase(syntax)]` attribute to an `extern mod`. All extensions registered by the specified crate are loaded with the same scoping rules as `macro_rules!` macros. If you want to use a crate both for syntax extensions and normal linkage, you can use `#[phase(syntax, link)]`.

Open questions
===========
* ~~Does the `macro_crate` syntax make sense? It wraps an entire `extern mod` declaration which looks a bit weird but is nice in the sense that the crate lookup logic can be identical between normal external crates and external macro crates. If the `extern mod` syntax, changes, this will get it for free, etc.~~ Changed to a `phase` attribute.
* ~~Is the magic name `macro_crate_registration` the right way to handle extension registration? It could alternatively be handled by a function annotated with `#[macro_registration]` I guess.~~ Switched to an attribute.
* The crate loading logic lives inside of librustc, which means that the syntax extension infrastructure can't directly access it. I've worked around this by passing a `CrateLoader` trait object from the driver to libsyntax that can call back into the crate loading logic. It should be possible to pull things apart enough that this isn't necessary anymore, but it will be an enormous refactoring project. I think we'll need to create a couple of new libraries: libsynext libmetadata/ty and libmiddle.
* Item decorator extensions can be loaded but the `deriving` decorator itself can't be extended so you'd need to do e.g. `#[deriving_MyTrait] #[deriving(Clone)]` instead of `#[deriving(MyTrait, Clone)]`. Is this something worth bothering with for now?

Remaining work
===========
- [x] ~~There is not yet support for rustdoc downloading and compiling referenced macro crates as it does for other referenced crates. This shouldn't be too hard I think.~~
- [x] ~~This is not testable at stage1 and sketchily testable at stages above that. The stage *n* rustc links against the stage *n-1* libsyntax and librustc. Unfortunately, crates in the test/auxiliary directory link against the stage *n* libstd, libextra, libsyntax, etc. This causes macro crates to fail to properly dynamically link into rustc since names end up being mangled slightly differently. In addition, when rustc is actually installed onto a system, there are actually do copies of libsyntax, libstd, etc: the ones that user code links against and a separate set from the previous stage that rustc itself uses. By this point in the bootstrap process, the two library versions *should probably* be binary compatible, but it doesn't seem like a sure thing. Fixing this is apparently hard, but necessary to properly cross compile as well and is being tracked in #11145.~~ The offending tests are ignored during `check-stage1-rpass` and `check-stage1-cfail`. When we get a snapshot that has this commit, I'll look into how feasible it'll be to get them working on stage1.
- [x] ~~`macro_rules!` style macros aren't being exported. Now that the crate loading infrastructure is there, this should just require serializing the AST of the macros into the crate metadata and yanking them out again, but I'm not very familiar with that part of the compiler.~~
- [x] ~~The `macro_crate_registration` function isn't type-checked when it's loaded. I poked around in the `csearch` infrastructure a bit but didn't find any super obvious ways of checking the type of an item with a certain name. Fixing this may also eliminate the need to `#[no_mangle]` the registration function.~~ Now that the registration function is identified by an attribute, typechecking this will be like typechecking other annotated functions.
- [x] ~~The dynamic libraries that are loaded are never unloaded. It shouldn't require too much work to tie the lifetime of the `DynamicLibrary` object to the `MapChain` that its extensions are loaded into.~~
- [x] ~~The compiler segfaults sometimes when loading external crates. The `DynamicLibrary` reference and code objects from that library are both put into the same hash table. When the table drops, due to the random ordering the library sometimes drops before the objects do. Once #11228 lands it'll be easy to fix this.~~
@steveklabnik
Copy link
Member

Triage: while Loadable Syntax Extensions have landed, I don't think much else has changed here.

@steveklabnik
Copy link
Member

Triage: still not aware of any actual changes yet, but @alexcrichton 's Cargo-based build system may fix this, maybe not?

@pnkfelix pnkfelix added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 9, 2016
@pnkfelix
Copy link
Member

pnkfelix commented Feb 9, 2016

Assigning to compiler team ... though in hindsight maybe we don't have a team that owns build infrastructure issues

@alexcrichton
Copy link
Member

Quite a bit of time has passed since this was first opened, and I don't think it's wholly relevant any more. In any case with rustbuild now we've refactored the build into the current desired format for many moons to come, so closing.

flip1995 pushed a commit to flip1995/rust that referenced this issue Jul 14, 2023
[`arithmetic_side_effect`]: allow different types on the right hand side for `Wrapping<T>`

Fixes rust-lang#11145

This lint has a list of allowed types, one of which is `Wrapping<T>`, but it was only actually allowed if the type on the right hand side was also `Wrapping<T>`, which meant that, for example, `Wrapping<u32> += u32` would still lint. It now allows binary ops involving `Wrapping<T>` regardless of the type on the rhs.
These impls have only existed since Rust 1.60.0, so that is probably why the lint was previously not handling this correctly

changelog: [`arithmetic_side_effect`]: allow different types on the right hand side for `Wrapping<T>` (e.g. `Wrapping<T> += T`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants