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

create a valid DefIdTable for proc macro crates #53711

Merged
merged 2 commits into from
Aug 29, 2018

Conversation

arielb1
Copy link
Contributor

@arielb1 arielb1 commented Aug 25, 2018

At least the incremental compilation code, and a few other places in the
compiler, require the CrateMetadata for a loaded target crate to contain a
valid DefIdTable for the DefIds in the target.

Previously, the CrateMetadata for a proc macro contained the crate's
"host" DefIdTable, which is of course incompatible with the "target"
DefIdTable, causing ICEs. This creates a DefIdTable that properly refers
to the "proc macro" DefIds.

Fixes #49482.

r? @michaelwoerister

Should we beta-nominate this?

@arielb1
Copy link
Contributor Author

arielb1 commented Aug 25, 2018

r? @michaelwoerister

At least the incremental compilation code, and a few other places in the
compiler, require the CrateMetadata for a loaded target crate to contain a
valid DefIdTable for the DefIds in the target.

Previously, the CrateMetadata for a proc macro contained the crate's
"host" DefIdTable, which is of course incompatible with the "target"
DefIdTable, causing ICEs. This creates a DefIdTable that properly refers
to the "proc macro" DefIds.

Fixes rust-lang#49482.
@michaelwoerister
Copy link
Member

Thanks a lot, @arielb1!

@bors r+

@bors
Copy link
Contributor

bors commented Aug 27, 2018

📌 Commit 025d014 has been approved by michaelwoerister

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 27, 2018
@michaelwoerister michaelwoerister added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Aug 27, 2018
@bors
Copy link
Contributor

bors commented Aug 29, 2018

⌛ Testing commit 025d014 with merge ca0de63...

bors added a commit that referenced this pull request Aug 29, 2018
create a valid DefIdTable for proc macro crates

At least the incremental compilation code, and a few other places in the
compiler, require the CrateMetadata for a loaded target crate to contain a
valid DefIdTable for the DefIds in the target.

Previously, the CrateMetadata for a proc macro contained the crate's
"host" DefIdTable, which is of course incompatible with the "target"
DefIdTable, causing ICEs. This creates a DefIdTable that properly refers
to the "proc macro" DefIds.

Fixes #49482.

r? @michaelwoerister

Should we beta-nominate this?
@bors
Copy link
Contributor

bors commented Aug 29, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: michaelwoerister
Pushing ca0de63 to master...

@bors bors merged commit 025d014 into rust-lang:master Aug 29, 2018
@pietroalbini pietroalbini added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 30, 2018
@nikomatsakis
Copy link
Contributor

cc @rust-lang/compiler

I'm torn on whether to backport this. The fix is a bit large — I don't have a good read on our "confidence level" that it is correct. I guess a worthy question is how much the people who filed #49482 care if it is backported. I'll ask on the issue :)

@glebpom
Copy link

glebpom commented Sep 4, 2018

@nikomatsakis Regarding the possible backporting: I generally fixed this issue in my code by dropping some of the dependencies with a bit of code rework. As soon as there are few users affected by the issue, I think there is no strong need to backport this to beta.

@pnkfelix
Copy link
Member

pnkfelix commented Sep 6, 2018

discussed in compiler team meeting. Team decided PR is too big, and the bug sufficiently low rsk, that we will not backport to beta.

@pnkfelix pnkfelix removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Sep 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants