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

Expansion info is reset for the mark #52363

Closed
dtolnay opened this issue Jul 14, 2018 · 11 comments · Fixed by #62476
Closed

Expansion info is reset for the mark #52363

dtolnay opened this issue Jul 14, 2018 · 11 comments · Fixed by #62476
Assignees
Labels
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.

Comments

@dtolnay
Copy link
Member

dtolnay commented Jul 14, 2018

format_args!({ #[derive(repro)] struct ProcMacroHack; });

Correct behavior on all compilers 1.15.0 through 1.28.0-beta.10:

error: format argument must be a string literal.
 --> src/main.rs:5:18
  |
5 |     format_args!({ #[derive(repro)] struct ProcMacroHack; });
  |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

But on nightly:
(UPDATE: The ICE is now suppressed, but the issue still needs investigation.)

thread 'main' panicked at 'expansion info is reset for the mark 2
old: ExpnInfo {
    call_site: Span {
        lo: BytePos(
            56
        ),
        hi: BytePos(
            113
        ),
        ctxt: #0
    },
    def_site: None,
    format: MacroBang(
        format_args
    ),
    allow_internal_unstable: true,
    allow_internal_unsafe: false,
    local_inner_macros: false,
    edition: Edition2015
}
new: ExpnInfo {
    call_site: Span {
        lo: BytePos(
            88
        ),
        hi: BytePos(
            109
        ),
        ctxt: #0
    },
    def_site: None,
    format: MacroAttribute(
        derive(repro)
    ),
    allow_internal_unstable: true,
    allow_internal_unsafe: false,
    local_inner_macros: false,
    edition: Edition2015
}', libsyntax_pos/hygiene.rs:114:17
note: Run with `RUST_BACKTRACE=1` for a backtrace.

error: internal compiler error: unexpected panic

Repro script

#!/bin/sh

cargo new repro
cargo new --lib repro_macro

cat >repro/src/main.rs <<'REPRO'
#[macro_use]
extern crate repro_macro;

fn main() {
    format_args!({ #[derive(repro)] struct ProcMacroHack; });
}
REPRO

cat >>repro/Cargo.toml <<'REPRO'
repro_macro = { path = "../repro_macro" }
REPRO

cat >repro_macro/src/lib.rs <<'REPRO'
extern crate proc_macro;
use proc_macro::TokenStream;

#[proc_macro_derive(repro)]
pub fn repro(_input: TokenStream) -> TokenStream {
    "".parse().unwrap()
}
REPRO

cat >>repro_macro/Cargo.toml <<'REPRO'
[lib]
proc-macro = true
REPRO

cargo build --manifest-path repro/Cargo.toml

Mentioning @petrochenkov because this bug was exposed by #51726.

@dtolnay dtolnay added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Jul 14, 2018
@petrochenkov petrochenkov self-assigned this Jul 14, 2018
@pnkfelix pnkfelix added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. P-high High priority labels Jul 19, 2018
@pnkfelix
Copy link
Member

visiting for triage.

@petrochenkov do you have any status update to report here?

@petrochenkov
Copy link
Contributor

No updates, low priority.
I'll look into this after edition-related macro modularization work is complete.

@petrochenkov
Copy link
Contributor

In the worst case, the issue can be masked by turning the assert into debug assert.

@pietroalbini pietroalbini added regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Jul 31, 2018
@pietroalbini pietroalbini added this to Triaged in 1.29 regressions Jul 31, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Aug 2, 2018

downgrading to P-medium to reflect lower-priority status.

@pnkfelix pnkfelix added P-medium Medium priority and removed P-high High priority labels Aug 2, 2018
@nikomatsakis
Copy link
Contributor

@petrochenkov

In the worst case, the issue can be masked by turning the assert into debug assert.

So do we want to remove the assertion and file a follow-up issue to resurrect it? Seems like we should "close the issue" by doing that?

Some discussion here

@nikomatsakis
Copy link
Contributor

(In particular, it seems a bit odd to have a debug_assert that we know fails sometimes.)

@nikomatsakis nikomatsakis added P-high High priority and removed P-medium Medium priority labels Aug 23, 2018
@nikomatsakis
Copy link
Contributor

Bumping up to P-high to "stop the bleeding"

@arielb1
Copy link
Contributor

arielb1 commented Aug 23, 2018

For easy reference: we are referring to the following assertion:

pub fn set_expn_info(self, info: ExpnInfo) {
HygieneData::with(|data| {
let old_info = &mut data.marks[self.0 as usize].expn_info;
if let Some(old_info) = old_info {
panic!("expansion info is reset for the mark {}\nold: {:#?}\nnew: {:#?}",
self.0, old_info, info);
}
*old_info = Some(info);
})
}

@petrochenkov
Copy link
Contributor

Assert is removed in #53653

@petrochenkov
Copy link
Contributor

petrochenkov commented Aug 24, 2018

I'll keep this issue open for future investigation.
Expansion data being reset like this is clearly a bug, this shouldn't happen.

@petrochenkov petrochenkov removed I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Aug 24, 2018
bors added a commit that referenced this issue Aug 24, 2018
Address two regressions

Remove assert checking that expansion data is immutable until I have time to investigate why it's firing, cc #52363
Turn the error for module-relative access to macro-expanded `macro_export` macros into a deprecation lint, closes #53495
@pietroalbini pietroalbini moved this from Triaged to Backport in progress in 1.29 regressions Aug 25, 2018
@pietroalbini pietroalbini moved this from Backport in progress to Fixed in 1.29 regressions Sep 1, 2018
@petrochenkov
Copy link
Contributor

Fixed in #62476

petrochenkov added a commit to petrochenkov/rust that referenced this issue Jul 10, 2019
…rkers

Create a fresh expansion for them instead - this is the usual way to allow unstable features for generated/desugared code.
Fixes rust-lang#52363
Centril added a commit to Centril/rust that referenced this issue Jul 11, 2019
Continue refactoring macro expansion and resolution

This PR continues the work started in rust-lang#62042.
It contains a set of more or less related refactorings with the general goal of making things simpler and more orthogonal.
Along the way most of the issues uncovered in rust-lang#62086 are fixed.

The PR is better read in per-commit fashion with whitespace changes ignored.
I tried to leave some more detailed commit messages describing the motivation behind the individual changes.

Fixes rust-lang#44692
Fixes rust-lang#52363
Unblocks rust-lang#62086
r? @matthewjasper
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, ..) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

6 participants