From 45dd45561e02ae2708c4db73a92d28915849ca3e Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 7 Jan 2020 17:32:39 -0800 Subject: [PATCH] go/parser: better error recovery for (invalid) local contracts Change-Id: I76b0d2af3767978ca138d899a083a6e399fea0fe --- src/go/parser/parser.go | 25 +++++++++++++++++++++---- src/go/parser/short_test.go | 1 + src/go/parser/testdata/generic.go2 | 12 ++++++++++++ src/go/types/NOTES | 1 - src/go/types/examples/contracts.go2 | 8 ++++++++ src/go/types/testdata/tmp.go2 | 8 ++++++++ 6 files changed, 50 insertions(+), 5 deletions(-) create mode 100644 src/go/parser/testdata/generic.go2 diff --git a/src/go/parser/parser.go b/src/go/parser/parser.go index edec5a7d5b..e88bb0f8f3 100644 --- a/src/go/parser/parser.go +++ b/src/go/parser/parser.go @@ -1983,6 +1983,19 @@ func (p *parser) parseSimpleStmt(mode int) (ast.Stmt, bool) { } switch p.tok { + case token.IDENT: + // possibly a local contract declaration - accept but complain + // Note: This still doesn't catch local grouped contract declarations + // since they look like a function call at first. But those will + // be exceedingly rare, and syntax errors will lead a writer to + // trying ungrouped contracts which will be caught here. + if ident, isIdent := x[0].(*ast.Ident); isIdent && ident.Name == "contract" { + pos := ident.Pos() + c := p.parseContractSpec(nil, pos, token.ILLEGAL, 0) + p.error(pos, "contract declaration cannot be inside function") + return &ast.BadStmt{From: pos, To: c.End()}, false + } + case token.COLON: // labeled statement colon := p.pos @@ -2706,14 +2719,12 @@ func (p *parser) parseTypeSpec(doc *ast.CommentGroup, _ token.Pos, _ token.Token return spec } -// ContractType = ident "(" [ IdentList [ "," ] ] ")" "{" { Constraint ";" } "}" . -func (p *parser) parseContractSpec(doc *ast.CommentGroup, pos token.Pos, _ token.Token, _ int) ast.Spec { +// ContractSpec = ident "(" [ IdentList [ "," ] ] ")" "{" { Constraint ";" } "}" . +func (p *parser) parseContractSpec(doc *ast.CommentGroup, pos token.Pos, keyword token.Token, _ int) ast.Spec { if p.trace { defer un(trace(p, "ContractSpec")) } - // For now we represent a contract specification like a type representation. - // They cannot have "outer" type parameters, though. ident := p.parseIdent() var tparams []*ast.Ident @@ -2738,6 +2749,12 @@ func (p *parser) parseContractSpec(doc *ast.CommentGroup, pos token.Pos, _ token rbrace := p.expect(token.RBRACE) spec := &ast.ContractSpec{Doc: doc, Name: ident, TParams: tparams, Lbrace: lbrace, Constraints: constraints, Rbrace: rbrace} + if keyword == token.ILLEGAL { + // parseContract was called from parseSimpleStmt; it is invalid and + // a semicolon will be expected externally => don't parse semicolon + // and ignore the comment. + return spec + } p.declare(spec, nil, p.topScope, ast.Typ, ident) // TODO(gri) should really be something other that ast.Typ p.expectSemi() // call before accessing p.linecomment spec.Comment = p.lineComment diff --git a/src/go/parser/short_test.go b/src/go/parser/short_test.go index 2fcba3d429..7b249300f1 100644 --- a/src/go/parser/short_test.go +++ b/src/go/parser/short_test.go @@ -173,6 +173,7 @@ var invalids = []string{ // contracts `package p; contract C(T, T /* ERROR "T redeclared" */ ) {}`, `package p; contract C(T) { imported /* ERROR "expected type parameter name" */ .T int }`, + `package p; func _() { contract /* ERROR "cannot be inside function" */ C(T) { T m(); type int, float32 } }`, // issue 8656 `package p; func f() (a b string /* ERROR "missing ','" */ , ok bool)`, diff --git a/src/go/parser/testdata/generic.go2 b/src/go/parser/testdata/generic.go2 new file mode 100644 index 0000000000..e036de95f3 --- /dev/null +++ b/src/go/parser/testdata/generic.go2 @@ -0,0 +1,12 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package p + +// Verify clean recovery after invalid local contract declarations. +func _() { + contract /* ERROR "inside function" */ C() {} + contract /* ERROR "inside function" */ C(T) {} + contract /* ERROR "inside function" */ C(T) { C(T); T m(); T int } +} diff --git a/src/go/types/NOTES b/src/go/types/NOTES index 2b54fcc658..2d4f163f3a 100644 --- a/src/go/types/NOTES +++ b/src/go/types/NOTES @@ -7,7 +7,6 @@ TODO - type assertions on/against parameterized types - no need for error messages where a _ type parameter cannot be inferred (_ cannot be used) - use Underlying() to return a type parameter's bound? investigate! -- better error message when declaring a contract local to a function (parser gets out of sync) - if type parameters are repeated in recursive instantiation, they must be the same order (not yet checked) - debug (and error msg) printing of generic instantiated types needs some work - improve error messages! diff --git a/src/go/types/examples/contracts.go2 b/src/go/types/examples/contracts.go2 index d48d441e40..0ea35aa61b 100644 --- a/src/go/types/examples/contracts.go2 +++ b/src/go/types/examples/contracts.go2 @@ -34,6 +34,14 @@ contract Sequence(T) { T string, []byte } +// Contracts may not be declared locally. They are accepted +// for good syntax error recovery, but an errors are reported. +func _() { + contract /* ERROR "inside function" */ C() {} + contract /* ERROR "inside function" */ C(T) {} + contract /* ERROR "inside function" */ C(T) { C(T); T m(); T int } +} + // Contracts may constrain multiple type parameters // in mutually recursive ways. contract G(Node, Edge) { diff --git a/src/go/types/testdata/tmp.go2 b/src/go/types/testdata/tmp.go2 index 3dc51460b5..4dc15e2279 100644 --- a/src/go/types/testdata/tmp.go2 +++ b/src/go/types/testdata/tmp.go2 @@ -4,6 +4,7 @@ package p +/* type I interface{ m(type A)(A) } @@ -24,3 +25,10 @@ type II interface{ } var _ I = II(nil) +*/ + +func _() { + contract /* ERROR "inside function" */ C() {} + contract /* ERROR "inside function" */ C(T) {} + contract /* ERROR "inside function" */ C(T) { C(T); T m(); T int } +} \ No newline at end of file