Skip to content

[refactoring] MIR stores the return_ty field in two places! #46001

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

Closed
nikomatsakis opened this issue Nov 15, 2017 · 3 comments
Closed

[refactoring] MIR stores the return_ty field in two places! #46001

nikomatsakis opened this issue Nov 15, 2017 · 3 comments
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

Right now, the MIR has a special field, return_ty:

/// Return type of the function.
pub return_ty: Ty<'tcx>,

However, the MIR also has an array of locals, and the 0th local is the "return slot". Its type is the return type:

/// Declarations of locals.
///
/// The first local is the return value pointer, followed by `arg_count`
/// locals for the function arguments, followed by any user-declared
/// variables and temporaries.
pub local_decls: LocalDecls<'tcx>,

It would be nice to eliminate the redundancy. My preference would be to remove the return_ty field, perhaps adding an accessor to Mir<'tcx> like:

fn return_ty(&self) -> Ty<'tcx> {
    self.local_vars[RETURN_POINTER].ty
}
@nikomatsakis nikomatsakis added C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 15, 2017
@nikomatsakis
Copy link
Contributor Author

I marked this as E-mentor, the mentoring instructions are inline above. Just remove the field in question and follow the thread of the compiler. Feel free to pop in on gitter with any questions that come up!

@j-haj
Copy link

j-haj commented Nov 15, 2017

I would like to give this a shot

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Nov 18, 2017
@sodiumjoe
Copy link

looks like this is done and can be closed?

@arielb1 arielb1 closed this as completed Dec 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. 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

4 participants