From f5758739a8f011c1d146a7736ab8f0d2834e1783 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 1 Aug 2016 14:33:19 -0700 Subject: [PATCH 1/3] cmd/doc: handle embedded interfaces properly Changes made: * Disallow star expression on interfaces as this is not possible. * Show an embedded "error" in an interface as public similar to how godoc does it. * Properly handle selector expressions in both structs and interfaces. This is possible since a type may refer to something defined in another package (e.g. io.Reader). Before: <<< $ go doc runtime.Error type Error interface { // RuntimeError is a no-op function but // serves to distinguish types that are run time // errors from ordinary errors: a type is a // run time error if it has a RuntimeError method. RuntimeError() // Has unexported methods. } $ go doc compress/flate Reader doc: invalid program: unexpected type for embedded field doc: invalid program: unexpected type for embedded field type Reader interface { io.Reader io.ByteReader } >>> After: <<< $ go doc runtime.Error type Error interface { error // RuntimeError is a no-op function but // serves to distinguish types that are run time // errors from ordinary errors: a type is a // run time error if it has a RuntimeError method. RuntimeError() } $ go doc compress/flate Reader type Reader interface { io.Reader io.ByteReader } >>> Fixes #16567 Change-Id: I272dede971eee9f43173966233eb8810e4a8c907 Reviewed-on: https://go-review.googlesource.com/25365 Reviewed-by: Rob Pike Run-TryBot: Joe Tsai TryBot-Result: Gobot Gobot --- src/cmd/doc/doc_test.go | 8 ++++++++ src/cmd/doc/pkg.go | 24 ++++++++++++++++++++---- src/cmd/doc/testdata/pkg.go | 4 ++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/cmd/doc/doc_test.go b/src/cmd/doc/doc_test.go index 5cb1ec990e..bfb9099dd2 100644 --- a/src/cmd/doc/doc_test.go +++ b/src/cmd/doc/doc_test.go @@ -221,6 +221,7 @@ var tests = []test{ `func \(ExportedType\) ExportedMethod\(a int\) bool`, `const ExportedTypedConstant ExportedType = iota`, // Must include associated constant. `func ExportedTypeConstructor\(\) \*ExportedType`, // Must include constructor. + `io.Reader.*Comment on line with embedded Reader.`, }, []string{ `unexportedField`, // No unexported field. @@ -228,6 +229,7 @@ var tests = []test{ `Comment about exported method.`, // No comment about exported method. `unexportedMethod`, // No unexported method. `unexportedTypedConstant`, // No unexported constant. + `error`, // No embedded error. }, }, // Type -u with unexported fields. @@ -243,6 +245,8 @@ var tests = []test{ `\*ExportedEmbeddedType.*Comment on line with exported embedded \*field.`, `unexportedType.*Comment on line with unexported embedded field.`, `\*unexportedType.*Comment on line with unexported embedded \*field.`, + `io.Reader.*Comment on line with embedded Reader.`, + `error.*Comment on line with embedded error.`, `func \(ExportedType\) unexportedMethod\(a int\) bool`, `unexportedTypedConstant`, }, @@ -274,6 +278,8 @@ var tests = []test{ `type ExportedInterface interface`, // Interface definition. `Comment before exported method.*\n.*ExportedMethod\(\)` + `.*Comment on line with exported method`, + `io.Reader.*Comment on line with embedded Reader.`, + `error.*Comment on line with embedded error.`, `Has unexported methods`, }, []string{ @@ -293,6 +299,8 @@ var tests = []test{ `Comment before exported method.*\n.*ExportedMethod\(\)` + `.*Comment on line with exported method`, `unexportedMethod\(\).*Comment on line with unexported method.`, + `io.Reader.*Comment on line with embedded Reader.`, + `error.*Comment on line with embedded error.`, }, []string{ `Has unexported methods`, diff --git a/src/cmd/doc/pkg.go b/src/cmd/doc/pkg.go index efd681d514..eec9f1e803 100644 --- a/src/cmd/doc/pkg.go +++ b/src/cmd/doc/pkg.go @@ -494,14 +494,19 @@ func trimUnexportedElems(spec *ast.TypeSpec) { } switch typ := spec.Type.(type) { case *ast.StructType: - typ.Fields = trimUnexportedFields(typ.Fields, "fields") + typ.Fields = trimUnexportedFields(typ.Fields, false) case *ast.InterfaceType: - typ.Methods = trimUnexportedFields(typ.Methods, "methods") + typ.Methods = trimUnexportedFields(typ.Methods, true) } } // trimUnexportedFields returns the field list trimmed of unexported fields. -func trimUnexportedFields(fields *ast.FieldList, what string) *ast.FieldList { +func trimUnexportedFields(fields *ast.FieldList, isInterface bool) *ast.FieldList { + what := "methods" + if !isInterface { + what = "fields" + } + trimmed := false list := make([]*ast.Field, 0, len(fields.List)) for _, field := range fields.List { @@ -511,12 +516,23 @@ func trimUnexportedFields(fields *ast.FieldList, what string) *ast.FieldList { // Nothing else is allowed. switch ident := field.Type.(type) { case *ast.Ident: + if isInterface && ident.Name == "error" && ident.Obj == nil { + // For documentation purposes, we consider the builtin error + // type special when embedded in an interface, such that it + // always gets shown publicly. + list = append(list, field) + continue + } names = []*ast.Ident{ident} case *ast.StarExpr: // Must have the form *identifier. - if ident, ok := ident.X.(*ast.Ident); ok { + // This is only valid on embedded types in structs. + if ident, ok := ident.X.(*ast.Ident); ok && !isInterface { names = []*ast.Ident{ident} } + case *ast.SelectorExpr: + // An embedded type may refer to a type in another package. + names = []*ast.Ident{ident.Sel} } if names == nil { // Can only happen if AST is incorrect. Safe to continue with a nil list. diff --git a/src/cmd/doc/testdata/pkg.go b/src/cmd/doc/testdata/pkg.go index 5f79414b33..9c5cf8f557 100644 --- a/src/cmd/doc/testdata/pkg.go +++ b/src/cmd/doc/testdata/pkg.go @@ -66,6 +66,8 @@ type ExportedType struct { *ExportedEmbeddedType // Comment on line with exported embedded *field. unexportedType // Comment on line with unexported embedded field. *unexportedType // Comment on line with unexported embedded *field. + io.Reader // Comment on line with embedded Reader. + error // Comment on line with embedded error. } // Comment about exported method. @@ -96,6 +98,8 @@ type ExportedInterface interface { // Comment before exported method. ExportedMethod() // Comment on line with exported method. unexportedMethod() // Comment on line with unexported method. + io.Reader // Comment on line with embedded Reader. + error // Comment on line with embedded error. } // Comment about unexported type. From 6317c213c953d0879fe88593b4372f03d25f369b Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 1 Aug 2016 20:04:25 -0700 Subject: [PATCH 2/3] cmd/doc: ensure functions with unexported return values are shown The commit in golang.org/cl/22354 groups constructors functions under the type that they construct to. However, this caused a minor regression where functions that had unexported return values were not being printed at all. Thus, we forgo the grouping logic if the type the constructor falls under is not going to be printed. Fixes #16568 Change-Id: Idc14f5d03770282a519dc22187646bda676af612 Reviewed-on: https://go-review.googlesource.com/25369 Run-TryBot: Joe Tsai Reviewed-by: Rob Pike TryBot-Result: Gobot Gobot --- src/cmd/doc/doc_test.go | 8 +++++--- src/cmd/doc/pkg.go | 4 +++- src/cmd/doc/testdata/pkg.go | 3 +++ 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/cmd/doc/doc_test.go b/src/cmd/doc/doc_test.go index bfb9099dd2..453a3d53aa 100644 --- a/src/cmd/doc/doc_test.go +++ b/src/cmd/doc/doc_test.go @@ -61,6 +61,7 @@ var tests = []test{ `var ExportedVariable = 1`, // Simple variable. `var VarOne = 1`, // First entry in variable block. `func ExportedFunc\(a int\) bool`, // Function. + `func ReturnUnexported\(\) unexportedType`, // Function with unexported return type. `type ExportedType struct { ... }`, // Exported type. `const ExportedTypedConstant ExportedType = iota`, // Typed constant. `const ExportedTypedConstant_unexported unexportedType`, // Typed constant, exported for unexported type. @@ -89,9 +90,10 @@ var tests = []test{ "full package with u", []string{`-u`, p}, []string{ - `const ExportedConstant = 1`, // Simple constant. - `const internalConstant = 2`, // Internal constants. - `func internalFunc\(a int\) bool`, // Internal functions. + `const ExportedConstant = 1`, // Simple constant. + `const internalConstant = 2`, // Internal constants. + `func internalFunc\(a int\) bool`, // Internal functions. + `func ReturnUnexported\(\) unexportedType`, // Function with unexported return type. }, []string{ `Comment about exported constant`, // No comment for simple constant. diff --git a/src/cmd/doc/pkg.go b/src/cmd/doc/pkg.go index eec9f1e803..defddfd74a 100644 --- a/src/cmd/doc/pkg.go +++ b/src/cmd/doc/pkg.go @@ -317,7 +317,9 @@ func (pkg *Package) funcSummary(funcs []*doc.Func, showConstructors bool) { isConstructor = make(map[*doc.Func]bool) for _, typ := range pkg.doc.Types { for _, constructor := range typ.Funcs { - isConstructor[constructor] = true + if isExported(typ.Name) { + isConstructor[constructor] = true + } } } } diff --git a/src/cmd/doc/testdata/pkg.go b/src/cmd/doc/testdata/pkg.go index 9c5cf8f557..6a52ac2f65 100644 --- a/src/cmd/doc/testdata/pkg.go +++ b/src/cmd/doc/testdata/pkg.go @@ -123,3 +123,6 @@ const unexportedTypedConstant unexportedType = 1 // In a separate section to tes // For case matching. const CaseMatch = 1 const Casematch = 2 + +func ReturnUnexported() unexportedType { return 0 } +func ReturnExported() ExportedType { return ExportedType{} } From 2da5633eb9091608047881953f75b489a3134cdc Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 1 Aug 2016 21:54:40 -0700 Subject: [PATCH 3/3] runtime: fix nanotime for macOS Sierra, again. macOS Sierra beta4 changed the kernel interface for getting time. DX now optionally points to an address for additional info. Set it to zero to avoid corrupting memory. Fixes #16570 Change-Id: I9f537e552682045325cdbb68b7d0b4ddafade14a Reviewed-on: https://go-review.googlesource.com/25400 Reviewed-by: David Crawshaw Reviewed-by: Ian Lance Taylor Reviewed-by: Quentin Smith Run-TryBot: Brad Fitzpatrick Reviewed-by: Austin Clements TryBot-Result: Gobot Gobot --- src/runtime/sys_darwin_386.s | 7 ++++--- src/runtime/sys_darwin_amd64.s | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/runtime/sys_darwin_386.s b/src/runtime/sys_darwin_386.s index 83f4709f38..b5e65e6869 100644 --- a/src/runtime/sys_darwin_386.s +++ b/src/runtime/sys_darwin_386.s @@ -196,15 +196,16 @@ timeloop: systime: // Fall back to system call (usually first call in this thread) - LEAL 12(SP), AX // must be non-nil, unused + LEAL 16(SP), AX // must be non-nil, unused MOVL AX, 4(SP) MOVL $0, 8(SP) // time zone pointer + MOVL $0, 12(SP) // required as of Sierra; Issue 16570 MOVL $116, AX INT $0x80 CMPL AX, $0 JNE inreg - MOVL 12(SP), AX - MOVL 16(SP), DX + MOVL 16(SP), AX + MOVL 20(SP), DX inreg: // sec is in AX, usec in DX // convert to DX:AX nsec diff --git a/src/runtime/sys_darwin_amd64.s b/src/runtime/sys_darwin_amd64.s index e4837ce291..ea2cc068f6 100644 --- a/src/runtime/sys_darwin_amd64.s +++ b/src/runtime/sys_darwin_amd64.s @@ -157,6 +157,7 @@ systime: // Fall back to system call (usually first call in this thread). MOVQ SP, DI MOVQ $0, SI + MOVQ $0, DX // required as of Sierra; Issue 16570 MOVL $(0x2000000+116), AX SYSCALL CMPQ AX, $0