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

Error if the wrong architecture is being used #41

Merged
merged 1 commit into from
Aug 9, 2020

Conversation

lights0123
Copy link
Contributor

There have been a couple times where an incorrectly set target file has caused errors that don't immediately point to the problem. Instead, we should show an error immediately telling the user that the wrong architecture has been selected.

@Rahix
Copy link
Owner

Rahix commented Aug 9, 2020

Hm, I had a check like this in a previous incarnation of these crates (here) and it wasn't well received ... Looking at cortex-m I see they also had trouble with this: rust-embedded/cortex-m#63

So I think we should go a route similar to what they did over there: 875ee38398f6 (" map asm! ops to unimplemented! on non ARM targets ")

Though I think using cfg-if is a lot nicer than their match () { ... } approach.


But this will still not address the issue of people accidentally building for the host. We'll have to find another way to address that, which doesn't interfere with the above ...

@lights0123
Copy link
Contributor Author

What if, instead of going to unimplemented!(), we call an undefined function? That will give the user a link-time error, and if we name the undefined symbol something that explains the problem like no-panic does, it should hopefully be somewhat obvious to the user. But if the function is never called (i.e. in a unit test on the host), the function will never actually get included and not cause an error.

Alternatively, we could move the changes I made in this PR to your #[entry] macro as that should never be used on a non-AVR platform.

@Rahix
Copy link
Owner

Rahix commented Aug 9, 2020

What if, instead of going to unimplemented!(), we call an undefined function? That will give the user a link-time error, and if we name the undefined symbol something that explains the problem like no-panic does, it should hopefully be somewhat obvious to the user. But if the function is never called (i.e. in a unit test on the host), the function will never actually get included and not cause an error.

I think the problem over in cortex-m was exactly the cases where the calls/code-paths couldn't get optimized out. So this would just eventually come back to bite ...

Alternatively, we could move the changes I made in this PR to your #[entry] macro as that should never be used on a non-AVR platform.

Actually, that's a very good idea! #[entry] won't prevent any lib-crates breaking but keeps applications from compiling for the wrong target. Definitely my favorite approach!

@lights0123
Copy link
Contributor Author

I referenced this PR in the error message as I link to some of the issues where this occurs, but we may want to create a FAQ section and link to that instead. The seven spaces before the error message is significant: error: is 7 characters, so this will wrap nicely:

error: Ensure that you are using an AVR target! You may need to change directories or pass a --target flag to cargo. See
       https://github.com/Rahix/avr-device/pull/41 for more details.

Copy link
Owner

@Rahix Rahix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, thanks!

@Rahix Rahix merged commit 91f3ea8 into Rahix:master Aug 9, 2020
@rvdende
Copy link

rvdende commented Dec 25, 2021

I'm still running into this issue with vscode and the template default.
Where can I set the target arch so rust-analyzer does not complain about this issue?

avr_device_macros
pub macro entry
Ensure that you are using an AVR target! You may need to change
directories or pass a --target flag to cargo. See
#41 for more details.rust-analyzermacro-error

@Rahix
Copy link
Owner

Rahix commented Dec 26, 2021

It looks like vscode is ignoring the .cargo/config.toml file in your project. rust-analyzer should be doing that automatically but maybe your project dir is not detected correctly? I'm afraid I can't help you much here :/

@bscubed
Copy link

bscubed commented Dec 30, 2021

I'm still running into this issue with vscode and the template default. Where can I set the target arch so rust-analyzer does not complain about this issue?

avr_device_macros
pub macro entry
Ensure that you are using an AVR target! You may need to change
directories or pass a --target flag to cargo. See
#41 for more details.rust-analyzermacro-error

I was running into the same issue with a default project directly from cargo generate. I was able to get rid of the error by disabling Rust-analyzer > Proc Macro: Enable in the extension's settings.
Hope this helps!

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.

4 participants