From: griesemer Date: Fri, 6 Oct 2017 00:03:53 +0000 (-0700) Subject: go/doc: fix constant type propagation X-Git-Tag: go1.10beta1~818 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=d4023430fd77cd6c3ee97aa4ba9f4153427c8a8f;p=gostls13.git go/doc: fix constant type propagation The old code was seriously broken: It assumed that a constant declaration without a type would always inherit the type of the previous declaration, but in fact it only inherits the type of the previous declaration when there's no type and no constant value. While fixing this bug, found that the result was not sorted deterministically in all situations due to a poor choice of order value (which led to spurious test failures since the tests assume deterministic outputs). Fixed that as well. Added new test cases and fixed some old (broken) tests. Fixes #16153. Change-Id: I95b480e019b0fd3538638caba02fe651c69e0513 Reviewed-on: https://go-review.googlesource.com/68730 Reviewed-by: Alan Donovan --- diff --git a/src/go/doc/exports.go b/src/go/doc/exports.go index da9ea1f027..40cbb22797 100644 --- a/src/go/doc/exports.go +++ b/src/go/doc/exports.go @@ -200,7 +200,7 @@ func (r *reader) filterSpecList(list []ast.Spec, tok token.Token) []ast.Spec { var prevType ast.Expr for _, spec := range list { spec := spec.(*ast.ValueSpec) - if spec.Type == nil && prevType != nil { + if spec.Type == nil && len(spec.Values) == 0 && prevType != nil { // provide current spec with an explicit type spec.Type = copyConstType(prevType, spec.Pos()) } diff --git a/src/go/doc/reader.go b/src/go/doc/reader.go index 17635f0561..140f587233 100644 --- a/src/go/doc/reader.go +++ b/src/go/doc/reader.go @@ -154,6 +154,7 @@ type reader struct { imports map[string]int hasDotImp bool // if set, package contains a dot import values []*Value // consts and vars + order int // sort order of const and var declarations (when we can't use a name) types map[string]*namedType funcs methodSet @@ -256,11 +257,9 @@ func (r *reader) readValue(decl *ast.GenDecl) { if n, imp := baseTypeName(s.Type); !imp { name = n } - case decl.Tok == token.CONST: - // no type is present but we have a constant declaration; - // use the previous type name (w/o more type information - // we cannot handle the case of unnamed variables with - // initializer expressions except for some trivial cases) + case decl.Tok == token.CONST && len(s.Values) == 0: + // no type or value is present but we have a constant declaration; + // use the previous type name (possibly the empty string) name = prev } if name != "" { @@ -297,9 +296,15 @@ func (r *reader) readValue(decl *ast.GenDecl) { Doc: decl.Doc.Text(), Names: specNames(decl.Specs), Decl: decl, - order: len(*values), + order: r.order, }) decl.Doc = nil // doc consumed - remove from AST + + // Note: It's important that the order used here is global because the cleanupTypes + // methods may move values associated with types back into the global list. If the + // order is list-specific, sorting is not deterministic because the same order value + // may appear multiple times (was bug, found when fixing #16153). + r.order++ } // fields returns a struct's fields or an interface's methods. diff --git a/src/go/doc/testdata/blank.0.golden b/src/go/doc/testdata/blank.0.golden index c2987cf140..70f2929f8a 100644 --- a/src/go/doc/testdata/blank.0.golden +++ b/src/go/doc/testdata/blank.0.golden @@ -21,11 +21,18 @@ CONSTANTS C4 int ) - // Constants with an imported type that needs to be propagated. + // Constants with a single type that is not propagated. const ( - Default os.FileMode = 0644 - Useless = 0312 - WideOpen = 0777 + Default = 0644 + Useless = 0312 + WideOpen = 0777 + ) + + // Constants with an imported type that is propagated. + const ( + M1 os.FileMode + M2 + M3 ) // Package constants. diff --git a/src/go/doc/testdata/blank.1.golden b/src/go/doc/testdata/blank.1.golden index ee5054a4ed..8098cb6e88 100644 --- a/src/go/doc/testdata/blank.1.golden +++ b/src/go/doc/testdata/blank.1.golden @@ -23,14 +23,7 @@ CONSTANTS C4 ) - // Package constants. - const ( - _ int = iota - I1 - I2 - ) - - // Constants with an imported type that needs to be propagated. + // Constants with a single type that is not propagated. const ( zero os.FileMode = 0 Default = 0644 @@ -38,6 +31,21 @@ CONSTANTS WideOpen = 0777 ) + // Constants with an imported type that is propagated. + const ( + zero os.FileMode = 0 + M1 + M2 + M3 + ) + + // Package constants. + const ( + _ int = iota + I1 + I2 + ) + // Unexported constants counting from blank iota. See issue 9615. const ( _ = iota diff --git a/src/go/doc/testdata/blank.2.golden b/src/go/doc/testdata/blank.2.golden index c2987cf140..70f2929f8a 100644 --- a/src/go/doc/testdata/blank.2.golden +++ b/src/go/doc/testdata/blank.2.golden @@ -21,11 +21,18 @@ CONSTANTS C4 int ) - // Constants with an imported type that needs to be propagated. + // Constants with a single type that is not propagated. const ( - Default os.FileMode = 0644 - Useless = 0312 - WideOpen = 0777 + Default = 0644 + Useless = 0312 + WideOpen = 0777 + ) + + // Constants with an imported type that is propagated. + const ( + M1 os.FileMode + M2 + M3 ) // Package constants. diff --git a/src/go/doc/testdata/blank.go b/src/go/doc/testdata/blank.go index 419a78f7d5..5ea6186935 100644 --- a/src/go/doc/testdata/blank.go +++ b/src/go/doc/testdata/blank.go @@ -29,7 +29,7 @@ const ( C4 ) -// Constants with an imported type that needs to be propagated. +// Constants with a single type that is not propagated. const ( zero os.FileMode = 0 Default = 0644 @@ -37,6 +37,14 @@ const ( WideOpen = 0777 ) +// Constants with an imported type that is propagated. +const ( + zero os.FileMode = 0 + M1 + M2 + M3 +) + // Package constants. const ( _ int = iota diff --git a/src/go/doc/testdata/issue16153.0.golden b/src/go/doc/testdata/issue16153.0.golden new file mode 100644 index 0000000000..189260b4d0 --- /dev/null +++ b/src/go/doc/testdata/issue16153.0.golden @@ -0,0 +1,32 @@ +// +PACKAGE issue16153 + +IMPORTPATH + testdata/issue16153 + +FILENAMES + testdata/issue16153.go + +CONSTANTS + // + const ( + X3 int64 = iota + Y3 = 1 + ) + + // + const ( + X4 int64 = iota + Y4 + ) + + // original test case + const ( + Y1 = 256 + ) + + // variations + const ( + Y2 uint8 + ) + diff --git a/src/go/doc/testdata/issue16153.1.golden b/src/go/doc/testdata/issue16153.1.golden new file mode 100644 index 0000000000..803df3e709 --- /dev/null +++ b/src/go/doc/testdata/issue16153.1.golden @@ -0,0 +1,34 @@ +// +PACKAGE issue16153 + +IMPORTPATH + testdata/issue16153 + +FILENAMES + testdata/issue16153.go + +CONSTANTS + // original test case + const ( + x1 uint8 = 255 + Y1 = 256 + ) + + // variations + const ( + x2 uint8 = 255 + Y2 + ) + + // + const ( + X3 int64 = iota + Y3 = 1 + ) + + // + const ( + X4 int64 = iota + Y4 + ) + diff --git a/src/go/doc/testdata/issue16153.2.golden b/src/go/doc/testdata/issue16153.2.golden new file mode 100644 index 0000000000..189260b4d0 --- /dev/null +++ b/src/go/doc/testdata/issue16153.2.golden @@ -0,0 +1,32 @@ +// +PACKAGE issue16153 + +IMPORTPATH + testdata/issue16153 + +FILENAMES + testdata/issue16153.go + +CONSTANTS + // + const ( + X3 int64 = iota + Y3 = 1 + ) + + // + const ( + X4 int64 = iota + Y4 + ) + + // original test case + const ( + Y1 = 256 + ) + + // variations + const ( + Y2 uint8 + ) + diff --git a/src/go/doc/testdata/issue16153.go b/src/go/doc/testdata/issue16153.go new file mode 100644 index 0000000000..528be42356 --- /dev/null +++ b/src/go/doc/testdata/issue16153.go @@ -0,0 +1,27 @@ +// Copyright 2017 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 issue16153 + +// original test case +const ( + x1 uint8 = 255 + Y1 = 256 +) + +// variations +const ( + x2 uint8 = 255 + Y2 +) + +const ( + X3 int64 = iota + Y3 = 1 +) + +const ( + X4 int64 = iota + Y4 +)