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

Improve usage of compiler-builtins #49851

Closed
jamesmunns opened this issue Apr 10, 2018 · 10 comments
Closed

Improve usage of compiler-builtins #49851

jamesmunns opened this issue Apr 10, 2018 · 10 comments
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.

Comments

@jamesmunns
Copy link
Member

jamesmunns commented Apr 10, 2018

This is regarding the changes made in #49380. This change works, however when the user makes some kind of mistake, especially forgetting to install the new targets (like thumbv7em-none-eabihf), the error messages are not currently helpful.

The current error message when forgetting to install the target looks like this:

cargo build --target thumbv7em-none-eabihf --example blinky
   Compiling vcell v0.1.0
   Compiling bare-metal v0.1.1
   Compiling aligned v0.1.1
   Compiling untagged-option v0.1.1
error[E0463]: can't find crate for `compiler_builtins`

error[E0463]: can't find crate for `compiler_builtins`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error[E0463]: can't find crate for `compiler_builtins`

error: aborting due to previous error

error[E0463]: can't find crate for `compiler_builtins`
For more information about this error, try `rustc --explain E0463`.

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.

compared to the following when you try to compile for a not-installed target:

cargo build --target i686-unknown-linux-gnu
   Compiling foo v0.1.0 (file:///tmp/foo)
error[E0463]: can't find crate for `std`
  |
  = note: the `i686-unknown-linux-gnu` target may not be installed

error: aborting due to previous error

For more information about this error

The following #[no_std] code shows (the first part of) a typical embedded project:

//! Blinks an LED

#![deny(unsafe_code)]
#![deny(warnings)]
#![no_std]

extern crate cortex_m;

using cargo expand, this code expands to:

#![feature(prelude_import)]
#![no_std]
//! Blinks an LED

#![deny(unsafe_code)]
#![deny(warnings)]
#![no_std]
#[prelude_import]
use core::prelude::v1::*;
#[macro_use]
extern crate compiler_builtins;
#[macro_use]
extern crate core;

extern crate cortex_m;

The Embedded WG talked about this, and we think the error messages would be improved if the order of extern crate compiler_builtins; and extern crate core; were swapped.

@jamesmunns
Copy link
Member Author

CC @japaric and @oli-obk

@japaric
Copy link
Member

japaric commented Apr 10, 2018

we think the error messages would be improved if the order of extern crate compiler_builtins; and extern crate core; were swapped.

I can confirm that swapping them (checked with a hand written #![no_core] crate) produces the old, more helpful error messages of "note: the thumbv6m-none-eabi target may not be installed". It would also be good to drop the #[macro_use] from extern crate compiler_builtins as it seems like it could inject weird macros into #![no_std] programs.

cc @oli-obk

@jamesmunns
Copy link
Member Author

Also I just noticed that in the expanded version, #![no_std] appears twice.

@oli-obk
Copy link
Contributor

oli-obk commented Apr 10, 2018

I wonder how that order swapping happens... should be easy enough to fix. The macro use comes from reusing the core import code. I guess we'll have to duplicate some now, but it's not too much

@oli-obk
Copy link
Contributor

oli-obk commented Apr 10, 2018

Ah.. inserting at position 0 will do that:

https://github.com/oli-obk/rust/blob/679657b863c2a53a3052d8af9defbce48e12db10/src/libsyntax/std_inject.rs#L61

This should be an easy to fix issue. Also needs a ui test

@oli-obk oli-obk added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Apr 10, 2018
@nagisa
Copy link
Member

nagisa commented Apr 11, 2018

Furthermore, since extern crate statements can now appear at any position, it should be fine to replace that insert with a push.

@rcoh
Copy link
Contributor

rcoh commented Apr 16, 2018

I have a diff for this but I'm having a hard time reproducing the error message in the ui tests. Any pointers @oli-obk ? I assume I need compiler flags but the only error I can seem to get is:

"Error loading target specification: Could not find specification for target \"thumbv7em-none-eabih\""

@japaric
Copy link
Member

japaric commented Apr 16, 2018

@rcoh That looks like a typo in the target name; it should be thumbv7em-none-eabihf, with an f at the end.

@rcoh
Copy link
Contributor

rcoh commented Apr 16, 2018

Yep, that was it. Much appreciated.

bors added a commit that referenced this issue Apr 18, 2018
Reorder injection of std to get better compilation error

Per #49851, reorder injection imports to get a better error message.

r? @oli-obk
@japaric
Copy link
Member

japaric commented Apr 26, 2018

This was fixed in #50006. Thanks @rcoh!

@japaric japaric closed this as completed Apr 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
Projects
None yet
Development

No branches or pull requests

5 participants