Skip to content

Commit

Permalink
errors, fmt: revert rejected changes for Go 1.13
Browse files Browse the repository at this point in the history
Reverts the following changes:

  https://go.googlesource.com/go/+/1f90d081391d4f5911960fd28d81d7ea5e554a8f
  https://go.googlesource.com/go/+/8bf18b56a47a98b9dd2fa03beb358312237a8c76
  https://go.googlesource.com/go/+/5402854c3557f87fa2741a52ffc15dfb1ef333cc
  https://go.googlesource.com/go/+/37f84817247d3b8e687a701ccb0d6bc7ffe3cb78
  https://go.googlesource.com/go/+/6be6f114e0d483a233101a67c9644cd72bd3ae7a

Partially reverts the followinng change, removing the errors.Opaque
function and the errors.Wrapper type definition:

  https://go.googlesource.com/go/+/62f5e8156ef56fa61e6af56f4ccc633bde1a9120

Updates documentation referencing the Wrapper type.

Change-Id: Ia622883e39cafb06809853e3fd90b21441124534
Reviewed-on: https://go-review.googlesource.com/c/go/+/176997
Run-TryBot: Damien Neil <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Marcel van Lohuizen <[email protected]>
  • Loading branch information
neild committed May 15, 2019
1 parent 599ec77 commit 3e2c522
Show file tree
Hide file tree
Showing 15 changed files with 37 additions and 1,173 deletions.
34 changes: 2 additions & 32 deletions src/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,46 +5,16 @@
// Package errors implements functions to manipulate errors.
package errors

import (
"internal/errinternal"
"runtime"
)

// New returns an error that formats as the given text.
//
// The returned error contains a Frame set to the caller's location and
// implements Formatter to show this information when printed with details.
func New(text string) error {
// Inline call to errors.Callers to improve performance.
var s Frame
runtime.Callers(2, s.frames[:])
return &errorString{text, nil, s}
}

func init() {
errinternal.NewError = func(text string, err error) error {
var s Frame
runtime.Callers(3, s.frames[:])
return &errorString{text, err, s}
}
return &errorString{text}
}

// errorString is a trivial implementation of error.
type errorString struct {
s string
err error
frame Frame
s string
}

func (e *errorString) Error() string {
if e.err != nil {
return e.s + ": " + e.err.Error()
}
return e.s
}

func (e *errorString) FormatError(p Printer) (next error) {
p.Print(e.s)
e.frame.Format(p)
return e.err
}
34 changes: 0 additions & 34 deletions src/errors/format.go

This file was deleted.

47 changes: 0 additions & 47 deletions src/errors/frame.go

This file was deleted.

66 changes: 0 additions & 66 deletions src/errors/frame_test.go

This file was deleted.

29 changes: 3 additions & 26 deletions src/errors/wrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,35 +8,12 @@ import (
"internal/reflectlite"
)

// A Wrapper provides context around another error.
type Wrapper interface {
// Unwrap returns the next error in the error chain.
// If there is no next error, Unwrap returns nil.
Unwrap() error
}

// Opaque returns an error with the same error formatting as err
// but that does not match err and cannot be unwrapped.
func Opaque(err error) error {
return noWrapper{err}
}

type noWrapper struct {
error
}

func (e noWrapper) FormatError(p Printer) (next error) {
if f, ok := e.error.(Formatter); ok {
return f.FormatError(p)
}
p.Print(e.error)
return nil
}

// Unwrap returns the result of calling the Unwrap method on err, if err
// implements Wrapper. Otherwise, Unwrap returns nil.

This comment has been minimized.

Copy link
@AlekSi

AlekSi May 17, 2019

Contributor

@neild Here Wrapper type is still referenced. (Sorry, can't send CL myself)

This comment has been minimized.

Copy link
@neild

neild May 17, 2019

Author Contributor
func Unwrap(err error) error {
u, ok := err.(Wrapper)
u, ok := err.(interface {
Unwrap() error
})
if !ok {
return nil
}
Expand Down
61 changes: 0 additions & 61 deletions src/errors/wrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
package errors_test

import (
"bytes"
"errors"
"fmt"
"os"
Expand All @@ -16,8 +15,6 @@ func TestIs(t *testing.T) {
err1 := errors.New("1")
erra := wrapped{"wrap 2", err1}
errb := wrapped{"wrap 3", erra}
erro := errors.Opaque(err1)
errco := wrapped{"opaque", erro}

err3 := errors.New("3")

Expand All @@ -35,18 +32,13 @@ func TestIs(t *testing.T) {
{err1, err1, true},
{erra, err1, true},
{errb, err1, true},
{errco, erro, true},
{errco, err1, false},
{erro, erro, true},
{err1, err3, false},
{erra, err3, false},
{errb, err3, false},
{poser, err1, true},
{poser, err3, true},
{poser, erra, false},
{poser, errb, false},
{poser, erro, false},
{poser, errco, false},
{errorUncomparable{}, errorUncomparable{}, true},
{errorUncomparable{}, &errorUncomparable{}, false},
{&errorUncomparable{}, errorUncomparable{}, true},
Expand Down Expand Up @@ -107,10 +99,6 @@ func TestAs(t *testing.T) {
errF,
&errP,
true,
}, {
errors.Opaque(errT),
&errT,
false,
}, {
errorT{},
&errP,
Expand Down Expand Up @@ -187,7 +175,6 @@ func TestAsValidation(t *testing.T) {
func TestUnwrap(t *testing.T) {
err1 := errors.New("1")
erra := wrapped{"wrap 2", err1}
erro := errors.Opaque(err1)

testCases := []struct {
err error
Expand All @@ -198,9 +185,6 @@ func TestUnwrap(t *testing.T) {
{err1, nil},
{erra, err1},
{wrapped{"wrap 3", erra}, erra},

{erro, nil},
{wrapped{"opaque", erro}, erro},
}
for _, tc := range testCases {
if got := errors.Unwrap(tc.err); got != tc.want {
Expand All @@ -209,39 +193,6 @@ func TestUnwrap(t *testing.T) {
}
}

func TestOpaque(t *testing.T) {
someError := errors.New("some error")
testCases := []struct {
err error
next error
}{
{errorT{}, nil},
{wrapped{"b", nil}, nil},
{wrapped{"c", someError}, someError},
}
for _, tc := range testCases {
t.Run("", func(t *testing.T) {
opaque := errors.Opaque(tc.err)

f, ok := opaque.(errors.Formatter)
if !ok {
t.Fatal("Opaque error does not implement Formatter")
}
var p printer
next := f.FormatError(&p)
if next != tc.next {
t.Errorf("next was %v; want %v", next, tc.next)
}
if got, want := p.buf.String(), tc.err.Error(); got != want {
t.Errorf("error was %q; want %q", got, want)
}
if got := errors.Unwrap(opaque); got != nil {
t.Errorf("Unwrap returned non-nil error (%v)", got)
}
})
}
}

type errorT struct{}

func (errorT) Error() string { return "errorT" }
Expand All @@ -255,18 +206,6 @@ func (e wrapped) Error() string { return e.msg }

func (e wrapped) Unwrap() error { return e.err }

func (e wrapped) FormatError(p errors.Printer) error {
p.Print(e.msg)
return e.err
}

type printer struct {
errors.Printer
buf bytes.Buffer
}

func (p *printer) Print(args ...interface{}) { fmt.Fprint(&p.buf, args...) }

type errorUncomparable struct {
f []string
}
Expand Down
18 changes: 5 additions & 13 deletions src/fmt/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,28 +149,20 @@
1. If the operand is a reflect.Value, the operand is replaced by the
concrete value that it holds, and printing continues with the next rule.
2. If an operand implements the Formatter interface, and not
errors.Formatter, it will be invoked. Formatter provides fine
control of formatting.
2. If an operand implements the Formatter interface, it will
be invoked. Formatter provides fine control of formatting.
3. If the %v verb is used with the # flag (%#v) and the operand
implements the GoStringer interface, that will be invoked.
If the format (which is implicitly %v for Println etc.) is valid
for a string (%s %q %v %x %X), the following three rules apply:
for a string (%s %q %v %x %X), the following two rules apply:
4. If an operand implements errors.Formatter, the FormatError
method will be invoked with an errors.Printer to print the error.
If the %v flag is used with the + flag (%+v), the Detail method
of the Printer will return true and the error will be formatted
as a detailed error message. Otherwise the printed string will
be formatted as required by the verb (if any).
5. If an operand implements the error interface, the Error method
4. If an operand implements the error interface, the Error method
will be invoked to convert the object to a string, which will then
be formatted as required by the verb (if any).
6. If an operand implements method String() string, that method
5. If an operand implements method String() string, that method
will be invoked to convert the object to a string, which will then
be formatted as required by the verb (if any).
Expand Down
Loading

3 comments on commit 3e2c522

@AlekSi
Copy link
Contributor

@AlekSi AlekSi commented on 3e2c522 May 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any public discussion about reasons for that revert? https://go-review.googlesource.com/c/go/+/176997 doesn't have it IIUC.

Edit: #29934 (comment)

@ianlancetaylor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlekSi We don't use GitHub for code review. Very few people will see this comment. If you want to ask a question about a change, please comment on Gerrit, in this case at https://golang.org/cl/176997. Or ask on the golang-nuts mailing list. Thanks.

@neild
Copy link
Contributor Author

@neild neild commented on 3e2c522 May 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlekSi The decision on which portion of these changes to retain was announced here #29934 (comment) (buried in the issue comments, but there's a link to from the issue head). There's some follow-up discussion there.

Please sign in to comment.