From caf3fc90ed3fe63c78010a733202d0d1933ab7a1 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 31 Jul 2013 20:08:18 -0700 Subject: [PATCH] go.tools/go/types: test against test/fixedbugs Fixed several bugs: - lhs blank identifier may be parenthesized: (_) = 0 is ok - constant shift counts in non-constant shifts must be >= 0 - init functions must have a body Classified currently failing tests: - 10 classes of errors not checked at the moment R=adonovan, r CC=golang-dev https://golang.org/cl/11952046 --- go/types/assignments.go | 5 ++-- go/types/expr.go | 7 ++++- go/types/resolver.go | 5 ++++ go/types/stdlib_test.go | 58 ++++++++++++++++++++++-------------- go/types/testdata/decls0.src | 1 + go/types/testdata/shifts.src | 25 ++++++++-------- go/types/testdata/stmt0.src | 5 ++++ 7 files changed, 69 insertions(+), 37 deletions(-) diff --git a/go/types/assignments.go b/go/types/assignments.go index ca68ca0a1d2..d79125396dd 100644 --- a/go/types/assignments.go +++ b/go/types/assignments.go @@ -110,12 +110,13 @@ func (check *checker) assignVar(lhs ast.Expr, x *operand) Type { return nil } - // Don't evaluate lhs if it is the blank identifier. - if ident, _ := lhs.(*ast.Ident); ident != nil && ident.Name == "_" { + // Don't evaluate lhs if it is the (possibly parenthesized) blank identifier. + if ident, _ := unparen(lhs).(*ast.Ident); ident != nil && ident.Name == "_" { check.recordObject(ident, nil) // If the lhs is untyped, determine the default type. // The spec is unclear about this, but gc appears to // do this. + // TODO(gri) This is still not correct (try: _ = nil; _ = 1<<1e3) typ := x.typ if isUntyped(typ) { // convert untyped types to default types diff --git a/go/types/expr.go b/go/types/expr.go index 57d0cbeedff..72e79c9e210 100644 --- a/go/types/expr.go +++ b/go/types/expr.go @@ -542,7 +542,7 @@ func (check *checker) shift(x, y *operand, op token.Token) { const stupidShift = 1023 - 1 + 52 // so we can express smallestFloat64 s, ok := exact.Uint64Val(y.val) if !ok || s > stupidShift { - check.invalidOp(y.pos(), "%s: stupid shift", y) + check.invalidOp(y.pos(), "stupid shift count %s", y) x.mode = invalid return } @@ -577,6 +577,11 @@ func (check *checker) shift(x, y *operand, op token.Token) { } } + // constant rhs must be >= 0 + if y.mode == constant && exact.Sign(y.val) < 0 { + check.invalidOp(y.pos(), "shift count %s must not be negative", y) + } + // non-constant shift - lhs must be an integer if !isInteger(x.typ) { check.invalidOp(x.pos(), "shifted operand %s must be integer", x) diff --git a/go/types/resolver.go b/go/types/resolver.go index b4d547f1783..6c4cd089717 100644 --- a/go/types/resolver.go +++ b/go/types/resolver.go @@ -288,6 +288,11 @@ func (check *checker) resolveFiles(files []*ast.File) { // don't declare init functions in the package scope - they are invisible obj.parent = pkg.scope check.recordObject(d.Name, obj) + // init functions must have a body + if d.Body == nil { + check.errorf(obj.pos, "missing function body") + // ok to continue + } } else { check.declareObj(pkg.scope, d.Name, obj) } diff --git a/go/types/stdlib_test.go b/go/types/stdlib_test.go index f032e9b9dda..d9c206802ca 100644 --- a/go/types/stdlib_test.go +++ b/go/types/stdlib_test.go @@ -37,34 +37,21 @@ func TestStdlib(t *testing.T) { } } -func TestStdtest(t *testing.T) { - path := filepath.Join(runtime.GOROOT(), "test") - +func testTestDir(t *testing.T, path string, ignore ...string) { files, err := ioutil.ReadDir(path) if err != nil { t.Fatal(err) } + excluded := make(map[string]bool) + for _, filename := range ignore { + excluded[filename] = true + } + fset := token.NewFileSet() for _, f := range files { // filter directory contents - if f.IsDir() || !strings.HasSuffix(f.Name(), ".go") { - continue - } - - // explicitly exclude files that the type-checker still has problems with - switch f.Name() { - case "cmplxdivide.go": - // This test also needs file cmplxdivide1.go; ignore. - continue - case "goto.go", "label1.go": - // TODO(gri) implement missing label checks - continue - case "sizeof.go", "switch.go": - // TODO(gri) tone down duplicate checking in expression switches - continue - case "typeswitch2.go": - // TODO(gri) implement duplicate checking in type switches + if f.IsDir() || !strings.HasSuffix(f.Name(), ".go") || excluded[f.Name()] { continue } @@ -76,13 +63,13 @@ func TestStdtest(t *testing.T) { file, err := parser.ParseFile(fset, filename, nil, parser.ParseComments|parser.AllErrors) // check per-file instructions - // For now we only check two cases. + // For now we only check some cases. expectErrors := false if len(file.Comments) > 0 { if group := file.Comments[0]; len(group.List) > 0 { cmd := strings.TrimSpace(group.List[0].Text[2:]) // 2: ignore // or /* of comment switch cmd { - case "skip": + case "skip", "compiledir": continue case "errorcheck": expectErrors = true @@ -107,6 +94,33 @@ func TestStdtest(t *testing.T) { } } +func TestStdtest(t *testing.T) { + testTestDir(t, filepath.Join(runtime.GOROOT(), "test"), + "cmplxdivide.go", // also needs file cmplxdivide1.go - ignore + "goto.go", "label1.go", // TODO(gri) implement missing label checks + "sizeof.go", "switch.go", // TODO(gri) tone down duplicate checking in expr switches + "typeswitch2.go", // TODO(gri) implement duplicate checking in type switches + ) +} + +func TestStdfixed(t *testing.T) { + testTestDir(t, filepath.Join(runtime.GOROOT(), "test/fixedbugs"), + "bug050.go", "bug088.go", "bug106.go", // TODO(gri) parser loses comments when bailing out early + "bug222.go", "bug282.go", "bug306.go", // TODO(gri) parser loses comments when bailing out early + "bug136.go", "bug179.go", "bug344.go", // TODO(gri) implement missing label checks + "bug165.go", // TODO(gri) isComparable not working for incomplete struct type + "bug176.go", // TODO(gri) composite literal array index must be non-negative constant + "bug200.go", // TODO(gri) complete duplicate checking in expr switches + "bug223.go", "bug413.go", "bug459.go", // TODO(gri) complete initialization checks + "bug248.go", "bug302.go", "bug369.go", // complex test instructions - ignore + "bug250.go", // TODO(gri) fix recursive interfaces + "bug326.go", // TODO(gri) assignment doesn't guard against len(rhs) == 0 + "bug373.go", // TODO(gri) implement use checks + "bug376.go", // TODO(gri) built-ins must be called (no built-in function expressions) + "issue3924.go", // TODO(gri) && and || produce bool result (not untyped bool) + ) +} + // Package paths of excluded packages. var excluded = map[string]bool{ "builtin": true, diff --git a/go/types/testdata/decls0.src b/go/types/testdata/decls0.src index 48dc07945e9..d0b44fd19d9 100644 --- a/go/types/testdata/decls0.src +++ b/go/types/testdata/decls0.src @@ -57,6 +57,7 @@ type init /* ERROR "cannot declare init" */ struct{} var _, init /* ERROR "cannot declare init" */ int func init() {} +func init /* ERROR "missing function body" */ () func _() { const init = 0 } func _() { type init int } diff --git a/go/types/testdata/shifts.src b/go/types/testdata/shifts.src index d576cc2fa94..cdc57e70e04 100644 --- a/go/types/testdata/shifts.src +++ b/go/types/testdata/shifts.src @@ -11,6 +11,7 @@ func shifts0() { _ = 0<<0 _ = 1<