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

Singular extern declarations #3980

Merged
merged 19 commits into from
Aug 8, 2024
Merged

Conversation

jonmeow
Copy link
Contributor

@jonmeow jonmeow commented May 23, 2024

Each entity is restricted to one, optional extern declaration. If used, it must be imported by the defining library. The defining library annotates the existence of an extern with the has_extern modifier.

@jonmeow jonmeow added proposal A proposal proposal draft Proposal in draft, not ready for review labels May 23, 2024
@jonmeow jonmeow force-pushed the proposal-extern branch from ea5e332 to 128118a Compare May 23, 2024 16:52
@jonmeow jonmeow changed the title extern Singular extern declarations May 23, 2024
@jonmeow jonmeow force-pushed the proposal-extern branch 7 times, most recently from e5f5c2f to 8c2f391 Compare May 23, 2024 23:37
@jonmeow jonmeow marked this pull request as ready for review May 23, 2024 23:37
@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 May 23, 2024
@github-actions github-actions bot requested a review from zygoloid May 23, 2024 23:38
proposals/p3980.md Outdated Show resolved Hide resolved
proposals/p3980.md Outdated Show resolved Hide resolved
proposals/p3980.md Outdated Show resolved Hide resolved
proposals/p3980.md Outdated Show resolved Hide resolved
@jonmeow jonmeow force-pushed the proposal-extern branch from 601a615 to d59f2af Compare May 24, 2024 20:24
@jonmeow jonmeow force-pushed the proposal-extern branch from f7c7f89 to d6f07f9 Compare May 24, 2024 22:34
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.

Looks good to me other than the open leads issue around has_extern syntax.

proposals/p3980.md Outdated Show resolved Hide resolved
@jonmeow jonmeow force-pushed the proposal-extern branch from d6f07f9 to 0116d26 Compare June 6, 2024 18:01
@jonmeow jonmeow force-pushed the proposal-extern branch from 0116d26 to 91ea4a0 Compare July 3, 2024 23:39
@jonmeow jonmeow force-pushed the proposal-extern branch from f7a5408 to 4db1e66 Compare July 9, 2024 23:37
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, overall I think the proposal looks great. Added some super minor suggestions in line, but they don't seem to be very significant.

I did suggest another attempt at writing up rationale for the alternative, although I'm not confident in the words I ended up with either. Maybe you or others have better ways of summarizing this.

In general, with whatever improvements to the alternative syntax rationale we converge on, I'm happy with this landing.

proposals/p3980.md Outdated Show resolved Hide resolved
proposals/p3980.md Outdated Show resolved Hide resolved
proposals/p3980.md Outdated Show resolved Hide resolved
proposals/p3980.md Outdated Show resolved Hide resolved
proposals/p3980.md Outdated Show resolved Hide resolved
jonmeow and others added 3 commits July 15, 2024 09:09
@jonmeow jonmeow force-pushed the proposal-extern branch 3 times, most recently from a121a62 to 0564943 Compare July 17, 2024 22:22
proposals/p3980.md Outdated Show resolved Hide resolved
@jonmeow
Copy link
Contributor Author

jonmeow commented Jul 24, 2024

FYI, added extended notes on syntactic matching (specifically, why it won't apply for extern library).

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, and approved for leads!

@chandlerc chandlerc added this pull request to the merge queue Aug 8, 2024
Merged via the queue into carbon-language:trunk with commit 0ac4711 Aug 8, 2024
13 checks passed
@jonmeow jonmeow deleted the proposal-extern branch August 8, 2024 23:20
github-merge-queue bot pushed a commit that referenced this pull request Aug 21, 2024
Trying to pull in key elements of #3762, #3763, and #3980 (decl matching
and `extern`, essentially). These aren't specific to any particular
declaration type, but are common to entities, so suggesting a new doc
oriented on that.

There's probably more that could be said here, I'm just focused on
getting the recent formal discussion mirrored into the design.

---------

Co-authored-by: josh11b <[email protected]>
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.

4 participants