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 some explanation of lowering #148

Merged
merged 5 commits into from
Jul 8, 2018
Merged

Add some explanation of lowering #148

merged 5 commits into from
Jul 8, 2018

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jun 1, 2018

mark-i-m
mark-i-m suggested changes Jun 2, 2018
View reviewed changes
Copy link
Member

@mark-i-m mark-i-m left a comment

Choose a reason for hiding this comment

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

This is great! It fills in a long-standing gap :)

A couple of minor requests

  • please add this in SUMMARY.md as a subchapter of the HIR chapter (so it can be found from the Table of Contents)
  • Could you also add a link to the end of the first paragraph of the HIR chapter. Something like "For more info on lowering, see ..."

Thanks!

@oli-obk
Copy link
Contributor Author

oli-obk commented Jun 2, 2018

sure.

Note that I just pushed this because @QuietMisdreavus was having the exact troubles I had and I already had this half finished chapter. I'll fix up your nits next week along with some more information on DefId generation during lowering

@nikomatsakis nikomatsakis mentioned this pull request Jun 4, 2018
Closed
4 tasks
@mark-i-m
Copy link
Member

@oli-obk Ping :)

1 similar comment
@mark-i-m
Copy link
Member

mark-i-m commented Jul 4, 2018

@oli-obk Ping :)

@oli-obk
Copy link
Contributor Author

oli-obk commented Jul 5, 2018

Addressed review comments and added some DefId info

@oli-obk oli-obk changed the title WIP: Add some explanation of lowering Add some explanation of lowering Jul 6, 2018
@mark-i-m
Copy link
Member

mark-i-m commented Jul 8, 2018

cc #17

mark-i-m
mark-i-m approved these changes Jul 8, 2018
View reviewed changes
Copy link
Member

@mark-i-m mark-i-m left a comment

Choose a reason for hiding this comment

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

Thanks :)

@mark-i-m mark-i-m merged commit c52d026 into master Jul 8, 2018
@oli-obk oli-obk deleted the lowering branch October 1, 2018 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mark-i-m mark-i-m mark-i-m approved these changes

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
@oli-obk @mark-i-m