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

Matching redeclarations #3763

Merged
merged 13 commits into from May 20, 2024

Conversation

zygoloid
Copy link
Contributor

@zygoloid zygoloid commented Mar 8, 2024

Require exact syntactic matching in redeclarations. Provide new terminology for
redeclaration matching and agreement. Specify non-redeclaration rules for the
other contexts where we require multiple declarations to match, such as impls
of interfaces, impls of virtual fns.

@zygoloid zygoloid added proposal A proposal proposal draft Proposal in draft, not ready for review labels Mar 8, 2024
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.

I had a couple questions about extern handling as mentioned here. I skimmed through everything else and 👍

proposals/p3763.md Show resolved Hide resolved
proposals/p3763.md Outdated Show resolved Hide resolved
proposals/p3763.md Outdated Show resolved Hide resolved
proposals/p3763.md Outdated Show resolved Hide resolved
proposals/p3763.md Outdated Show resolved Hide resolved
@zygoloid zygoloid changed the title Redeclarations Matching redeclarations Mar 13, 2024
@zygoloid zygoloid marked this pull request as ready for review March 13, 2024 22:31
@github-actions github-actions bot requested a review from chandlerc March 13, 2024 22:32
@github-actions github-actions bot added proposal rfc Proposal with request-for-comment sent out and removed proposal draft Proposal in draft, not ready for review labels Mar 13, 2024
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.

Largely looks good. Mostly cosmetic comments on my end except for the thunk part for impl functions. But even that is really just trying to find a somewhat cleaner model, not suggesting shifting direction.

proposals/p3763.md Outdated Show resolved Hide resolved
proposals/p3763.md Outdated Show resolved Hide resolved
proposals/p3763.md Show resolved Hide resolved
proposals/p3763.md Outdated Show resolved Hide resolved
proposals/p3763.md Outdated Show resolved Hide resolved
@zygoloid zygoloid requested a review from chandlerc March 27, 2024 20:47
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.

Sorry this somewhat fell off my radar, I think there's mostly just a wording/nuance tweak that I'd like to try to get working before landing still...

proposals/p3763.md Outdated Show resolved Hide resolved
proposals/p3763.md Outdated Show resolved Hide resolved
@zygoloid zygoloid mentioned this pull request May 11, 2024
proposals/p3763.md Show resolved Hide resolved
proposals/p3763.md Show resolved Hide resolved
@chandlerc chandlerc requested a review from jonmeow May 14, 2024 01:56
proposals/p3763.md Show resolved Hide resolved
proposals/p3763.md Show resolved Hide resolved
Co-authored-by: Chandler Carruth <[email protected]>
proposals/p3763.md Outdated Show resolved Hide resolved
Co-authored-by: Carbon Infra Bot <[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.

LG, shipping it!

@chandlerc chandlerc added this pull request to the merge queue May 20, 2024
Merged via the queue into carbon-language:trunk with commit 7fc69c0 May 20, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal rfc Proposal with request-for-comment sent out proposal A proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants