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

Principle: All APIs are library APIs #1280

Merged
merged 7 commits into from
Jun 13, 2022
Merged

Principle: All APIs are library APIs #1280

merged 7 commits into from
Jun 13, 2022

Conversation

geoffromer
Copy link
Contributor

No description provided.

@geoffromer geoffromer added the proposal A proposal label May 19, 2022
@geoffromer geoffromer marked this pull request as ready for review May 19, 2022 20:37
@geoffromer geoffromer requested review from a team as code owners May 19, 2022 20:37
@github-actions github-actions bot added the proposal rfc Proposal with request-for-comment sent out label May 19, 2022
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.

Looks really good to me, but would like to double check w/ other leads as well.

A minor enhancement to the rationale suggested as well.

Comment on lines +76 to +84
### Built-in primitive types

We could follow the general outline of C++, where arithmetic and pointer types
are built-in. However, that would substantially erode the advantages outlined
above. We expect Carbon to have multiple kinds of pointers (for example, to
represent different kinds of ownership), and multiple kinds of arithmetic types
(for example, to handle overflow in different ways). They can't all be built-in,
so putting even the common-case types in the library helps ensure that Carbon
has enough expressive power for the uncommon-case library types.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe worth calling out that C++ specifically has ended up with built-in types becoming inconsistent with user-defined types, and this has actually caused real problems with generic code (such as in templates) that is attempting to support both?

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM (with the suggested typo fix)

Actually, I can just merge this with the typo fix, it already tracks who it is authored by. Of course, we can fix-forward if there is any issue uncovered here.

Sorry for the noise. I forgot I suggested an enhanced rationale for not following C++ here. Let's see if that makes sense or not before merging w/o it...

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.

Gah, mashed the wrong button. Requesting changes until I confirm w/ other leads.

@chandlerc
Copy link
Contributor

Oh, I also wanted to explicitly say I like describing this as "the APIs are library APIs" -- I think that does a great job of capturing the goal here.

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 great to me.

docs/project/principles/library_apis_only.md Outdated Show resolved Hide resolved
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 (with the suggested typo fix)

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
@chandlerc
Copy link
Contributor

LGTM (with the suggested typo fix)

Actually, I can just merge this with the typo fix, it already tracks who it is authored by. Of course, we can fix-forward if there is any issue uncovered here.

@geoffromer geoffromer merged commit f792e63 into carbon-language:trunk Jun 13, 2022
@geoffromer geoffromer deleted the library-apis branch June 13, 2022 20:10
@github-actions github-actions bot added proposal accepted Decision made, proposal accepted and removed proposal rfc Proposal with request-for-comment sent out labels Jun 13, 2022
chandlerc added a commit that referenced this pull request Jun 28, 2022
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
@chandlerc chandlerc added the documentation An issue or proposed change to our documentation label Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation An issue or proposed change to our documentation proposal accepted Decision made, proposal accepted proposal A proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants