Skip to content

Summarize the lecture of ty into a chapter #530

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 6 commits into from
Jan 3, 2020

Conversation

spastorino
Copy link
Member

@spastorino spastorino commented Nov 27, 2019

This is coming out as part of the effort from the @rust-lang/wg-learning working group 🎉 🎉 🎉.

As currently is I have two comments that may worth fixing:

  1. This probably needs a very deep review in the way things are located and explained. I may have messed some stuff after adding a lot of content and after merging with what the guide already had. Right now I need a little break from this, maybe when I come back I will realize all my mistakes 😄
  2. There are probably a lot of style mistakes, this was just exported from Paper.

This may be better fixed using this link, then I can place the fixes into the PR.

Closes #375

@mark-i-m
Copy link
Member

cc @nikomatsakis who gave the lecture...

@amanjeev
Copy link
Member

Who reviews this? Niko?

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.

a few random comments from a quick skim through the first little bit

@spastorino spastorino force-pushed the add-ty-chapter branch 4 times, most recently from 8420959 to b7db53f Compare December 5, 2019 21:41
@spastorino
Copy link
Member Author

This is now ready for review. CI failures are unrelated. cc @nikomatsakis

@spastorino spastorino force-pushed the add-ty-chapter branch 2 times, most recently from 2fb7d70 to f2f1325 Compare December 5, 2019 22:03
@nikomatsakis nikomatsakis self-assigned this Dec 6, 2019
@Mark-Simulacrum
Copy link
Member

I will be fixing the CI failure due to changes of how early lints are defined today, which should hopefully fix CI on this PR.

@mark-i-m
Copy link
Member

mark-i-m commented Dec 6, 2019

@Mark-Simulacrum : @JohnTitor made a PR to fix the links, but we would still really appreciate a PR to update the section on early lints, if you have time :)

@mark-i-m
Copy link
Member

Ping @nikomatsakis :)

We would love to get this merged

where they can be (so, if they do not contain any inference variables or other "temporary" types,
they will be allocated in the global arena). However, the lifetime `'tcx` is always a safe
approximation, so that is what you get back.

Choose a reason for hiding this comment

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

Would be interested to know examples of when an arena would start and stop - is global=crate level?

Copy link
Member

Choose a reason for hiding this comment

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

I believe global means the whole compilation (ie the whole crate graph)

Copy link
Member Author

Choose a reason for hiding this comment

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

@gilescope thanks for noting that. Can you open an issue that ask for that information?. I think would be better if we get this PR in and we continue with follow ups.

src/ty.md Outdated

A bit more deeply, this corresponds to structs in Rust being *nominal* types — which means that they
are defined by their *name* (and that their contents are then indexed from the definition of that
name, and not carried along “within” the type itself).

Choose a reason for hiding this comment

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

Would it be possible having set out the above to explain how to query if a ty:Ty implements a specific Trait?
(Thank you so much for this chapter - it is like the rosetta stone)

Copy link
Member

Choose a reason for hiding this comment

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

@gilescope Perhaps take a look at the trait-solving chapters (already exist). Does that answer your question?

@mark-i-m
Copy link
Member

Ping @spastorino Could you address the review comments above? Also, is the hackmd still the canonical version of the chapter or this PR?

Ping @nikomatsakis feedback.await :)

@spastorino
Copy link
Member Author

Ping @spastorino Could you address the review comments above? Also, is the hackmd still the canonical version of the chapter or this PR?

The Hackmd document is the canonical version of the chapter yeah. When some review is made I sync both things.
About the reviews above I'm going to check them in a bit.

nikomatsakis
nikomatsakis previously approved these changes Dec 30, 2019
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 awesome! I left a bunch of comments. If you like, I can try to apply some of the edits myself. Let me know. In general, I think we should hurry and land this, and we can do successive edits and improvements as follow-up PRs -- but I did find some important inaccuracies that ought to be addressed.

@mark-i-m
Copy link
Member

@spastorino I addressed some of the low-hanging fruit in the hackmd. Also, it looks like the followup PR should create the following (sub)chapters:

  • Allocation, Interning
  • TypeFolder, TypeFoldable
  • Generics and Substitutions

@spastorino
Copy link
Member Author

@mark-i-m mark-i-m force-pushed the add-ty-chapter branch 2 times, most recently from 1364945 to 2031499 Compare January 2, 2020 20:34
@mark-i-m
Copy link
Member

mark-i-m commented Jan 2, 2020

@rust-lang/wg-learning I've resolved the remaining comments and rebased over master. The hackmd and this PR should be consistent.

If it looks good, then let's go ahead and merge, and subsequent comments can go in a followup.

Both @spastorino and I are going to be unavailable next week, btw...

@nikomatsakis nikomatsakis merged commit 2de64f7 into rust-lang:master Jan 3, 2020
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.

Transcribe the Compiler Lecture Representing types in rustc
7 participants