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: allow nested def statements and lambda expressions #145

Merged
merged 1 commit into from
Dec 11, 2020

Conversation

alandonovan
Copy link
Contributor

No description provided.

adonovan added a commit to google/starlark-go that referenced this pull request Dec 4, 2020
See bazelbuild/starlark#145
for spec changes.

Updates bazelbuild/starlark#20

Change-Id: I31e6258cc6caef6bcd3eab57ccec04f1b858b7e7
adonovan added a commit to google/starlark-go that referenced this pull request Dec 4, 2020
See bazelbuild/starlark#145
for spec changes.

Updates bazelbuild/starlark#20

Change-Id: I31e6258cc6caef6bcd3eab57ccec04f1b858b7e7
Comment on lines +816 to +939
Function definitions may be nested, and an inner function may refer
to a local variable of an outer function.
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably worth clarifying that:

  • it is not possible to assign to a captured variable (unlike Python with nonlocal keyword)
  • captured values can be mutated until captured values are frozen (similar to functions capturing globals)

I have mixed feelings about this proposal.

I have never seen any good code with nested def statements.

Lambdas are somewhat more useful, but anonymous functions in 99% of cases are used in parameters to map and filter functions, which is not the issue with Python/Starlark list comprehensions expressions.

But I heard some people want both nested defs and lambdas.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably worth clarifying that ...

Done, x2

I have mixed feelings about this proposal.

My two concerns are that some people will try to write Scheme or Haskell in Starlark, and that the memory footprint of a struct containing only closures (to simulate an opaque object) is greater than necessary. (It's approximately 15 words per closure in the Java implementation.) The first problem may be ameliorated by the lack of recursion, and the second can be addressed by optimizations (e.g. combining defaults and freevars Tuples into a single array; grouping all the per-module stuff behind a single reference).

I have never seen any good code with nested def statements.

I don't know what code you've seen, but I've seen tons of good and bad code with nested def statements, and in the absence of classes (the other way to combine code+data), they provide a much needed form of expressiveness. They also help to avoid polluting the toplevel namespace with helper functions that could be local.

@@ -1530,10 +1534,11 @@ for x in 1, 2:
```

Starlark (like Python 3) does not accept an unparenthesized tuple
expression as the operand of a list comprehension:
or lambda expression as the operand of a `for`-clause in a comprehension:
Copy link
Member

Choose a reason for hiding this comment

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

Python 3 does not accept an unparenthesized tuple in either a for or if clause, and it does not accept an unparenthesized lambda in a for clause.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding tuples, Python has never allowed an Expression (unparenthesized tuple) as the operand of an 'if' clause, nor would anyone want such a thing, so we needn't mention it. However, Python2 used to allow an Expression as the operand of 'for', and it's a common and natural thing to want, hence we point out that we follow Python3 in using a Test (tuples must be parenthesized).

By itself, using Test would permit lambda to be the operand of a for clause, but Python3 specifically rejects lambda there, I assume to avoid a parsing ambiguity that would arising from a lambda body that is a conditional expression.

I think the wording is correct.

The spec does need to be more explicit about precedence rules around if/else expressions, but I don't think there's anything relevant to say about it here.

A `lambda` expression yields a new function value.

```grammar {.good}
LambdaExpr = 'lambda' [Parameters] ':' Test .
Copy link
Member

Choose a reason for hiding this comment

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

I'm in a rush so not checking the full grammar atm, but aren't there two cases of the concrete grammar production for lambda, that differ in the expressiveness of the body?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but it's really to enforce the correct precedence of if/else expressions. I've added a note in Conditional Expressions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

During parsing, the if operator, considered as a postfix operator on
the "true" expression, has higher precedence than else (a prefix
operator on the "false" expression), which in turn has higher
precedence than the lambda prefix operator.

a if b else (c if d else e)          # parens are redundant                                          
(a if b else c) if d else e          # parens are required                                           
                                                                                                     
lambda: (a if b else c)              # parens are redunant                                           
(lambda: a) if b else c              # parens are required                                           
                                                                                                     
a if b else lambda: (c if d else e)  # parens are redundant                                          
a if b else (lambda: c if d else e)  # parens are required                                           
(a if b else lambda: c) if d else e  # parens are required                                           

spec.md Outdated

As with functions created by a `def` statement, a lambda function
captures the syntax of its body, the default values of any optional
parameters, the value of each free variable appearing in its body, and
Copy link
Member

Choose a reason for hiding this comment

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

value of each free variable

"Value" is wrong, it's the cell (or whatever the user-facing terminology for this concept would be).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to "reference to each free variable appearing in the body". Added an example.

parameters, the value of each free variable appearing in its body, and
the global dictionary of the current module.

The name of a function created by a lambda expression is `"lambda"`.
Copy link
Member

Choose a reason for hiding this comment

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

name

Despite it being "anonymous". Probably fine to leave as is though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's fine.

Change-Id: I4c515683709350f87fc9195ab2e12870e7e8df5e
@alandonovan alandonovan merged commit 98a71a3 into bazelbuild:master Dec 11, 2020
alandonovan pushed a commit to google/starlark-go that referenced this pull request Jan 21, 2021
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

Successfully merging this pull request may close these issues.

None yet

4 participants