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

librustc: Don't pass around Session with @ and make building_library just bool instead of @mut bool. #9551

Closed
wants to merge 1 commit into from

Conversation

luqmana
Copy link
Member

@luqmana luqmana commented Sep 27, 2013

Having to dereference sess.building_library is really annoying so this fixes that! Also, Session is no longer passed around with @.

2 caveats, due to pervasive use of @ in rustc there are 2 points where I just transmute to a &'static Session to put into Resolver and ty::ctxt which is probably fine given that the session will outlive them.

@@ -864,12 +867,10 @@ pub fn Resolver(session: Session,

/// The main resolver class.
pub struct Resolver {
session: @Session,
session: Cell<Session>,
Copy link
Member

Choose a reason for hiding this comment

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

It seems weird to have Cell inside of a structure, it's normally only ever used for transferring ownership between closures. What's the use-case here?

@alexcrichton
Copy link
Member

My concern over the cell should get alleviated once #9556 lands

@huonw
Copy link
Member

huonw commented Sep 28, 2013

#9556 has landed, and this needs a rebase.

@alexcrichton
Copy link
Member

I'm a little uneasy about the transmute to &'static Session. We should really avoid unsafe code and if we do resort to it there should be a very clear reason as to why. Here it's not quite clear why it's necessary. If we do get a benefit to memory usage or performance it's probably worth it, otherwise maybe not so much.

// since we know the session will outlive it, we just cheat
// and tell it the session has a static lifetime
let s: &'static session::Session = unsafe {
cast::transmute(s)
Copy link
Member

Choose a reason for hiding this comment

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

I think that this transmute in particular is not a safe assumption. Someone using the compiler libraries could fairly easily construct a situation in which the tcx outlives the session (unintentionally), only to get befuddled once segfaults start cropping up.

@luqmana
Copy link
Member Author

luqmana commented Oct 16, 2013

Closing this for now.

@luqmana luqmana closed this Oct 16, 2013
@luqmana luqmana deleted the blnam branch May 9, 2014 20:06
flip1995 pushed a commit to flip1995/rust that referenced this pull request Oct 6, 2022
…r=xFrednet

[`unnecessary_lazy_evaluations`] Do not suggest switching to early evaluation when type has custom `Drop`

fix rust-lang#9427

changelog: [`unnecessary_lazy_evaluations`] Do not suggest switching to early evaluation when type has custom `Drop`
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.

3 participants