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

Removal of NtTy resulted in regression #138975

Open
raoulstrackx opened this issue Mar 26, 2025 · 3 comments
Open

Removal of NtTy resulted in regression #138975

raoulstrackx opened this issue Mar 26, 2025 · 3 comments
Assignees
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@raoulstrackx
Copy link
Contributor

raoulstrackx commented Mar 26, 2025

We encountered an issue that starting with nightly-2025-02-23, the async-usercalls crate fails to compile. We managed to come up with a minimal working run-make test, and found that the issue was introduced by commit 76b04437be91069260c72a6d59d130a4e127a9a8 as part of #133436.

Code

See this branch for a run-make test. Starting from nightly-2025-02-23 this fails with:

error: no rules expected `ty` metavariable
 --> two/src/main.rs:9:13
  |
4 | macro_rules! type_matcher {
  | ------------------------- when calling this macro
...
9 |     let _x: call_with_type!(type_matcher) = 42;
  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no rules expected this token in macro call
  |
note: while trying to match `u64`
 --> two/src/main.rs:5:6
  |
5 |     (u64) => { u64 };
  |      ^^^
  = note: captured metavariables except for `:tt`, `:ident` and `:lifetime` cannot be compared to other tokens
  = note: see <https://doc.rust-lang.org/nightly/reference/macros-by-example.html#forwarding-a-matched-fragment> for more information
  = help: try using `:tt` instead in the macro definition
  = note: this error originates in the macro `call_with_type` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0308]: mismatched types
 --> two/src/main.rs:9:45
  |
9 |     let _x: call_with_type!(type_matcher) = 42;
  |             -----------------------------   ^^ expected `()`, found integer
  |             |
  |             expected due to this

For more information about this error, try `rustc --explain E0308`.
error: could not compile `two` (bin "two") due to 2 previous errors
@raoulstrackx raoulstrackx added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Mar 26, 2025
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 26, 2025
@petrochenkov
Copy link
Contributor

cc @nnethercote

@nnethercote nnethercote self-assigned this Mar 26, 2025
@jieyouxu jieyouxu added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 26, 2025
@nnethercote
Copy link
Contributor

It's late here, so I'll look at this closely tomorrow. But I will note an interesting thing from the crater run for #124141: there were several failures in the following form:

In every case, there is a declarative macro with some kind of complexity: a cfg attribute, or a cross-crate boundary, or a nested macro, or some combination. In every case, the old compiler accepts the complex declarative macro while rejecting a simpler form of the same macro, e.g. with the cfg removed, or in an intra-crate case. In every case the new compiler rejects both the more complex and simpler forms, for improved consistency. I believe that the old compiler is buggy in its current behaviour, and that the inconsistencies arise because of the complexity of the internal handling of Interpolated.

And the example involves a cross-crate boundary and a nested declarative macro, so it very much fits this pattern. In other words, it's possible that the code shouldn't have been compiling in its current form.

@nnethercote
Copy link
Contributor

My hunch from yesterday was correct: this is invalid code that the old compiler (pre-#133436) should not have accepted, and the new compiler (post-#133436) is now correctly rejecting it. Details below.


I reproduced the behaviour. I didn't use the run-make test, I just created two crates one and two, where two depends on one, and built two. Thank you for minimizing, that was very helpful.

If I combine one and two into a single crate like this:

macro_rules! define_call_with_type {
    // if you replace `ty` with `tt` here, it compiles with the latest nightly
    ($ty: ty) => {
        macro_rules! call_with_type {
            ($m:ident) => { $m! { $ty } }
        }
    };
}

define_call_with_type!(u64);

macro_rules! type_matcher {
    (u64) => { u64 };
}

fn main() {
    let _x: call_with_type!(type_matcher) = 42;
}

then both the old compiler and the new compiler give the same error (modulo some very slight wording changes in the error message). I believe this is the correct behaviour here, and likewise giving an error when the code is split across two crates is also the correct behaviour. In other words, this code is invalid, the old compiler was incorrectly accepting it when it was split across two crates due to a bug in the handling of declarative macros across crate boundaries, and #133436 fixed the bug. This matches the handful of other failures seen in the crater run. (I don't know why async-usercalls didn't show up as a failure in that crater run.)

So I think there is nothing to be done on the compiler side, and the appropriate course of action is to fix the problem in async-usercalls. As the error message explains, changing the ty fragment specifier to tt is enough to fix the reduced test case, and hopefully the fix in the full crate is equally simple. Apologies for the inconvenience!

@apiraino apiraino removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-untriaged Untriaged performance or correctness regression. labels Mar 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. 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