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

:! generic syntax #676

Merged
merged 11 commits into from
Aug 4, 2021
Merged

:! generic syntax #676

merged 11 commits into from
Aug 4, 2021

Conversation

josh11b
Copy link
Contributor

@josh11b josh11b commented Jul 26, 2021

This implements decision #565 to use T:! Type to declare generic parameters, and template T:! Type for template parameters.

@josh11b josh11b added the proposal A proposal label Jul 26, 2021
@josh11b josh11b added this to Draft in Proposals via automation Jul 26, 2021
@josh11b josh11b requested a review from a team July 26, 2021 18:25
@google-cla google-cla bot added the cla: yes PR meets CLA requirements according to bot. label Jul 26, 2021
@josh11b josh11b marked this pull request as ready for review July 28, 2021 21:23
@josh11b josh11b requested a review from a team as a code owner July 28, 2021 21:23
Copy link
Contributor

@zygoloid zygoloid 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 fine to me.

proposals/p0676.md Outdated Show resolved Hide resolved
Co-authored-by: Richard Smith <[email protected]>
Copy link
Contributor

@chandlerc chandlerc 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 this. FWIW, I think this summary is really great. This was a complex issue with a lot of discussion. I think this does a great job of pulling together a good overview of the thinking that went into the decision.

The change LGTM, but we should get at least one more thumbs up here before landing.

@jonmeow
Copy link
Contributor

jonmeow commented Jul 29, 2021

Is this proposal intended to be an RFC? It's currently categorized as draft.

@josh11b
Copy link
Contributor Author

josh11b commented Jul 29, 2021

Is this proposal intended to be an RFC? It's currently categorized as draft.

Yes, it is an RFC. It appears there are two independent ways it can be marked as a draft? @jonmeow

@josh11b josh11b moved this from Draft to RFC in Proposals Jul 29, 2021
@github-actions github-actions bot added the proposal rfc Proposal with request-for-comment sent out label Jul 29, 2021
@jonmeow
Copy link
Contributor

jonmeow commented Jul 29, 2021

Kind of, but switching project columns is the only one that really works. Switching project columns should also take care of the rest.

Copy link
Contributor

@jonmeow jonmeow left a comment

Choose a reason for hiding this comment

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

LGTM given zygoloid/chandlerc are okay, but I'll note I don't understand the indicated alternative.

Comment on lines 139 to 142
#### Square brackets

Using `[`...`]` consistently for generics creates the opposite problem of the
brackets being inconsistent with the deduced or explicit distinction.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I'm the only one confused by this, but aren't you using square brackets for the proposal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Square brackets are only used to indicate "deduced" in the proposal. Generics are indicated using :!. A generic parameter can be in the deduced list or in the regular (...) explicit parameter list. For now, the only example we have of non-generic deduced parameters is me: Self, but eventually we will probably have more.

Copy link
Contributor

Choose a reason for hiding this comment

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

But what was this alternative? Can you give an example of what was considered?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chandlerc Can you recall what was actually considered? I could potentially just delete this little bit if it doesn't match what you actually discussed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I think I found the example in the discussion notes, I'll add it to the doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wrote down the example, but it may have fallen under "We never really broke out of the idea that [...] were for deduced parameters" and wasn't actually considered. Perhaps I should include something in the "alternatives not considered" about figuring out which parameters are deduced the same way C++ does?

Copy link
Contributor

Choose a reason for hiding this comment

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

The example matches my memory -- this was exactly parallel to <>s and didn't try to get to a consistent story. I think what you have here is right, and the "not considered" thing at the bottom already captures well the space we didn't spend time exploring.

Copy link
Contributor

@chandlerc chandlerc left a comment

Choose a reason for hiding this comment

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

LGTM given the thumbs up and some time today for folks to look.

If there is a useful clarification for the open comment, can incorporate that or make a follow-up PR -- I'm pretty happy.

@chandlerc chandlerc moved this from RFC to Accepted in Proposals Jul 29, 2021
@github-actions github-actions bot added proposal accepted Decision made, proposal accepted and removed proposal rfc Proposal with request-for-comment sent out labels Jul 29, 2021
var to: Vector[i64] = CastAVector[i64](from);
```

One concern is that use of square brackets will be in the same contexts as
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zygoloid Could you remind me of your other concern about this option?

Copy link
Contributor

Choose a reason for hiding this comment

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

The other concern was that we might want to put generic / template parameters in the (...) list. Motivation for this can be found here: http:https://open-std.org/JTC1/SC22/WG21/docs/papers/2019/p1045r1.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

@josh11b josh11b merged commit 470ea2c into carbon-language:trunk Aug 4, 2021
@josh11b josh11b deleted the generics-syntax branch August 4, 2021 21:27
chandlerc pushed a commit that referenced this pull request Jun 28, 2022
This implements decision #565 to use `T:! Type` to declare generic parameters, and `template T:! Type` for template parameters.

Co-authored-by: Richard Smith <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes PR meets CLA requirements according to bot. proposal accepted Decision made, proposal accepted proposal A proposal
Projects
No open projects
Proposals
Accepted
Development

Successfully merging this pull request may close these issues.

None yet

4 participants