Forbidding lints doesn't really work in macros

[WaffleLapkin]

Story-driven explanation

I was writing a macro that implements ADT <=> int conversions:

TW: macros

trait Convert {
    fn into_u32(self) -> u32;

    fn from_u32(v: u32) -> Self;
}

macro_rules! impl_convert {
    (
        for $Self:ty;

        // This basically matches `Type::Variant { fields... } <=> 0,`
        $( $($path:ident)::* $( { $( $fields:tt )* })? <=> $int:literal, )*
    ) => {
        impl $crate::Convert for $Self {
            fn into_u32(self) -> u32 {
                #[forbid(unreachable_patterns)]
                match self {
                    $( $($path)::* $( { $( $fields )* } )? => $int, )*
                }
            }

            fn from_u32(v: u32) -> Self {
                #[forbid(unreachable_patterns)]
                match v {
                    $( $int => $($path)::* $( { $( $fields )* } )?, )*
                    _ => panic!("invalid value: {v:?}"),
                }
            }
        }
    };
}

Used like this:

enum Type {
    A,
    B { v: bool },
}

impl_convert! {
    for Type;
    Type::A <=> 0,
    Type::B { v: false } <=> 1,
    Type::B { v: true  } <=> 2,
}

I tested it, it worked perfectly. #[forbid(unreachable_patterns)] even ensures that the mapping is 1 to 1 (duplicate patterns would make the lint go off).

However, when I've made a compile_fail doc test to ensure that duplicate patterns are always rejected, it failed (meaning the compilation passed). At first I though that the problem is in rustdoc, but latter testing showed that this is actually the rustc's behavior: if a lint is forbidden inside a macro and is issued inside the macro, then it is not shown and does not abort compilation in case of #[forbid].

This is a shame! I would expect #[forbid(unreachable_patterns)] actually work and catch wrong macro uses. This behavior seems hard to anticipate, in other words "footgun-y". This is especially annoying given that the calls in the same crate, where you are most likely to test your macros, do produce lints.

More precise explanation

Given these two crates:

// crate `a`

#[macro_export]
macro_rules! example {
    () => {
        #[forbid(unused_variables)]
        const _: () = { let a = 0; };
    }
}

example! {} // <-- invocation in `a`

// crate `b`

a::example! {} // <-- invocation in `b`

(play)

Invocation in a fails the compilation because of the forbid. However, if you comment it out, the invocation in b compiles successfully, the lint and forbid are ignored. This is inconsistent, hard to test and error prone (especially if the lint is actually caused by the user input, as in my example in the story above).

Proposal?

I'd like to "fix" this, however, it's not clear what exact rules here should be, how much of a breaking change this is, etc. I'm opening this issue to hear the feedback.

[cjgillot]

This is a deliberate design choice: we suppress lints in expansions of external macros.
In that case, we expect the user wouldn't be able to modify the source, so linting would be counterproductive.

[WaffleLapkin]

The problem is that is that in many cases user is able to change the source of the error (namely, input to the parser).

[shadyfennec]

I have just hit this problem, where I have a function-like proc-macro that generates a match on a u8; ideally, the macro would generate a "unreachable pattern" warning if the user uses the macro in such a way that it creates duplicate match arms, but as noted since the macro is defined in another crate, the warning is suppressed (and even having #[deny(unreachable_patterns)] on the match expression doesn't work!).

I know it's not an important compiler error, but I believe allowing lints to fire in foreign macros (the list of which could be defined by the macro author, maybe?) would be very useful!

[peter-lyons-kehl]

1. Current: Unfortunately (and most likely by design, as mentioned above), this issue affects any macro-generated code coming from /originating in another crate:

• macros by example (macro_rules!),
• all three types of procedural macros,
• macros defined with nightly declarative macros 2.0 (rust-lang/rust#39412), and
• "inner" macro_rules! whose definition code is generated with "outer" macro_rules!. The consumer crate first invokes the "outer" macro (from another crate) - and invokes it only once per her crate. That generates code that contains "inner" macro_rules! definition of the macro that does the actual job. The user then invokes that "inner" macros wherever needed. (While that does compile on stable, it would be useless on stable because the "inner" macro_rules couldn't have any parameters/capturing variables on stable. But on nightly it could use use $$ metavariable. Either way, lint control still doesn't work even for those inner macros, even though they could be thought of as generated in your crate.)

2. Could we document the current design/functionality, probably with lints?

3. cargo expand (or, rather, -Zunpretty=expanded) makes this worse, because it doesn't indicate that lints/lint control does not apply. If you take code generated by cargo expand, that does compile, while the original code (across crates) does not.

4. A (painful and limited) hypothetical workaround

• even reading this may be a pain; I will provide an example if that turns out usable
• basically, split the macro codebase, and vary the "frontend" macros between crates.io-compatible consumers (no lints control) and other consumers (with lints enforced)
• "non-crates.io-mode" (lints enforced)
  • not for "consumer" crates that themselves are libraries. if those libraries also export any macros that invoke our macros and that want the lints. Such consumer libraries would have to apply the same approach as here.
  • only for consumers that are libraries exporting non-macro functionality, binary/integration tests and rustdoc, and only where you have CI/shell access/access to the filesystem.
  • only applies to macro_rules! (or declarative macros 2.0 on nightly)
  • split the relevant macro(s) into "frontend" that checks/generates any lints, and the rest ("backend").
  • "frontend"
    • distribute in a separate file (even on the same GIT branch, as crates.io doesn't care about a crate's GIT)
    • "frontend" macros must have non-conflicting names, so, for example, prefixed with your crate's name (since modules don't apply to macro_rules!; declarative macros 2.0 on nightly benefit from module namespacing). But we re-export them in a module with a name chosen by the user, and those macros can have short names. Those short names would be the same as macro names in the "standard" (crates.io) mode.
    • load the "frontend" macros from the distributed file in a separate module, like #[path = "../macro-crate/frontend.rs"] mod macro_crate_frontend;
    • doctests would use #![doc(test(no_crate_inject))]
    • Some of the previous could be bundled into a "loader" macro_rules!, which would take a path to the frontend file, and a name of the module to re-export the frontend macro short names.
• The crate could have a compatible "loader" macro in "crates.io mode", which would import the macros in a module with name given by the user.

[peter-lyons-kehl]

Simplified summary of the workaround with loading the macros with lints into user's namespace (either always so, or configurable on/off):

• pub use unique_prefix_xxx as xxx for each such macro (right after its definition), so that you can (re)export it in a module with a default name, or with the name given by the user
• prefer just to load the macro in the user space with lints, rather than to have a switch/option between that and exporting the macro(s) from your crate and hence used in the user space without lints
• if you really want such a switch/option:
  • it will affect your doctests (and any tests). To avoid duplicating
    • all the positive doctests, and
    • a part of negative doctests that fail for either choice,
      you can automate it with e.g.

    //! # Examples (linted)
    #![doc = macro_that_injects_doctests(load_in_userspace = yes)]`
    //! # Examples (not linted)
    #![doc = macro_that_injects_doctests(load_in_userspace = no)]`

    and have such macros inject the appropriate loading preamble. The drawback is that then those doctest files won't behave the same outside of cargo rustdoc.
• have two similar copies of the file/module that defines the macros that (optionally) use lints. Track changes between those files/modules (for example, with wdiff).

The most polished version is at https://github.com/prudent-rs/prudent/tree/96b0d1a47364a3b43a4165cdde9344c774278e42. The automation of two sets of positive doctests is in src/lib.rs:

//! # Examples (linted)
#![doc  = internal_coverage_positive!("any: \"frontend_linted.rs\"") ]
//! # Examples (not linted)
#![doc  = internal_coverage_positive!("") ]

See source comments for some pain and workarounds.

(I've ended up not publishing a version with this mechanism to crartes.io. Instead of using lints, my use case allows me to perform the checks with nightly. So I'll have a feature for that.)