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

PEP 695: Fix/improve syntax & content of Language Survey section #2725

Merged
merged 5 commits into from
Aug 28, 2022

Conversation

KotlinIsland
Copy link
Contributor

@KotlinIsland KotlinIsland commented Jul 22, 2022

This fixes up some of the inaccuracies in the 'Language Survey' section of the pep and adds a couple more languages.

@KotlinIsland KotlinIsland requested a review from gvanrossum as a code owner July 22, 2022 04:51
@KotlinIsland KotlinIsland force-pushed the pep-695 branch 5 times, most recently from f661b8e to 7f70b1f Compare July 22, 2022 05:01
@KotlinIsland KotlinIsland changed the title PEP 696: Fix Kotlin/Java details and add Dart PEP 695: Fix Kotlin/Java details and add Dart Jul 22, 2022
@KotlinIsland KotlinIsland changed the title PEP 695: Fix Kotlin/Java details and add Dart (🐞) PEP 695: Fix Kotlin/Java details and add Dart Jul 22, 2022
@KotlinIsland KotlinIsland force-pushed the pep-695 branch 6 times, most recently from f25f603 to 3430324 Compare July 22, 2022 06:34
@KotlinIsland KotlinIsland changed the title (🐞) PEP 695: Fix Kotlin/Java details and add Dart (🐞) PEP 695: Fix Language Survey section Jul 22, 2022
@hugovk hugovk changed the title (🐞) PEP 695: Fix Language Survey section PEP 695: Fix Language Survey section Jul 22, 2022
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Its up to @erictraut and @gvanrossum on the content, but I had some minor syntactic and textual fixes.

pep-0695.rst Outdated Show resolved Hide resolved
pep-0695.rst Outdated Show resolved Hide resolved
pep-0695.rst Outdated Show resolved Hide resolved
pep-0695.rst Outdated Show resolved Hide resolved
pep-0695.rst Outdated Show resolved Hide resolved
pep-0695.rst Outdated Show resolved Hide resolved
pep-0695.rst Outdated Show resolved Hide resolved
@gvanrossum
Copy link
Member

@erictraut For you to review.

@KotlinIsland KotlinIsland force-pushed the pep-695 branch 5 times, most recently from 5178320 to c2ba383 Compare July 23, 2022 04:01
@KotlinIsland KotlinIsland force-pushed the pep-695 branch 3 times, most recently from 11d3166 to a70eff5 Compare July 23, 2022 07:31
Copy link
Contributor

@erictraut erictraut left a comment

Choose a reason for hiding this comment

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

Thanks for the edits and corrections. Just one small nit, but overall looks good to me.

pep-0695.rst Show resolved Hide resolved
usage, not specified explicitly. TypeScript 4.7 introduced the ability
to specify variance using ``in`` and ``out`` keywords. This was added to handle
extremely complex types where inference of variance was expensive,
yet the maintainers state that is useful for increasing readability.
Copy link
Contributor

Choose a reason for hiding this comment

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

Readability was not the motivation for adding this capability. I meet often with the TypeScript team, and I'm very familiar with their motivations and the debates that led to their (reluctant) decision to add this capability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wasn't a motivation, but my edit here was based on information from the documentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you provide some references for these reasons? Reading the pr microsoft/TypeScript#48240, it seems to lead with positive motivations and nothing negative that I can see:

Indeed, when generic type instantiations are related structurally, variance annotations serve no purpose. This is why TypeScript strictly doesn't need variance annotations. However, variance annotations are useful to assert desired type relations of their subject generic types.

Copy link
Contributor

Choose a reason for hiding this comment

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

This information comes from direct conversations with Anders Hejlsberg, Ryan Cavanaugh, and Daniel Rosenwasser, the leads on the TypeScript team. I've discussed the arguments you've raised about variance. Here's how Daniel summarized their advice:
"our perspective is that ideally we wouldn't have added [explicit] variance if we didn't need to"

@Noratrieb
Copy link

Noratrieb commented Aug 4, 2022

Hi, I found this PR and wanted to add something about variance in Rust: As mentioned already, Rust doesn't have classic type subtyping but only for lifetimes. But, this still means that variance matters in Rust (because types can contain lifetimes), and there is a certain variance associated with every type parameter. Variance is always inferred by looking at the usage at the declaration site. So for example, if T is stored a normal owned value, it will be covariant. If it's stored behind a mutable reference, it will be invariant.

While the language itself does not provide any way to annotate the variance directly, you can control the variance manually by using the type in specific ways. There is a zero-sized-type called PhantomData<T> in the standard library, which can be used get a specific variance for a type.
For example

struct Cov<T> {
  _marker: PhantomData<fn(T) -> T>, // a function pointer where T is the input and output, 
}

will make T invariant.

This is also why an unused generic parameter is a compiler error in Rust, since it cannot possibly infer a variance for it.

@erictraut
Copy link
Contributor

@KotlinIsland, could you please address the feedback from @Nilstrieb before this PR is merged?

@KotlinIsland
Copy link
Contributor Author

@KotlinIsland, could you please address the feedback from @Nilstrieb before this PR is merged?

@erictraut I've added an example usage of lifetime subtyping to the rust section.

@erictraut
Copy link
Contributor

I'm in favor of merging this version. I have another large round of changes to the PEP that I'm actively working on, and I'd prefer to get this merged in now to avoid merge conflicts.

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Scanning the source and preview, the changes are to non-normative sections, improve the syntax and rendering of the PEP (with no obvious regressions), the content is approved by the PEP author and there are no other clear defects, so ✔️ from a PEP editor perspective, thanks.

@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Aug 28, 2022

Going ahead and merging per the preference of the PEP author, and the deference of such by the PEP sponsor. Thanks @erictraut and @KotlinIsland

@CAM-Gerlach CAM-Gerlach changed the title PEP 695: Fix Language Survey section PEP 695: Fix/improve syntax & content of Language Survey section Aug 28, 2022
@CAM-Gerlach CAM-Gerlach merged commit 754c8fb into python:main Aug 28, 2022
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.

5 participants