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.

Sign up for GitHub

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

Jump to bottom

Add resolve_local_name to resolve names in a function scope #98

Merged
merged 4 commits into from
Oct 7, 2018
Merged

Add resolve_local_name to resolve names in a function scope #98

merged 4 commits into from
Oct 7, 2018

Conversation

kjeremy
Copy link
Contributor

@kjeremy kjeremy commented Oct 5, 2018

First step to resolving #80

@kjeremy kjeremy changed the title Add resolve_local_name to resolve names in a function scope WIP: Add resolve_local_name to resolve names in a function scope Oct 5, 2018
@kjeremy
Copy link
Contributor Author

kjeremy commented Oct 5, 2018

I think the first commit is reasonable and tests suggest that it works correctly.

If you look at the second commit (doesn't compile due to a lifetime issue) I don't think it's the right abstraction and doesn't really match what the rest of ra_editor provides. What are your thoughts?

@matklad
Copy link
Member

matklad commented Oct 5, 2018

I don't think it's the right abstraction and doesn't really match what the rest of ra_editor provides. What are your thoughts?

The overall situation with abstractions is pretty volatile at the moment. I'd love to try to switch everything to https://github.com/salsa-rs/salsa, and that might affect architecture in big ways. This is not related to this PR specifically, just something we need to keep an eye on.

@matklad
Copy link
Member

matklad commented Oct 7, 2018

@kjeremy anything else to do here?

bors: delegate

@matklad
Copy link
Member

matklad commented Oct 7, 2018

bors: delegate+

@bors
Copy link
Contributor

bors bot commented Oct 7, 2018

✌️ kjeremy can now approve this pull request

@kjeremy
Copy link
Contributor Author

kjeremy commented Oct 7, 2018

bors: r+

bors bot added a commit that referenced this pull request Oct 7, 2018
@bors @kjeremy
Merge #98
e4fdfd1 
98: WIP: Add resolve_local_name to resolve names in a function scope r=kjeremy a=kjeremy

First step to resolving #80 

Co-authored-by: Jeremy A. Kolb <jkolb@ara.com>
@kjeremy kjeremy changed the title WIP: Add resolve_local_name to resolve names in a function scope Add resolve_local_name to resolve names in a function scope Oct 7, 2018
@bors
Copy link
Contributor

bors bot commented Oct 7, 2018

Build succeeded

@bors bors bot merged commit ff1b2da into rust-lang:master Oct 7, 2018
@aochagavia aochagavia mentioned this pull request Oct 11, 2018
Closed
@kjeremy kjeremy deleted the resolve_local_name branch February 12, 2019 15:52
@sify21 sify21 mentioned this pull request Sep 16, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matklad matklad matklad left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kjeremy @matklad