-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Code and name organization #107
Changes from 4 commits
2ae6b6f
5bba3a8
98562a6
a05e9c3
dfe42cb
3931951
43cfde9
b489c23
5b10b4d
6ce0f97
218df9f
5c33fbf
160b5c4
5829ac9
8692c9a
6b7f469
654cad6
aa96aaf
ca5d5bd
c132018
3d99524
5813814
3154969
a546035
1222334
3cebea3
4bbba84
d7d2850
7c4c32c
d5b8ab9
3e15730
e578b1f
5319498
06dea5d
bf74c75
9559bec
af6f4b1
2311f03
db2d550
1ebecc1
54f740b
cf53d74
0d81ff4
03f92f8
a5c4e16
8d62937
1d09728
9fed213
b53a98d
50cb15b
1e55c4f
365a089
0c6c6de
8297dbe
eae6a9b
2111545
23bee19
94bd816
69d1893
4f10ddb
e56549e
1cec071
99801c1
91f989e
559677b
7c464a5
e1c46e3
60dfa21
c8ea9bb
578591b
84cd55f
882d5e0
7a0a400
f914c1b
d0ba98a
b8f9fc2
53a1072
0da274b
16ba315
687c166
31a7619
6efd78e
4d15065
441fe24
6d9f997
d9a7b34
71375af
6c4d9e3
9607815
06b31f5
8a5b20b
79270d3
754cb6b
0388e5b
3c52bb7
2a1a662
e9f4404
3ef6558
f71c05d
cd6e7b4
fe4c7d2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,7 +49,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | |
- [Referring to the package as `package`](#referring-to-the-package-as-package) | ||
- [Remove the `library` keyword from `package` and `import`](#remove-the-library-keyword-from-package-and-import) | ||
- [Rename package concept](#rename-package-concept) | ||
- [Strict association between the filesystem path and library/namespace](#strict-association-between-the-filesystem-path-and-librarynamespace) | ||
- [No association between the filesystem path and library/namespace](#no-association-between-the-filesystem-path-and-librarynamespace) | ||
- [Libraries](#libraries-1) | ||
- [Allow exporting namespaces](#allow-exporting-namespaces) | ||
- [Allow importing implementation files from within the same library](#allow-importing-implementation-files-from-within-the-same-library) | ||
|
@@ -338,9 +338,15 @@ that are implementation. | |
`package Geometry library "Shapes" api;` | ||
- API filenames must have the `.carbon` extension. They must not have a | ||
`.impl.carbon` extension. | ||
- API file paths will correspond to the library name. | ||
- The precise form of this correspondence is undetermined, but should | ||
be expected to be similar to a "Math/Algebra" library being in a | ||
"Math/Algebra.carbon" file path. | ||
- The package will not be used when considering the file path. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we mention that the API file path above is relative to some package-specific root? or not at all? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd argue for leaving that to be resolved later. I think "package-specific" could imply that each package has its own root, and I believe the discussion so far had been:
Thus the idea of making a quick remark about a package-specific root seems pretty limiting, and one that should be delayed for further proposals. I can particularly see how #2 may get arguments. |
||
- An implementation file's `package` will have `impl`. For example, | ||
`package Geometry library "Shapes" impl;`. | ||
- Implementation filenames must have the `.impl.carbon` extension. | ||
- Implementation file paths need not correspond to the library name. | ||
- Implementation files implicitly import the library's API. Implementation | ||
files cannot import each other. There is no facility for file or | ||
non-`api` imports. | ||
|
@@ -721,8 +727,8 @@ may require manually adding imports. | |
- Rename a file, or move a file between directories. | ||
|
||
- Build configuration will need to be updated. | ||
- Carbon code will not change. The `package` keyword determines how a file | ||
is imported, so the library is unaffected by filesystem location. | ||
- This additionally requires the steps to rename a library, because | ||
library names must correspond to the renamed paths. | ||
|
||
### Preference for few child namespaces | ||
|
||
|
@@ -1014,7 +1020,7 @@ Disadvantages: | |
- [Swift](https://developer.apple.com/documentation/swift_packages), as a | ||
distributable unit. | ||
|
||
#### Strict association between the filesystem path and library/namespace | ||
#### No association between the filesystem path and library/namespace | ||
|
||
Several languages create a strict association between the method for pulling in | ||
an API and the path to the file that provides it. For example: | ||
|
@@ -1038,18 +1044,12 @@ For contrast: | |
- For example, `import "PATH/TO/NAME"` means there is a directory | ||
`PATH/TO` that contains one or more files starting with `package NAME`. | ||
|
||
In Carbon, we could say that `import PACKAGE library "PATH/TO/LIBRARY"` means | ||
there are one more more files `PACKAGE/PATH/TO/LIBRARY(.impl)?.carbon`. | ||
In Carbon, we are using a strict association to say that | ||
`import PACKAGE library "PATH/TO/LIBRARY"` means there is a file | ||
`PATH/TO/LIBRARY.carbon` under some package root. | ||
|
||
Advantages: | ||
|
||
- A strict association between filesystem path and import path makes it easier | ||
to find source files. This is used by some languages for compilation. | ||
- Allows getting rid of the `package` keyword by inferring related information | ||
from the filesystem path. | ||
|
||
Disadvantages: | ||
|
||
- The strict association makes it harder to move names between files without | ||
updating callers. | ||
- If there were a strict association of paths, it would also need to handle | ||
|
@@ -1059,10 +1059,18 @@ Disadvantages: | |
`config` and a directory `Config/` would conflict, even though this | ||
would be a valid structure on Unix-based filesystems. | ||
|
||
We are choosing to avoid the strict association with filesystem paths in order | ||
to ease refactoring. With this approach, | ||
[more refactorings](#potential-refactorings) will not need changes to imports of | ||
callers. | ||
Disadvantages: | ||
|
||
- A strict association between filesystem path and import path makes it easier | ||
to find source files. This is used by some languages for compilation. | ||
- Allows getting rid of the `package` keyword by inferring related information | ||
from the filesystem path. | ||
|
||
We are choosing to have some association between the filesystem path and library | ||
for API files to make it easier to find a library's files. We are not getting | ||
rid of the `package` keyword because we don't want to become dependent on | ||
filesystem structures, particularly as it would increase the complexity of | ||
distributed builds. | ||
|
||
### Libraries | ||
|
||
|
@@ -1397,7 +1405,7 @@ approaches looks like: | |
- Alternative: `import "Boost/Random";` | ||
- Multi-layer library: | ||
- Proposal: `import BoostRandom library "Uniform";` | ||
- Alternative: `import "Boost/Random.Uniform";` | ||
- Alternative: `import "Boost/Random.Uniform";` | ||
- Namespaces have no effect on `import` under both approaches. | ||
- Changes to use an imported entity: | ||
- Proposal: `BoostRandom.UniformDistribution` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had thought we would want to use the exact API file's path? That is, include the
.carbon
? But maybe not.(I don't feel strongly about this either way, and don't think this is super important, mostly want us to be intentional in the choice)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should omit it -- however, that can always change in another, more targeted proposal. I do admit an assumption that we wouldn't want to repeat file extensions, if possible. This is particularly notable if we expect all impl files to be prefixed with the library name, e.g. "Math/Algebra.impl.carbon", "Math/Algebra.impl.MyDetail.carbon", at which point adding the ".carbon" to the library will end up obfuscating the path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree it should be omitted, as it is just boilerplate noise. Including it will make it harder to support filesystems that use
.
as a directory separator.