Signed-off-by: b5 [email protected]

A few notes:

• I've added this to lib/time based on @alandonovan's feedback from #326
• we're waiting on a chat with @srebhan about the naming of the constructors vs. parsing functions before this is final
• We use a tool to extract documentation from this package in doc.go into other formats. I can remove this to conform to other packages, but if this info can stay & be maintained, I'd be deeply appreciated. Keeping documentation to the code itself has made our lives much easier.
• If we merge this I'll take this back to our contributors on the starlib project & chat about relying on this package upstream. @alandonovan in the past you'd mentioned interest in other packages from the project, now might be the time to go shopping 😉

@essobedo is going to be heading up turning this code into a package that closes #241. We're using the re package from starlib as a starting point, but it needs a lot of work before it's ready for initial review.

@essobedo I think at a minimum we should:

• [x] switch the defined here to match the API defined in #241
• [x] remove regexp/doc.go (I've kept it only so you have context)
• [x] document regexp/regexp.go
• [x] completely re-write the test suite to test the new API
• [x] add a test that confirms failure when attempting to compile a regular expression that that uses byte-ordered features: \C

One of the file formats in the application that I am developing uses Starlark for writing expressions. The Starlark expressions are surrounded by source code written in another language.

I want to specify a starting line number and column number when parsing expressions so that errors point back to the original source code.

Here's a suggested function for the syntax package.

// ParseExpr parses a Starlark expression in src. A comma-separated
// list of expressions is parsed as a tuple. Positions in the resulting 
// expression are based on pos.
func ParseExprPosition(pos Position, src []byte, mode Mode) (expr Expr, err error)

The proposed function does not have the usual filename, src pattern as in other functions in the syntax package because it's designed to work on a snippet from a larger file.

My feature request is for a way to specify the starting line and column number for the scanner, not for the specific function shown above.

My fallback plan is to rewrite positions in syntax.Error and starlark.EvalError using a source map that I maintain on the side. I can do that, but it seems that other users of Starlark expressions will have a similar requirement.

The repl program uses a "context" local variable suggesting that long-running operations should check this context and terminate early when the context expires. This solution will probably take care of 99% of use cases but won't stop the interpreter if the long-running operation is implemented in starlark itself.

I propose that a Context field is added to starlark.Thread and that the interpreter loop checks it every N-th instruction and returns an appropriate error when starlark.Thread.Context expires.

Starlark seem mighty terrific. I just have a minor query about doing some math.

In starlark-go, how would one raise n to the m-th power?

In Go, I would call math.Pow(n, m). In Python, math.pow(n, m).

Another way of asking: is there a way to get at the math library from Go perhaps?

This PR adds time and duration support to starlark and solves issue #19. The API is inspired by the discussion in above issue as well as the linked first implementation for skylark. However, we diverged on some points from the proposal (as well as from the python datetime API) to provide a more clean and consistent interface.

This change causes the AllowGlobalReassign flag (used for legacy Bazel semantics) to also allow use of predeclared names prior to a global binding of the same name, as in:

print(len); len=1; print(len)

This effectively reverses github.com/google/skylark/pull/123 behind an option.

See github.com/google/skylark/issues/116 for discussion.

I am trying to use a piece of software that relies on starlark-go on a system that rigourously enforces ulimits (in particular on virtual memory). Which means that I have to patch out the the line 62 in int_posix64.go, which tries to allocate 4Gb of memory, which the machine does not possess.

var smallints = reserveAddresses(1 << 32)

func reserveAddresses(len int) uintptr {
	b, err := unix.Mmap(-1, 0, len, unix.PROT_READ, unix.MAP_PRIVATE|unix.MAP_ANON)
	if err != nil {
		log.Fatalf("mmap: %v", err)
	}
	return uintptr(unsafe.Pointer(&b[0]))
}

Please, consider implementing proper memory management, instead of "just allocating 4Gb of RAM".

$ starlark -c "dict(S=5, S=5)"
$ python -c "dict(S=5, S=5)"
  File "<string>", line 1
SyntaxError: keyword argument repeated

Found by go-fuzz.

root cause:

https://github.com/google/starlark-go/blob/50ca820fafb940caeda09b96864b9215f8b84c04/starlark/int_posix64.go#L55-L57

when targeting on darwin/arm64, it case mmap: cannot allocate memory

In my use case, I receive a function from the user. And I passthrough params and execute it. But I need a way to stop the function running with a timeout (e.g. 2000ms).

can starlark-go provide a function like CallContext?

thread := &starlark.Thread{
    Name:  "example",
}

globals, _ := starlark.ExecFile(thread, "test.star", f, nil)

mainFn := globals["main"]
dict := starlark.StringDict{
    "param": starlark.String("test"),
}

m := starlarkstruct.FromStringDict(starlarkstruct.Default, dict)
arg := starlark.Tuple{m}

ctx, cancel := context.WithTimeout(context.background(), time.Second)
defer cancel()
starlark.CallContext(ctx, thread, mainFn, arg, nil)

*syntax.DictExpr has a field List []Expr which is supposed to only contain *syntax.DictEntrys:

https://github.com/google/starlark-go/blob/d7da88764354917a82dfa84a8ae1cb04f107ab78/syntax/syntax.go#L377

However, when we are walking *syntax.DictExpr node in syntax.Walk function, we are ignoring *syntax.DictEntrys, directly unwrapping it, and then continuing the walk recursions. This means the caller will not be able to "see" a *syntax.DictEntry node when calling the walk function.

This PR fixes this by directly walking on each element of the List field, which in turn will be automatically handled by the *syntax.DictEntry case in Walk.

For *List, *Dict, and *Set, it'd be nice to have convenience methods to shallow-copy them, so an application doesn't have to use an explicit loop or call the Slice method.

This one is pretty simple. I found myself starting to write a bunch of code to encode a Starlark value as JSON, when I realized that oh hey, there's a JSON encoder already in the project. It's just hidden.

I'd like to re-visit the proposal for python-like function type annotations in starlark, at least in the starlark-go implementation. This has been discussed before in:

• https://groups.google.com/g/bazel-dev/c/Pk9VrPCqby0/m/zoOL1IkxEAAJ?pli=1
• https://github.com/bazelbuild/starlark/issues/106
• https://github.com/bazelbuild/buildtools/issues/900

An example of what this looks like in practice:

def func_one(a, b: str, c: list[int]) -> bool:
	pass

This proposal is only for function definition type hints. Other kinds (such as assignment annotations) could be discussed, but I believe function annotations provide the most value with least effort, and can be done without changing any runtime semantics. Function type annotation syntax has already been implemented in buildifier (https://github.com/bazelbuild/buildtools/pull/946) and starlark-rust (https://github.com/facebookexperimental/starlark-rust/blob/main/docs/types.md).

The proposed syntax follows a subset of python type annotations, as well as the same syntax already supported by buildifier and starlark-rust. Type annotations are Test syntax elements, and are parsed as expressions.

-DefStmt = 'def' identifier '(' [Parameters [',']] ')' ':' Suite .
+DefStmt = 'def' identifier '(' [Parameters [',']] ')' ['->' Test] ':' Suite .

 Parameters = Parameter {',' Parameter}.

-Parameter = identifier | identifier '=' Test | '*' | '*' identifier | '**' identifier .
+Parameter = identifier [':' Test] | identifier [':' Test] '=' Test | '*' | '*' identifier [':' Test] | '**' identifier [':' Test] .

Requirements

• Support function definition type annotations in-line with existing starlark implementations and python syntax.
• Does not change the compiled result of a program.
• Maintain backwards compatibility with existing starlark code using the starlark-go interpeter and compiler.
• Maintain backwards compatability with existing code using starlark-go to parse and compile starlark code.

Why Now?

• Other starlark implementations already have function type annotations, with the same syntax, and share the same syntax as python.
• Python type hints (certainly function definition hints) have had quite a bit of time to mature.
• I believe starlark is a great language for defining simple logic and configuration, but lack of tooling and IDE assistance around typing hampers useability, safety, and simplicity. Even in the context of bazel -- I know my experience would be greatly improved with IDE assisted type support and documentation.

Choices

Syntatic Element of a Type Annotation

Test / Expr. This is the same element chosen by starlark-rust and buildifier, and is the closest analogue to python type hints which allows arbitrary expressions.

Ident with TypeHint Field instead of TypedIdent

The buildifier implementation parses a TypedIdent with an ident and type hint:

type TypedIdent struct {
	Comments
	Ident   *Ident
	Type    Expr
}

Whereas the proposed implementation for starlark-go reuses type Ident struct with a TypeHint Expr field:

type Ident struct {
	commentsRef
	NamePos  Position
	Name     string
	TypeHint Expr // TypeHint can be nil

	Binding interface{} // a *resolver.Binding, set by resolver
}

There are two reasons for this:

• Maintain backwards compatibility with programs using the parser: parameters with type hints will still produce the same output type, and Go programs make heavy use of type switches for consuming the tree.
• In writing code that consumes the type hints, I found the TypeHint field approach easier to use, as it requires fewer type switches and idents can be treated the same. Generally types are either extracted or an Any type is returned if TypeHint is nil.

syntax.Walk Support

This PR intentionally excludes syntax.Walk support for type hints, as it would change the behaviour for programs using starlark-go to parse starlark.

On-the-fence Choices

Syntax Feature Flag

Whether or not to lock the syntax behind a flag. Unlike other features behind flags, this does not change the output of programs and does not have backwards compatibility implications.

• This could potentially only be an issue if syntax.Walk is altered to recurse under type hints, as it would change the expected behaviour of programs calling it.
• By keeping this on by default, it increases the likelihood programs will take advantage of type hints as it will at least always be valid in starlark-go.

Resolving Type Annotation Identifiers

The PR adds a ResolveTypeHintIdents flag in the resolve package, which defaults to false. When it is false, type hints cannot cause resolution/compilation failures.

When it is true, type annotation identifiers are resolved and given binding information. If a type annotation refers to an unknown identifier, it could cause resolution to fail. Additionally, if a type hint in a nested function definition referred to an identifier from its parent scopes, it could cause local variables to be promoted to Cell. Because this can slightly change program behaviour, and cause compilation failures it is enabled behind a flag.

I consider this fairly desirable because:

• It follows python's behaviour with respect to resolving type annotation identifiers, and producing errors on resolution failure.
• Tooling that consumes type hints will likely want to make sure type annotations refer to valid identifiers.
• Tooling that consumes type hints might want scope/binding information for type identifiers. By re-using the existing resolve code this is readily available (otherwise the resolve logic would have to be heavily duplicated).

Skip Type Hints for Assignment Syntax

These are supported by starlark-rust, but not buildifier. Python PEP-526 formalizes this for python, however the only applicable syntax for starlark would be variable assignment and definition:

x  # Binding failure
x: int  # Works in python, binding failure in starlark
y: str = "asdf" # Would work in starlark, but we could infer type anyways.

Python changed its behaviour slightly to allow binding unknown variables if given a typehint. This would be a runtime behaviour change in starlark. The currently supported behaviour would require assignment anyways, where we can often infer the type anyways.

While assignment annotations could provide some value, it's a lot less clear cut to me than function definition annotations. I'd like to table discussion of assignment type hints for a different proposal.

Hello there! I have a project where I get this library as a transitive dependency. It's used for "stuff X". I also want to use this library for "stuff Y", unrelated to "stuff X", in the same codebase/binary. I'd like to configure the library and modules differently for different use cases. But in a few places the library uses package-level variables for "global" configuration and I cannot do what I want.

Examples:

• https://github.com/google/starlark-go/blob/acb66ad56dd25d86c91a1efc3869d4a0ec0fcbdb/lib/time/time.go#L74
• https://github.com/google/starlark-go/blob/acb66ad56dd25d86c91a1efc3869d4a0ec0fcbdb/starlark/library.go#L34
• https://github.com/google/starlark-go/blob/acb66ad56dd25d86c91a1efc3869d4a0ec0fcbdb/resolve/resolve.go#L100-L111
• Maybe a few more places I didn't find now.

In addition to the above, global mutable variables are sometimes abused by third-party libraries. They assume they are the center of the world and mutate such variables in other packages in their init(). No mutable globals -> no such problems.

p.s. thank you for the library! It's great!