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

Support for fatal compile_error that suppresses subsequent errors #68838

Open
joshtriplett opened this issue Feb 5, 2020 · 10 comments
Open

Support for fatal compile_error that suppresses subsequent errors #68838

joshtriplett opened this issue Feb 5, 2020 · 10 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@joshtriplett
Copy link
Member

If a crate includes something like:

#[cfg(not(target_os = "linux"))]
compile_error!("This crate only runs on Linux")

then it isn't helpful for rustc to report that error but then go on to emit many more cascading errors about failed use statements and types that don't exist and similar.

It's especially unhelpful when those additional errors scroll the initial error off the screen.

@Centril Centril added T-lang Relevant to the language team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Feb 5, 2020
@Mark-Simulacrum
Copy link
Member

I think making compile_error always immediately fatal makes sense. Since it's unconditional and only useful in cases like this (I believe?), that should be fine.

@Centril Centril added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-diagnostics Area: Messages for errors, warnings, and lints labels Feb 5, 2020
@Centril
Copy link
Contributor

Centril commented Feb 5, 2020

That's not the only use case for compile_error!. As the examples show, sometimes you want to error when a specific macro arm is taken which is unsupported. In that case the error shouldn't be fatal. Moreover, when you do emit a fatal error, you don't let other crates compile, which may have errors of their own. So I don't think we should make compile_error! emit fatal errors.

@Mark-Simulacrum
Copy link
Member

I think it's true that there are cases where it might not necessarily need to be fatal, but I don't know that I'd say that they are common, and that the usability impact of an (unnecessary) fatal error is worse than the impact of emitting a fatal error (and potentially losing others errors as a result).

I am not sure what you mean by "don't let other crates compile" -- to my knowledge, Cargo has no way of knowing whether a fatal error (or a normal error) happened?

If we decide not to pursue a fatal error, we're basically saying that good usage of compile_error for crate-level gating as @joshtriplett is proposing here should look something like this, I think, which while not bad seems much less ergonomic than the "just a fatal error" option.

// lib.rs
#[cfg(not(target_os = "linux"))]
compile_error!("This crate only runs on Linux")

#[cfg(target_os = "linux")]
mod reallib;
#[cfg(target_os = "linux")]
pub use reallib::*;

Of course, an alternative design would be to try to have some heuristic -- e.g., compile_error at the "top level" of macro expansion, modulo cfg gating, would lead to a fatal error, but that seems like a bad idea.

@Centril
Copy link
Contributor

Centril commented Feb 5, 2020

I am not sure what you mean by "don't let other crates compile" -- to my knowledge, Cargo has no way of knowing whether a fatal error (or a normal error) happened?

Hmm; maybe I'm misinformed; I thought a fatal error would prevent Cargo from proceeding to other crates.

Regarding "common" or not, ideally it would be based on some survey. Otherwise it feels like mostly speculation.

Doing #[cfg(...)] mod reallib; like that doesn't seem too onerous, particularly if you use https://crates.io/crates/cfg-if ?

A third design would be to add another macro (why I added the language team).

@joshtriplett
Copy link
Member Author

@Mark-Simulacrum I don't think we should make compile_error! always a fatal error. I would suggest that we introduce a compile_fail! or similar.

@Centril
Copy link
Contributor

Centril commented Feb 5, 2020

I would suggest fatal_error!(...) if we want another macro as it makes the distinction more obvious in the name.

@Mark-Simulacrum
Copy link
Member

I guess -- maybe a good question to ask: why is making compile_error produce fatal errors a bad thing?. Both of you seem to be against the fatal error, but I'm still not entirely clear why. I think it's true that we're trying to move away from fatal errors in general -- but if the proposal is introducing another macro that would fatal error anyway, it seems like there's not much advantage in making compile_error not error fatally anyway. I would expect that we'd rather avoid two macros that do almost the same thing than make the existing macro uniformly usable (if subpar in some sense in some cases).

@petrochenkov
Copy link
Contributor

FWIW, we could add an extra argument to compile_error:

compile_error!(error, "message"); // Non-fatal error
compile_error!(fatal_error, "message"); // Fatal error
compile_error!(warning, "message"); // Warning
compile_error!("message"); // Sugar for `compile_error!(error, "message")`

(Inspired by CMake, of all things.)

I also think non-fatal is a better default, we've done some significant amount of work making sure that non-fatal errors can be reported without causing a waterfall of follow-up errors in most cases.
We can gather some statistics on the use of compile_error though, to make a data-based decision.

@joshtriplett joshtriplett removed the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Feb 8, 2020
@kaimast
Copy link

kaimast commented Aug 6, 2020

To add my two cents to this:

The documentation currently states that compile_error! is "the compiler-level form of panic!, but emits an error during compilation rather than at runtime".
I assumed that it would abort compilation just like panic! aborts execution. I think the behavior either needs to be changed as requested or, at least, the documentation needs to be fixed.

Also, what is the current correct way to generate a fatal compile error? Calling compile_error! and then panic!?

Thomasdezeeuw added a commit to Thomasdezeeuw/socket2 that referenced this issue Sep 17, 2021
Although it still spits out a bunch of errors at least the first one
will be informative about the root cause.

rust-lang/rust#68838 tracks a version that
suppresses subsequent errors.
Thomasdezeeuw added a commit to rust-lang/socket2 that referenced this issue Sep 18, 2021
Although it still spits out a bunch of errors at least the first one
will be informative about the root cause.

rust-lang/rust#68838 tracks a version that
suppresses subsequent errors.
@philpax
Copy link
Contributor

philpax commented Jun 28, 2023

Hi there! I was wondering what the current status of this is, and if there's an idiomatic way to do it.

I have an attribute proc macro attached to main() that parses a manifest, does some logic, and then generates a bunch of boilerplate. If the macro itself fails (due to an invalid manifest or such), I want to terminate compilation entirely.

At present, I'm generating a compile_error! in the error case - but this still results in warnings for unused items being emitted, which I don't want. I'd like it to terminate compilation right there and then with my error message, and not attempt to do any further resolution.

Assuming that there's no language solution for this yet, what's the best way to terminate compilation and suppress as much as of the following output as possible?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-feature-request Category: A feature request, i.e: not implemented / a PR. 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

6 participants