diff options
| -rw-r--r-- | cmd/skylark/skylark.go | 39 | ||||
| -rw-r--r-- | doc/impl.md | 4 | ||||
| -rw-r--r-- | doc/spec.md | 27 | ||||
| -rw-r--r-- | resolve/testdata/resolve.sky | 19 | ||||
| -rw-r--r-- | syntax/grammar.txt | 6 | ||||
| -rw-r--r-- | syntax/parse.go | 97 | ||||
| -rw-r--r-- | syntax/parse_test.go | 2 | ||||
| -rw-r--r-- | syntax/scan.go | 3 | ||||
| -rw-r--r-- | syntax/testdata/errors.sky | 27 | ||||
| -rw-r--r-- | testdata/misc.sky | 10 |
10 files changed, 111 insertions, 123 deletions
diff --git a/cmd/skylark/skylark.go b/cmd/skylark/skylark.go index 8eaff8f..8fac681 100644 --- a/cmd/skylark/skylark.go +++ b/cmd/skylark/skylark.go @@ -127,14 +127,9 @@ outer: continue // blank or comment } - // If the line contains a well-formed - // expression, evaluate it. - if expr, err := syntax.ParseExpr("<stdin>", line); err == nil { - if isLoad(expr) { - if err := execFileNoFreeze(thread, line, globals); err != nil { - printError(err) - } - } else if v, err := skylark.Eval(thread, "<stdin>", line, globals); err != nil { + // If the line contains a well-formed expression, evaluate it. + if _, err := syntax.ParseExpr("<stdin>", line); err == nil { + if v, err := skylark.Eval(thread, "<stdin>", line, globals); err != nil { printError(err) } else if v != skylark.None { fmt.Println(v) @@ -142,6 +137,19 @@ outer: continue } + // If the input so far is a single load or assignment statement, + // execute it without waiting for a blank line. + if f, err := syntax.Parse("<stdin>", line); err == nil && len(f.Stmts) == 1 { + switch f.Stmts[0].(type) { + case *syntax.AssignStmt, *syntax.LoadStmt: + // Execute it as a file. + if err := execFileNoFreeze(thread, line, globals); err != nil { + printError(err) + } + continue + } + } + // Otherwise assume it is the first of several // comprising a file, followed by a blank line. var buf bytes.Buffer @@ -165,7 +173,7 @@ outer: // 1, // 2 // ) - if expr, err := syntax.ParseExpr("<stdin>", text); err == nil && !isLoad(expr) { + if _, err := syntax.ParseExpr("<stdin>", text); err == nil { if v, err := skylark.Eval(thread, "<stdin>", text, globals); err != nil { printError(err) } else if v != skylark.None { @@ -240,16 +248,3 @@ func printError(err error) { fmt.Fprintln(os.Stderr, err) } } - -// isLoad reports whether e is a load(...) function call. -// If so, we must parse it again as a file, not an expression, -// so that it is is converted to a load statement. -// ("load" should really be a reserved word.) -func isLoad(e syntax.Expr) bool { - if call, ok := e.(*syntax.CallExpr); ok { - if id, ok := call.Fn.(*syntax.Ident); ok && id.Name == "load" { - return true - } - } - return false -} diff --git a/doc/impl.md b/doc/impl.md index b896ef9..6ac20e0 100644 --- a/doc/impl.md +++ b/doc/impl.md @@ -28,10 +28,6 @@ technique of [precedence climbing](http://www.engr.mun.ca/~theo/Misc/exp_parsing.htm#climbing) to reduce the number of productions. -Because `load` is not a reserved word, Skylark's `load` statements are -created by post-processing `load(...)` function calls that appear in -an expression statement. - In some places the parser accepts a larger set of programs than are strictly valid, leaving the task of rejecting them to the subsequent resolver pass. For example, in the function call `f(a, b=c)` the diff --git a/doc/spec.md b/doc/spec.md index 91e8c40..2c8035d 100644 --- a/doc/spec.md +++ b/doc/spec.md @@ -247,13 +247,11 @@ characters are tokens: identifiers: ```text -and if -break in -continue lambda -def not -elif or -else pass -for return +and else load +break for not +continue if or +def in pass +elif lambda return ``` The tokens below also may not be used as identifiers although they do not @@ -2314,6 +2312,7 @@ SmallStmt = ReturnStmt | BreakStmt | ContinueStmt | PassStmt | AssignStmt | ExprStmt + | LoadStmt . ``` @@ -2643,12 +2642,6 @@ loop. The `load` statement loads another Skylark module, extracts one or more values from it, and binds them to names in the current module. -Syntactically, a load statement looks like a function call `load(...)`. -However, `load` is not a keyword, so the parser converts any expression -statement containing a function call of the form `load(...)` into a -load statement. -In all other contexts, `load` acts like an ordinary identifier. - <!-- The awkwardness of load statements is a consequence of staying a strict subset of Python syntax, which allows reuse of existing tools @@ -2656,7 +2649,11 @@ such as editor support. Python import statements are inadequate for Skylark because they don't allow arbitrary file names for module names. --> -A load statement within a function is a static error. +Syntactically, a load statement looks like a function call `load(...)`. + +```grammar {.good} +LoadStmt = 'load' '(' string {',' [identifier '='] string} [','] ')' . +``` A load statement requires at least two "arguments". The first must be a literal string; it identifies the module to load. @@ -2683,6 +2680,8 @@ load("module.sky", "x", "y", "z") # assigns x, y, and z load("module.sky", "x", y2="y", "z") # assigns x, y2, and z ``` +A load statement within a function is a static error. + ## Module execution diff --git a/resolve/testdata/resolve.sky b/resolve/testdata/resolve.sky index c48af1c..f8c4c9a 100644 --- a/resolve/testdata/resolve.sky +++ b/resolve/testdata/resolve.sky @@ -120,6 +120,7 @@ def f(): f() --- +load("module", "name") # ok def f(): load("foo", "bar") ### "load statement within a function" @@ -132,24 +133,6 @@ load("foo", _e="f") # ok --- -# A load() call as an expression statement is converted into a -# load statement, but load is not currently a reserved word. -# TODO(adonovan): clarify the Skylark spec on this issue. - -def load(): # doesn't affect following call - pass - -_ = 1 + load() # ok - -load("foo.sky", "") ### "load: empty identifier" - ---- - -def f(load): - _ = (load, load()) # ok - load("foo.sky", "x") ### "load statement within a function" - ---- # return, if statements and for loops at top-level are forbidden for x in "abc": ### "for loop not within a function" diff --git a/syntax/grammar.txt b/syntax/grammar.txt index 4e6f788..d2ab419 100644 --- a/syntax/grammar.txt +++ b/syntax/grammar.txt @@ -25,6 +25,7 @@ SmallStmt = ReturnStmt | BreakStmt | ContinueStmt | PassStmt | AssignStmt | ExprStmt + | LoadStmt . ReturnStmt = 'return' Expression . @@ -34,6 +35,8 @@ PassStmt = 'pass' . AssignStmt = Expression ('=' | '+=' | '-=' | '*=' | '/=' | '//=' | '%=') Expression . ExprStmt = Expression . +LoadStmt = 'load' '(' string {',' [identifier '='] string} [','] ')' . + Test = LambdaExpr | IfExpr | PrimaryExpr @@ -111,9 +114,6 @@ LoopVariables = PrimaryExpr {',' PrimaryExpr} . - plus all quoted tokens such as '+=', 'return'. # Notes: -- 'load' is an identifier, not a reserved word. - Load statements are synthesized when reducing 'small_stmt = expr' - if the name of the identifier is 'load'. This is a wart. - Ambiguity is resolved using operator precedence. - The grammar does not enforce the legal order of params and args, nor that the first compclause must be a 'for'. diff --git a/syntax/parse.go b/syntax/parse.go index c824738..e3efec3 100644 --- a/syntax/parse.go +++ b/syntax/parse.go @@ -217,23 +217,26 @@ func (p *parser) parseSimpleStmt(stmts []Stmt) []Stmt { // small_stmt = RETURN expr? // | PASS | BREAK | CONTINUE +// | LOAD ... // | expr ('=' | '+=' | '-=' | '*=' | '/=' | '%=') expr // assign // | expr func (p *parser) parseSmallStmt() Stmt { - if p.tok == RETURN { + switch p.tok { + case RETURN: pos := p.nextToken() // consume RETURN var result Expr if p.tok != EOF && p.tok != NEWLINE && p.tok != SEMI { result = p.parseExpr(false) } return &ReturnStmt{Return: pos, Result: result} - } - switch p.tok { case BREAK, CONTINUE, PASS: tok := p.tok pos := p.nextToken() // consume it return &BranchStmt{Token: tok, TokenPos: pos} + + case LOAD: + return p.parseLoadStmt() } // Assignment @@ -246,72 +249,74 @@ func (p *parser) parseSmallStmt() Stmt { return &AssignStmt{OpPos: pos, Op: op, LHS: x, RHS: rhs} } - // Convert load(...) call into LoadStmt special form. - // TODO(adonovan): This affects all calls to load, not just at toplevel. - // Spec clarification needed. - if call, ok := x.(*CallExpr); ok { - if id, ok := call.Fn.(*Ident); ok && id.Name == "load" { - return p.convertCallToLoad(call, id.NamePos) - } - } - // Expression statement (e.g. function call, doc string). return &ExprStmt{X: x} } -// Because load is not a reserved word, it is impossible to parse a load -// statement using an LL(1) grammar. Instead we parse load(...) as a function -// call and then convert it to a LoadStmt. -func (p *parser) convertCallToLoad(call *CallExpr, loadPos Position) *LoadStmt { - if len(call.Args) < 2 { - p.in.errorf(call.Lparen, "load statement needs at least 2 operands, got %d", len(call.Args)) - } - module, ok := call.Args[0].(*Literal) - if !ok || module.Token != STRING { - start, _ := call.Args[0].Span() - p.in.errorf(start, "first operand of load statement must be a string literal") +// stmt = LOAD '(' STRING {',' (IDENT '=')? STRING} [','] ')' +func (p *parser) parseLoadStmt() *LoadStmt { + loadPos := p.nextToken() // consume LOAD + lparen := p.consume(LPAREN) + + if p.tok != STRING { + p.in.errorf(p.in.pos, "first operand of load statement must be a string literal") } + module := p.parsePrimary().(*Literal) - // synthesize identifiers - args := call.Args[1:] - to := make([]*Ident, len(args)) - from := make([]*Ident, len(args)) - for i, arg := range args { - if lit, ok := arg.(*Literal); ok && lit.Token == STRING { + var from, to []*Ident + for p.tok != RPAREN && p.tok != EOF { + p.consume(COMMA) + if p.tok == RPAREN { + break // allow trailing comma + } + switch p.tok { + case STRING: // load("module", "id") // To name is same as original. + lit := p.parsePrimary().(*Literal) id := &Ident{ NamePos: lit.TokenPos.add(`"`), Name: lit.Value.(string), } - to[i] = id - from[i] = id - continue - } else if binary, ok := arg.(*BinaryExpr); ok && binary.Op == EQ { + to = append(to, id) + from = append(from, id) + + case IDENT: // load("module", to="from") - // Symbol is locally renamed. - if lit, ok := binary.Y.(*Literal); ok && lit.Token == STRING { - id := &Ident{ - NamePos: lit.TokenPos.add(`"`), - Name: lit.Value.(string), - } - to[i] = binary.X.(*Ident) - from[i] = id - continue + id := p.parseIdent() + to = append(to, id) + if p.tok != EQ { + p.in.errorf(p.in.pos, `load operand must be "%s" or %s="originalname" (want '=' after %s)`, id.Name, id.Name) + } + p.consume(EQ) + if p.tok != STRING { + p.in.errorf(p.in.pos, `original name of loaded symbol must be quoted: %s="originalname"`, id.Name) } + lit := p.parsePrimary().(*Literal) + from = append(from, &Ident{ + NamePos: lit.TokenPos.add(`"`), + Name: lit.Value.(string), + }) + + case RPAREN: + p.in.errorf(p.in.pos, "trailing comma in load statement") + + default: + p.in.errorf(p.in.pos, `load operand must be "name" or localname="name" (got %#v)`, p.tok) } + } + rparen := p.consume(RPAREN) - start, _ := arg.Span() - p.in.errorf(start, `load operand must be "name" or localname="name"`) + if len(to) == 0 { + p.in.errorf(lparen, "load statement must import at least 1 symbol") } return &LoadStmt{ Load: loadPos, Module: module, To: to, From: from, - Rparen: call.Rparen, + Rparen: rparen, } - } // suite is typically what follows a COLON (e.g. after DEF or FOR). diff --git a/syntax/parse_test.go b/syntax/parse_test.go index dc9e4a9..6657df6 100644 --- a/syntax/parse_test.go +++ b/syntax/parse_test.go @@ -154,8 +154,6 @@ else: `(AssignStmt Op== LHS=(TupleExpr List=(x y)) RHS=1)`}, {`load("", "a", b="c")`, `(LoadStmt Module="" From=(a c) To=(a b))`}, - {`load = 1`, // load is not a reserved word - `(AssignStmt Op== LHS=load RHS=1)`}, {`if True: load("", "a", b="c")`, // load needn't be at toplevel `(IfStmt Cond=True True=((LoadStmt Module="" From=(a c) To=(a b))))`}, {`def f(x, *args, **kwargs): diff --git a/syntax/scan.go b/syntax/scan.go index 7e8cb9a..6b8aaf8 100644 --- a/syntax/scan.go +++ b/syntax/scan.go @@ -79,6 +79,7 @@ const ( IF IN LAMBDA + LOAD NOT NOT_IN // synthesized by parser from NOT IN OR @@ -151,6 +152,7 @@ var tokenNames = [...]string{ IF: "if", IN: "in", LAMBDA: "lambda", + LOAD: "load", NOT: "not", NOT_IN: "not in", OR: "or", @@ -887,6 +889,7 @@ var keywordToken = map[string]Token{ "if": IF, "in": IN, "lambda": LAMBDA, + "load": LOAD, "not": NOT, "or": OR, "pass": PASS, diff --git a/syntax/testdata/errors.sky b/syntax/testdata/errors.sky index 71fb4da..39ef781 100644 --- a/syntax/testdata/errors.sky +++ b/syntax/testdata/errors.sky @@ -120,21 +120,40 @@ print(1, 2, 3 ### `got end of file, want '\)'` --- _ = a if b ### "conditional expression without else clause" --- -load("") ### "load statement needs at least 2 operands, got 1" +load("") ### "load statement must import at least 1 symbol" --- -load("", 1) ### `load operand must be "name" or localname="name"` +load("", 1) ### `load operand must be "name" or localname="name" \(got int literal\)` +--- +load("a", "x") # ok --- load(1, 2) ### "first operand of load statement must be a string literal" --- -load("a", x) ### `load operand must be "name" or localname="name"` +load("a", x) ### `load operand must be "x" or x="originalname"` --- -load("a", x2=x) ### `load operand must be "name" or localname="name"` +load("a", x2=x) ### `original name of loaded symbol must be quoted: x2="originalname"` --- # All of these parse. load("a", "x") load("a", "x", y2="y") load("a", x2="x", "y") # => positional-before-named arg check happens later (!) --- +# 'load' is not an identifier +load = 1 ### `got '=', want '\('` +--- +# 'load' is not an identifier +f(load()) ### `got load, want primary` +--- +# 'load' is not an identifier +def load(): ### `not an identifier` + pass +--- +# 'load' is not an identifier +def f(load): ### `not an identifier` + pass +--- +# A load statement allows a trailing comma. +load("module", "x",) +--- x = 1 + 2 ### "got newline, want primary expression" --- diff --git a/testdata/misc.sky b/testdata/misc.sky index 2388ecd..5537a6e 100644 --- a/testdata/misc.sky +++ b/testdata/misc.sky @@ -97,13 +97,3 @@ assert.eq(1 + \ # A regression test for error position information. _ = {}.get(1, default=2) ### "get: unexpected keyword arguments" - ---- -load("assert.sky", "assert") - -# load(...) calls outside an expression statement are not load statements. - -assert.eq([load("123") for load in [int, len]], [123, 3]) - -load = lambda x: x -assert.eq(load("abc"), "abc") |