Skip to content

Implicit Caller Location section. #582

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

Merged
merged 2 commits into from
Apr 1, 2020
Merged

Implicit Caller Location section. #582

merged 2 commits into from
Apr 1, 2020

Conversation

anp
Copy link
Member

@anp anp commented Feb 16, 2020

Documenting the overall feature here as the current implementation is spread across several PRs and has deviated a good bit from the RFC.

recommended in rust-lang/rust#47809 (comment)

@eddyb
Copy link
Member

eddyb commented Feb 20, 2020

Probably an easy way to explain it is to show the definition and what it codegens as, both in regular Rust code (even if nothing before codegen, not even MIR, encodes the caller location passing).

@anp anp changed the title WIP Implicit Caller Location section. Implicit Caller Location section. Mar 28, 2020
@anp anp marked this pull request as ready for review March 28, 2020 22:30
@anp
Copy link
Member Author

anp commented Mar 28, 2020

cc @nikomatsakis you had specifically asked for this to be able to learn more about the feature. Would love to hear if it achieves that goal!

Copy link
Contributor

@Centril Centril left a comment

Choose a reason for hiding this comment

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

This looks like a fabulous eventual stabilization report and some great technical writing.

nikomatsakis
nikomatsakis previously approved these changes Mar 30, 2020
Copy link
Contributor

@nikomatsakis nikomatsakis 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 fantastic. I think it was just about the perfect level of detail -- outlined the high-level strategy without getting too bogged down in the implementation details. I imagine if you were paging through code, it'd be really helpful. One final note -- maybe we want to modify the code to link to this chapter? (Once it lands)

@anp
Copy link
Member Author

anp commented Mar 30, 2020

OK! Thank you for the reviews and the kind words <3. I think I've addressed all the feedback.

Once this is re-approved, it's gated on #641 and #646 to get CI green. I'll add a link from the code to this once it lands?

Centril
Centril previously approved these changes Mar 30, 2020
mark-i-m
mark-i-m previously approved these changes Apr 1, 2020
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 again @anp!

@mark-i-m
Copy link
Member

mark-i-m commented Apr 1, 2020

@anp Could you rebase on top of the most recent master? It has a bunch of link fixes.

@anp
Copy link
Member Author

anp commented Apr 1, 2020

Actually just finished doing that, those breakages were from my own links 😅 -- fixed up.

@mark-i-m mark-i-m merged commit f38f17c into rust-lang:master Apr 1, 2020
@anp anp deleted the track-caller branch April 1, 2020 02:35
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.

6 participants