diff --git a/docs/design/code_and_name_organization/README.md b/docs/design/code_and_name_organization/README.md index 6f6000945636f..1ca79425d48cb 100644 --- a/docs/design/code_and_name_organization/README.md +++ b/docs/design/code_and_name_organization/README.md @@ -339,28 +339,27 @@ for separate compilation. #### Exporting entities from an API file -In order to actually be part of a library's API, entities must both be in the -API file and explicitly marked as an API. This is done using the `api` keyword, -which is only allowed in the API file. For example: +Entities in the API file are part of the library's public API by default. They +may be marked as `private` to indicate they should only be visible to other +parts of the library. ```carbon package Geometry library "Shapes" api; -// Circle is marked as an API, and will be available to other libraries as -// Geometry.Circle. -api struct Circle { ... } +// Circle is an API, and will be available to other libraries as + Geometry.Circle. +struct Circle { ... } -// CircleHelper is not marked as an API, and so will not be available to other -// libraries. -fn CircleHelper(Circle circle) { ... } +// CircleHelper is private, and so will not be available to other libraries. +private fn CircleHelper(Circle circle) { ... } // Only entities in namespaces should be marked as an API, not the namespace // itself. namespace Operations; -// Operations.GetCircumference is marked as an API, and will be available to +// Operations.GetCircumference is an API, and will be available to // other libraries as Geometry.Operations.GetCircumference. -api fn Operations.GetCircumference(Circle circle) { ... } +fn Operations.GetCircumference(Circle circle) { ... } ``` This means that an API file can contain all implementation code for a library. @@ -374,7 +373,9 @@ However, separate implementation files are still desirable for a few reasons: - From a code maintenance perspective, having smaller files can make a library more maintainable. -Use of the `api` keyword is not allowed within files marked as `impl`. +Entities in the `impl` file should never have visibility keywords. If they are +forward declared in the `api` file, they use the declaration's visibility; if +they are only present in the `impl` file, they are implicitly `private`. #### Granularity of libraries @@ -404,7 +405,7 @@ package Checksums library "Sha" api; namespaces Sha256; -api fn Sha256.HexDigest(Bytes data) -> String { ... } +fn Sha256.HexDigest(Bytes data) -> String { ... } ``` Calling code may look like: @@ -434,7 +435,7 @@ import IDENTIFIER (library NAME_PATH)?; ``` An import declares a package entity named after the imported package, and makes -`api`-tagged entities from the imported library through it. The full name path +API entities from the imported library available through it. The full name path is a concatenation of the names of the package entity, any namespace entities applied, and the final entity addressed. Child namespaces or entities may be [aliased](/docs/design/aliases.md) if desired. @@ -444,7 +445,7 @@ For example, given a library: ```carbon package Math api; namespace Trigonometry; -api fn Trigonometry.Sin(...); +fn Trigonometry.Sin(...); ``` Calling code would import it and use it like: @@ -574,12 +575,12 @@ server for open source packages. Conflicts can also be addressed by renaming one of the packages, either at the source, or as a local modification. We do need to address the case of package names conflicting with other entity -names. It's possible that a pre-existing `api` entity will conflict with a new -import, and that the `api` is infeasible to rename due to existing callers. -Alternately, the `api` entity may be using an idiomatic name that it would -contradict naming conventions to rename. In either case, this conflict may exist -in a single file without otherwise affecting users of the API. This will be -addressed by [name lookup](/docs/design/name_lookup.md). +names. It's possible that a pre-existing entity will conflict with a new import, +and that renaming the entity is infeasible to rename due to existing callers. +Alternately, the entity may be using an idiomatic name that it would contradict +naming conventions to rename. In either case, this conflict may exist in a +single file without otherwise affecting users of the API. This will be addressed +by [name lookup](/docs/design/name_lookup.md). ### Potential refactorings @@ -816,10 +817,13 @@ should be part of a larger testing plan. - [Collapse file and library concepts](/proposals/p0107.md#collapse-file-and-library-concepts) - [Collapse the library concept into packages](/proposals/p0107.md#collapse-the-library-concept-into-packages) - [Collapse the package concept into libraries](/proposals/p0107.md#collapse-the-package-concept-into-libraries) + - [Default `api` to private](/proposals/p0752.md#default-api-to-private) + - [Default `impl` to public](/proposals/p0752.md#default-impl-to-public) - [Different file type labels](/proposals/p0107.md#different-file-type-labels) - [Function-like syntax](/proposals/p0107.md#function-like-syntax) - [Inlining from implementation files](/proposals/p0107.md#inlining-from-implementation-files) - [Library-private access controls](/proposals/p0107.md#library-private-access-controls) + - [Make keywords either optional or required in separate definitions](/proposals/p0752.md#make-keywords-either-optional-or-required-in-separate-definitions) - [Managing API versus implementation in libraries](/proposals/p0107.md#managing-api-versus-implementation-in-libraries) - [Multiple API files](/proposals/p0107.md#multiple-api-files) - [Name paths as library names](/proposals/p0107.md#name-paths-as-library-names) diff --git a/proposals/README.md b/proposals/README.md index 84ab5c6018cc1..5a02b6c54fe32 100644 --- a/proposals/README.md +++ b/proposals/README.md @@ -69,6 +69,7 @@ request: - [0680 - And, or, not](p0680.md) - [0722 - Nominal classes and methods](p0722.md) - [0731 - Generics details 2: adapters, associated types, parameterized interfaces](p0731.md) +- [0752 - `api` file default-`public`](p0752.md) - [0777 - Inheritance](p0777.md) diff --git a/proposals/p0752.md b/proposals/p0752.md new file mode 100644 index 0000000000000..3f9bec4086f11 --- /dev/null +++ b/proposals/p0752.md @@ -0,0 +1,191 @@ +# `api` file default-`public` + + + +[Pull request](https://github.com/carbon-language/carbon-lang/pull/752) + + + +## Table of contents + +- [Problem](#problem) +- [Background](#background) +- [Proposal](#proposal) +- [Rationale based on Carbon's goals](#rationale-based-on-carbons-goals) +- [Alternatives considered](#alternatives-considered) + - [Default `api` to private](#default-api-to-private) + - [Default `impl` to public](#default-impl-to-public) + - [Make keywords either optional or required in separate definitions](#make-keywords-either-optional-or-required-in-separate-definitions) + + + +## Problem + +Question for leads +[#665: private vs public _syntax_ strategy, as well as other visibility tools like external/api/etc.](https://github.com/carbon-language/carbon-lang/issues/665) +decided that methods on classes should default to public. Should `api` echo the +similar strategy? + +## Background + +- In C++, `struct` members default public, while `class` members default + `public`. +- In proposal + [#107: Code and name organization](https://github.com/carbon-language/carbon-lang/pull/107), + an `api` keyword was used to indicate public APIs within an `api` file. +- In [#665](https://github.com/carbon-language/carbon-lang/issues/665), it was + decided that Carbon class members should default `public`. + - This issue was reopened to discuss alternatives in this proposal. + +## Proposal + +APIs in the `api` file should default public, without need for an additional +`api` keyword. `private` may be specified to designate APIs that are internal to +the library, and only visible to `impl` files. + +Nothing is necessary within `impl` files, and APIs there will be private unless +forward declared in the `api` file. + +## Rationale based on Carbon's goals + +- [Code that is easy to read, understand, and write](/docs/project/goals.md#code-that-is-easy-to-read-understand-and-write): + It will be easier for developers to understand code if APIs have + semantically similar behavior when comparing the visibility of class methods + to other code, and the library to other packages. + +## Alternatives considered + +### Default `api` to private + +Default private is what was implied by `api`, and was the previous state. + +Advantages: + +- Decreases the likelihood that developers will accidentally expose APIs, + because it's an explicit choice. +- Can move functions between `api` and `impl` without visibility changing. + +Disadvantages: + +- The `api` file's primary purpose is to expose APIs, and so it may be more + natural for developers to assume things there are public. +- Inconsistent with "default public" behavior on classes. + +We are switching to default public in `api` files for consistency with class +behaviors. + +### Default `impl` to public + +Noting that we default `api` to public, we could similarly default `impl` to +public. + +Advantages: + +- Can move functions between `api` and `impl` without visibility changing. + +Disadvantages: + +- Everything in an `impl` file must be private unless it's a separate + definition of an `api` declaration. As a consequence, everything declared in + the `impl` file would need to be explicitly `private`. + +In order to avoid the toil of explicitly declaring everything in the `impl` as +`private`, `impl` will be `private` by default. As a consequence of being the +default behavior, no `private` should be specified, just as `public` is not +allowed in `api` files. + +Note the visibility behavior can be described as making declarations the most +visible possible for its context, which in `api` files is `public`, and in +`impl` is `private`. + +### Make keywords either optional or required in separate definitions + +When a prior declaration exists, keywords are _disallowed_ in separate +definitions. We could instead allow keywords, making them either optional or +required. This would allow developers to determine visibility when reading a +definition. + +The downside of this is that optional keywords can be confusing. For example: + +- `api` file: + + ``` + class Foo { + private fn Bar(); + private fn Wiz(); + }; + ``` + +- `impl` file: + + ``` + fn Foo.Bar() { ...impl... } + private fn Foo.Wiz() { ...impl... } + fn Baz() { ...impl... } + ``` + +In an "optional" setup, the above is valid code. However, the lack of a +`private` keyword on `Foo.Bar` may lead a developer to conclude that it's public +without checking the `api` file (particularly because `Foo.Wiz` is explicitly +private), when it's actually private. This is an accident that could also occur +on refactoring; for example, removing the keyword on the `impl` version of +`Foo.Wiz` would be valid but does not make it public. + +A response may be to make keywords required to match, so that reading the `impl` +file would have a compiled guarantee of correctness, avoiding confusion. +However, consider a similar example: + +- `api` file: + + ``` + class Foo { + fn Bar(); + private fn Wiz(); + }; + ``` + +- `impl` file: + + ``` + fn Foo.Bar() { ...impl... } + private fn Foo.Wiz() { ...impl... } + fn Baz() { ...impl... } + ``` + +In this example, `Foo.Bar` is public, and that may lead developers to conclude +that `Baz` is public. This could be corrected by requiring `private` on `Baz`, +but we are hesitant to do that per +[Default `impl` to public](#default-impl-to-public). + +There is still some risk of confusion if the forward declaration and separate +definition are both in the `api` file. For example: + +``` +private fn PrintLeaves(Node node); + +fn PrintNode(Node node) { + Print(node.value); + PrintLeaves(node); +); + +fn PrintLeaves(Node node) { + for (Node leaf : node.leaves) { + PrintNode(leaf); + } +} +``` + +In this, a reader may read the `PrintLeaves` definition and incorrectly conclude +that it is implicitly `public` because (a) it has no keywords and (b) it is in +the `api` file. This will be addressed as part of +[Open question: Calling functions defined later in the same file #472](https://github.com/carbon-language/carbon-lang/issues/472#issuecomment-915407683). + +Overall, the decision to _disallow_ keywords on separate definitions means that +`impl` files shouldn't have any visibility keywords at the file scope (they will +on classes), which is considered a writability improvement while keeping the +`api` as the single source of truth for `public` entities, addressing +readability.