Skip to content

Stop ignoring trailing semicolons in a macro body when a macro is invoked in expression position #70

Closed

Description

Proposal

Summary and problem statement

When a macro_rules! macro is invoked in expression position, a trailing semicolon in the macro body is silently ignored (see issue rust-lang/rust#33953). For example, the following code compiles:

macro_rules! foo {
    () => {
        true;
    }
}

fn main() {
    let val = match true {
        true => false,
        _ => foo!()
    };
}

This behavior is inconsistent with how semicolons normally work. In any other context, <expr>; produces a value of (), regardless of the type of <expr>. If the type of <expr> has drop glue, then this could lead to unexpected runtime behavior.

Motivation, use-cases, and solution sketches

I propose to remove this special handling of trailing semicolons. As a result, the following code will stop compiling:

macro_rules! foo {
    () => {
        true;
    }
}

fn main() {
    let val = match true {
        true => false,
        _ => foo!() //~ ERROR: unexpected semicolon
    };
	let _ = foo!(); //~ ERROR: unexpected semicolon
	let _ = false || foo!(); //~ ERROR: unexpected semicolon
}

The match arm case is straightforward: _ => true; is a syntax error, since a match arm cannot end with a semicolon.

The two let statements require some explanation. Under the macro expansion proposal described by @petrochenkov, macro expansion works by only reparsing certain tokens after macro expansion. In both let _ = foo!(); and let _ = false || foo!();, the invocation foo!() is used in expression position. As a result, macro expansion will cause us to attempt to parse true; as an expression, which fails.

The alternative would be to reparse the entire let expression - that it, we would try to parse let _ = true;;, resulting in a statement let _ = true; followed by an empty statement ;. In addition to complicating parsing, this would make understanding a macro definition more difficult. After seeing <expr>; as the trailing statement in a macro body, the user now needs to examine the call sites of the macro to determine if the result of <expr> is actually used.

Rolling out this change would take a significant amount of time. As demonstrated in rust-lang/rust#78685, many crates in the ecosystem rely on this behavior, to the point where several upstream fixes are needed for the compiler to even be able to bootstrap. To make matters worse, rustfmt was inserting semicolons into macro arms up until a very recent version (it was fixed by rust-lang/rustfmt#4507). This means that any crates gating CI on stable rustfmt may find it impossible to make the necessary changes until the latest rustfmt rides the release train to stable.

I propose the following strategy for rolling out this change:

  1. Add an allow-by-default future-compatibility lint, and deny it for internal rustc crates
  2. Do a Crater run to determine the extent of impact
  3. When the necessary rustfmt fix makes it way into stable (or earlier, if we determine the impact to be small enough), switch the lint to warn-by-default
  4. Mark the lint for inclusion in the cargo future-incompat-report (see Tracking Issue for cargo report future-incompat rust#71249)
  5. After some time passes, switch the lint to deny-by-default
  6. Make the lint into a hard error (possibly only for macros defined in a crate in a new Rust edition).

Fortunately, this change is very easy on a technical level: we simply need to emit a warning in this code

Prioritization

This doesn't appear to fit into any particular lang-team priority. However, it's part of a larger effort to fix bugs and inconsistencies in Rust's macro system.

Links and related work

Some PRs to crates removing trailing semicolons:

Initial people involved

I plan to implement this if accepted, with @petrochenkov reviewing the implementation.

What happens now?

This issue is part of the experimental MCP process described in RFC 2936. Once this issue is filed, a Zulip topic will be opened for discussion, and the lang-team will review open MCPs in its weekly triage meetings. You should receive feedback within a week or two.

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions