Pre-RFC - Add compile_warning! macro

language design
1

Rendered

This RFC proposes compile time warning evaluation similar to compile_error!. This is my first RFC, and I don’t have much internal knowledge on Rust.

1 Like
2

As far as I can tell, the only reason that compile_warning! was not introduced in the exact same RFC as compile_error! was this set of open questions that need to be answered for warnings but not errors.

  • How does it interact with #[deny(warnings)]?
  • Is it #[allow]able?
  • How is the lint name specified?

I don’t think any of these points have been addressed yet in the new document.

Drawbacks, Rationale and Alternatives, and Prior Art sections all need to cover the #[allow] story. The main purpose of a compile_warnings! RFC would be to resolve that aspect.

5 Likes
3

I can take a look at this when I can, thank you.

EDIT: Editing it right now.

4

I modified the macro signature a bit to include the lint name, be more specific about #[deny(warnings)] and #[allow], and added a new unresolved question that emerges from this.

I’ll do more work to be specific about #[allow], I believe that a good solution would be to namespace warnings similar to Clippy.

5

  • I am confused about this sentence:

    The warning would be implemented the same as functions like println!

    Println formatting is evaluated at runtime, not at compile time. The implementation would need to be substantially different given that warnings need to appear at compile time, not runtime. The RFC would need to go into more detail on how this can work.

    Separately, println! is not a function.

  • #[allow] is not covered. It is mentioned in the summary, but the summary is supposed to summarize what is covered by the rest of the document.

  • The Rationale + Alternatives section needs more work.

    The only alternative is compiler plugins, which are not as user friendly as an addition to the standard library.

    • Compiler plugins are not the only alternative. Every detail of the RFC will have alternatives. For example, not accepting a warning name is an alternative. Accepting warning name as an identifier rather than string literal is an alternative. Having the existing unimplemented!() macro unconditionally emit a warning is an alternative for the TODO use case in the guide-level explanation. You can probably come up with ~10 alternatives. For each one, this section needs to justify why the approach being proposed is preferable.

    • User friendliness is not the only consideration when deciding what goes in crates vs core. Please check out some other library RFCs to see how they rationalize new functionality.

    • I am not clear what user friendliness refers to. Does everything become more user friendly when moved from a crate into the standard library? That makes the argument useless because it could be used to justify anything at all.

  • warnings generated by compile_warning! will not show when #[deny(warnings)] is in effect.

    This would be surprising to me. It does not match the normal behavior of deny(warnings).

  • Reference-level Explanation needs work. Some basic examples of what a reference would need to cover are the formal input syntax, what makes a legal warning name, and conflicts with builtin lint and warning names.

  • I don't think "developers may not know that compile_error exists" should be considered a drawback of introducing compile_warning. You can remove that one but please try to come up with genuine drawbacks.

    • For example I don't see how const fn could emit conditional warnings under this proposal.

    • Using compile_warning for lots of todo code makes it way more likely to miss rustc warnings during development. This seems bad; I would personally not use compile_warning! over unimplemented! for this reason. Unimplemented is easy to grep.

    I would recommend brainstorming ~5 drawbacks.

  • The motivation mentions const fn so I would have expected some explanation of how this feature is important to const fn. In fact const fn use cases seem like one of the major downsides of this proposal.

  • Prior Art needs to cover prior art in Rust too.

    • How do macros emit errors?
    • How do const fn emit errors?
    • #![feature(const_panic)]
    • #![feature(proc_macro_diagnostic)]
    • #[deprecated]

  • The Motivation section implies that this is already possible with weird hacks. Each of those hacks should be explained as alternatives.

  • The motivation emphasizes (directly in the first sentence) procedural macros. But procedural macros can already emit warnings through proc_macro_diagnostic. I think this weakens the motivation so it would be better to start it in a different way.

  • I find the guide-level explanation the opposite of compelling. As mentioned in my comment on drawbacks, I would not want to use compile_warning for this use case. Could you find a different use case to highlight in the explanation?

2 Likes
6

Also, warnings should be able to provide rustfix-enabled suggestions.

7

I just want to say that I am strongly in favor of this idea. Some lints/warnings should not come from clippy but from rustc itself. For instance:

  • the unimplemented! macro should trigger a warning. If, like @dtolnay, you think it will clutter the compilation with too many warnings, then you could just add a #![allow(unimplemented)]. Saying that unimplemented! is easy to grep is like saying that there was no need for -Wformat-security in C since erroneous usage of format strings can be spoted using $ grep -RnE '\bv?(sn?)?printf *\( *[^"]' . (ok, that’s a strawman “argument”, I agree that those two things are not really comparable and apologise for that; but still, the “no need for a warning thanks to grep” looks like a weird argument to me);

  • regarding the whole "spotting dependencies that use unsafe" topic, warning on unsafe could be a nice opt-in lint (a special kind of lint, that could transitively override the enabled state on the specified dependencies)

But, between the time (some) people feel the need to have some kind of warning and when it is finally included in the language (if ever), custom warnings would come in handy.

For reference, a dirty hack to get the “custom warning message” working today (I use it with my override of unimplemented! so that it triggers a warning), is to use the #[must_use = $error_msg] on a custom type (or a custom function):

Crate ::compile_warning :

#[macro_export]
macro_rules! compile_warning {(
    $name:ident, $message:expr $(,)*
) => (
    mod $name {
        #[must_use = $message]
        struct compile_warning;
        #[allow(dead_code)]
        fn trigger_warning () { compile_warning; }
    }
)}

Main crate:

#[macro_use] extern crate compile_warning;

fn main() {
    compile_warning!(example, "Memento buggy");
}

$ cargo check

warning: unused `main::example::compile_warning` that must be used
 --> src/main.rs:4:5
  |
4 |     compile_warning!(example, "Memento buggy");
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: #[warn(unused_must_use)] on by default
  = note: Memento buggy
  = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

This is quite hacky and dirty (cannot use the same “warning name” within a given namespace twice + it uses the unused_must_use “lint space”), and could be greatly improved by what the OP suggests.

That being said, most points raised by @dtolnay remain valid : there is still work to make an actual RFC out of this idea.

3 Likes
8

You're right, this seems like something that makes an implementation demonstrably harder. I'm going to remove that part unless you feel it is necessary.

I plan on adding onto that, but thanks for the tip on the summary.

My bad, I thought this section was asking for alternatives already in Rust, but I understand now it is asking for alternative solutions as well.

Agh, I'm dumb. I forgot what #[deny(warnings)] actually did. :man_facepalming: Fixing this

I need to test to see if compile_error! works with const fn, I just assumed. Update: It doesn't, strange.

9

I brought up conditional warnings because const fn can emit conditional errors by panicking, at least once the accepted RFC 2343 has been implemented:

#![feature(const_fn, const_panic)]

const fn len(x: usize) -> usize {
    if x < 4 {
        x * 2
    } else {
        panic!("try x<4")
    }
}

fn main() {
    let _: [u8; len(5)];
}
2 Likes
10

I see, so I have to be explicit that I propose compile_warning! works in that context?

11

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.