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

Add an intital HIR and lowering step #28138

Merged
merged 1 commit into from
Sep 3, 2015
Merged

Add an intital HIR and lowering step #28138

merged 1 commit into from
Sep 3, 2015

Conversation

nrc
Copy link
Member

@nrc nrc commented Sep 1, 2015

r? @nikomatsakis

Trying to land this first stab, which basically just duplicates the AST. Will file issues for the various things I've got in mind to improve.

@bors
Copy link
Contributor

bors commented Sep 1, 2015

☔ The latest upstream changes (presumably #28136) made this pull request unmergeable. Please resolve the merge conflicts.

@nikomatsakis
Copy link
Contributor

is it possible to separate out the ast => hir renamings and other "hyper-rote" fallback into a distinct commit?

@nikomatsakis
Copy link
Contributor

ok so this seems fine, r+ modulo the various nits. Per IRC discussion, I think the duplication is ok as long as have a plan to remove it.

@jroesch
Copy link
Member

jroesch commented Sep 1, 2015

Looked through the majority of this as well, the duplicated logic would be worrisome in the long term, but it sounds like you will relatively quickly refactor and remove it. I also like the approach of landing this chunk before it gets too big. 👍

@nrc
Copy link
Member Author

nrc commented Sep 2, 2015

@bors: r=nikomatsakis p=1

@bors
Copy link
Contributor

bors commented Sep 2, 2015

📌 Commit facdf2e has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Sep 2, 2015

⌛ Testing commit facdf2e with merge cd138dc...

bors added a commit that referenced this pull request Sep 2, 2015
r? @nikomatsakis 

Trying to land this first stab, which basically just duplicates the AST. Will file issues for the various things I've got in mind to improve.
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.

4 participants