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

Remove special case for Box<ZST> in trans #38574

Merged
merged 1 commit into from
Dec 27, 2016

Conversation

Mark-Simulacrum
Copy link
Member

Remove extra lang item, exchange_free; use box_free instead.

Trans used to insert code equivalent to box_free in a wrapper around
exchange_free, and that code is now removed from trans.

Fixes #37710.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@@ -144,7 +144,8 @@ unsafe fn exchange_malloc(size: usize, align: usize) -> *mut u8 {
}

#[cfg(not(test))]
#[lang = "exchange_free"]
#[cfg_attr(stage0, lang = "exchange_free")]
#[cfg_attr(not(stage0), allow(unused))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add just #[cfg(stage0)] so it simply doesn't exist after that.

bcx
}
let ptr = MaybeSizedValue::sized(bcx.load(ptr.value));
drop_ty(&bcx, ptr, content_ty);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can move this call out of the if/else.

@Mark-Simulacrum
Copy link
Member Author

r? @eddyb

@Mark-Simulacrum
Copy link
Member Author

Okay, this should be ready to go now. Smoke tests passed locally, but we can wait for Travis.

@Mark-Simulacrum
Copy link
Member Author

Travis failed, but it looks spurious; I can't see any problems in the build log.

@eddyb
Copy link
Member

eddyb commented Dec 24, 2016

Restarting Travis just in case.

assert!(bcx.ccx.shared().type_is_sized(content_ty));
let sizing_type = sizing_type_of(bcx.ccx, content_ty);
let content_size = llsize_of_alloc(bcx.ccx, sizing_type);
ptr.value = bcx.pointercast(ptr.value, Type::i8p(bcx.ccx));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't feel right. Isn't the function written to take *mut T? Wouldn't, then, this cast trigger a LLVM assertion?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, odd. Tests passed locally both with and without, but since I agree it's odd to have it, I've removed it. Code is cleaner that way too.

@eddyb
Copy link
Member

eddyb commented Dec 24, 2016

@bors r+

@bors
Copy link
Contributor

bors commented Dec 24, 2016

📌 Commit b4a3d22 has been approved by eddyb

@@ -725,18 +727,13 @@ fn find_drop_glue_neighbors<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>,

// Make sure the exchange_free_fn() lang-item gets translated if
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

outdated comment here?

@Mark-Simulacrum Mark-Simulacrum force-pushed the box-free-unspecialize branch 2 times, most recently from b3ef574 to adf7b50 Compare December 25, 2016 22:49
@eddyb
Copy link
Member

eddyb commented Dec 25, 2016

@bors r+

@bors
Copy link
Contributor

bors commented Dec 25, 2016

📌 Commit adf7b50 has been approved by eddyb

Trans used to insert code equivalent to box_free in a wrapper around
exchange_free, and that code is now removed from trans.
@eddyb
Copy link
Member

eddyb commented Dec 27, 2016

@bors r+

@bors
Copy link
Contributor

bors commented Dec 27, 2016

📌 Commit ca115dd has been approved by eddyb

@bors
Copy link
Contributor

bors commented Dec 27, 2016

⌛ Testing commit ca115dd with merge d849b13...

bors added a commit that referenced this pull request Dec 27, 2016
Remove special case for Box<ZST> in trans

Remove extra lang item, `exchange_free`; use `box_free` instead.

Trans used to insert code equivalent to `box_free` in a wrapper around
`exchange_free`, and that code is now removed from trans.

Fixes #37710.
@bors
Copy link
Contributor

bors commented Dec 27, 2016

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

@bors bors merged commit ca115dd into rust-lang:master Dec 27, 2016
@bors bors mentioned this pull request Dec 27, 2016
@Mark-Simulacrum Mark-Simulacrum deleted the box-free-unspecialize branch March 25, 2017 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants