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

spec.md minor self-contradiction on scope of load statements #227

Open
eqvinox opened this issue Jun 29, 2022 · 3 comments
Open

spec.md minor self-contradiction on scope of load statements #227

eqvinox opened this issue Jun 29, 2022 · 3 comments

Comments

@eqvinox
Copy link

eqvinox commented Jun 29, 2022

From current spec.md:

starlark/spec.md

Lines 1344 to 1359 in 3d97ad7

Nested beneath the predeclared block is the _module_ block,
which contains the bindings of the current module.
Bindings in the module block (such as `a`, `b`, `c`, and `h` in the
example) are called _global_ and may be visible to other modules.
The module block is empty at the start of the file
and is populated by top-level binding statements,
but an application may pre-bind one or more global names,
to provide domain-specific functions to that file, for example.
Nested beneath the module block is the _file_ block,
which contains bindings local to the current file.
Names in this block (such as `a` and `b` in the example)
are bound only by `load` statements.
The sets of names bound in the file block and in the module block do not overlap:
it is an error for a load statement to bind the name of a global,
or for a top-level statement to bind a name bound by a load statement.

a and b are listed as both module-scoped and file-scoped, which isn't exactly possible. I'm assuming this is an oversight with the bazelbuild/bazel#5636 change and a and b should simply be removed from the module scope list? (I'm not fully aware of the history here but it seems like load-imported names have been changed to not be re-exported…)

@adonovan
Copy link
Contributor

The correct wording can be found at https://github.com/google/starlark-go/blob/master/doc/spec.md#name-binding-and-variables, which was updated in google/starlark-go#178. A similar update needs to be applied to bazelbuild.

@eqvinox
Copy link
Author

eqvinox commented Jun 29, 2022

The correct wording can be found at google/starlark-go@master/doc/spec.md#name-binding-and-variables, which was updated in google/starlark-go#178. A similar update needs to be applied to bazelbuild.

Easy enough, thanks for the quick confirm 👍 — was about to open a quick PR but then noticed the CLA requirement, so, uh, apologies, not gonna do that as I don't really expect to become a contributor. 😞

(I'm also slightly alarmed by the existence of multiple slightly-different variations of what seems to be 99% the same spec? Is the one in starlark-go more up to date?)

@adonovan
Copy link
Contributor

(I'm also slightly alarmed by the existence of multiple slightly-different variations of what seems to be 99% the same spec? Is the one in starlark-go more up to date?)

The Go implementation and its docs came out of an effort to build a reference implementation and derive a sensible language spec. It identified a number of places where the Java implementation (used by Blaze) did not match the ideal semantics; many of those have since been fixed. The bazelbuild copy of the document was copied from the Go one, and the portions of the spec that the Java implementation did not implement were deleted without further ado. In an ideal world they would have been flagged as missing features and the two documents converged. The Go implementation and its document are closer to what I think we all agree are the ideal semantics; the Java implementation still lacks in terms of string encodings and support for byte strings. The team has been too short-staffed to spend more time on implementation and documentation convergence recently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants