Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

errors: As can be confusing to use when given a pointer to a type that does not implement error, but a pointer to that type does #37625

Closed
alvaroaleman opened this issue Mar 3, 2020 · 9 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@alvaroaleman
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.13.8 linux/amd64

Does this issue reproduce with the latest release?

asumming the playground uses the latest release, yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/alvaro/.gocache"
GOENV="/home/alvaro/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/alvaro/git/golang"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/alvaro/git/golang/src/k8s.io/kubernetes/staging/src/k8s.io/apimachinery/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build251903277=/tmp/go-build -gno-record-gcc-switches"

What did you do?

https://play.golang.org/p/TeXqOwa0wGM

What did you expect to see?

Successful compilation

What did you see instead?

second argument to errors.As must be a pointer to an interface or a type implementing error

The workaround is to pass in a double pointer, but that is super unintuitive: https://play.golang.org/p/kehktd-1b5t

@alvaroaleman alvaroaleman changed the title errors.Is does not work for error types that implement error via pointer receiver errors.As does not work for error types that implement error via pointer receiver Mar 3, 2020
@dmitshur dmitshur changed the title errors.As does not work for error types that implement error via pointer receiver errors: As does not work for error types that implement error via pointer receiver Mar 3, 2020
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 3, 2020
@dmitshur dmitshur added this to the Backlog milestone Mar 3, 2020
@dmitshur
Copy link
Contributor

dmitshur commented Mar 3, 2020

The second argument to errors.As must be a pointer, because As needs to modify the value that is being passed in. If you provide it a value rather than a pointer, that value gets copied, and any changes made to by errors.As would not be visible to the caller.

Additionally, the pointer must be to a type that implements error, since errors.As needs to deal with errors.

In your original snippet, the type *ptrErr implements the error interface. The type ptrErr does not. Note that this fails to compile:

var _ error = ptrErr{}

// ./prog.go:15:5: cannot use ptrErr literal (type ptrErr) as type error in assignment:
// 	ptrErr does not implement error (Error method has pointer receiver)

There isn't a way errors.As can do its job if you don't provide it with a pointer to a type that implements the error interface, that's why you see the error from it:

second argument to errors.As must be a pointer to an interface or a type implementing error

Do you think the error text can be improved to make this more clear?

/cc @jba @neild

@dmitshur dmitshur changed the title errors: As does not work for error types that implement error via pointer receiver errors: As can be confusing to use when given a pointer to a type that does not implement error, but a pointer to that type does Mar 3, 2020
@alvaroaleman
Copy link
Author

There isn't a way errors.As can do its job if you don't provide it with a pointer to a type that implements the error interface, that's why you see the error from it

But couldn't it check if the pointer type implements the error interface, rather than dereferencing it unconditionally first? As mentioned, its possible to workaround this by passing in a double pointer, but that is IMHO far from intuitive.

@dmitshur
Copy link
Contributor

dmitshur commented Mar 3, 2020

But couldn't it check if the pointer type implements the error interface, rather than dereferencing it unconditionally first?

At this time, I'm not sure if this is possible.

its possible to workaround this by passing in a double pointer

As far as I know, that's not just a workaround, that's the intended usage. A double pointer can be thought of as a single pointer to a value that implements the error interface.

From errors package documentation:

var perr *os.PathError
if errors.As(err, &perr) {
	fmt.Println(perr.Path)
}

Note that perr value is a pointer to os.PathError, and the second argument being passed to errors.As is a pointer to that.

Can you clarify if you're only interested in the boolean result value from errors.As? In the original snippet you shared, even if it didn't panic, it wouldn't be possible to get the value of the second argument to errors.As.

@dmitshur
Copy link
Contributor

dmitshur commented Mar 3, 2020

Also see https://blog.golang.org/go1.13-errors#TOC_3.2. if you haven't already.

@alvaroaleman
Copy link
Author

I see, thanks for your responses. So I think the main source of confusion for me here was that I read the error second argument to errors.As must be a pointer to an interface or a type implementing error as second argument to errors.As must be a (pointer to an interface) or (a type implementing error) rather than second argument to errors.As must be pointer to (an interface) or (a type implementing error).

Maybe we can reword it to sth like second argument to errors.As must be pointer to an interface or a pointer to a type implementing error (which itself may be a pointer) ?

Can you clarify if you're only interested in the boolean result value from errors.As? In the original snippet you shared, even if it didn't panic, it wouldn't be possible to get the value of the second argument to errors.As.

Yes I am, I just tried to keep the snippet short

@dmitshur
Copy link
Contributor

dmitshur commented Mar 3, 2020

I'm glad we figured out the source of confusion, that's very helpful and can be used to improve the error message.

There is yet another possible interpretation: second argument to errors.As must be a pointer to (an interface or a type) implementing error.

So, it should be helpful to reword the error text to make the interpretation less ambiguous.

@jba jba self-assigned this Mar 3, 2020
@gopherbot
Copy link

Change https://golang.org/cl/221877 mentions this issue: go/analysis/passes/erroras: clarify message

gopherbot pushed a commit to golang/tools that referenced this issue Mar 3, 2020
Make it clear that the second argument must be a non-nil pointer.

The new message text matches the phrasing used in the errors.As doc.

Updates golang/go#37625.

Change-Id: I69dc2e34a5f3a5573030ba0f63f20e0821be1816
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221877
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 3, 2020
@dmitshur dmitshur modified the milestones: Backlog, Go1.15 Mar 3, 2020
@odeke-em
Copy link
Member

odeke-em commented May 26, 2020

Thank you for filing this bug @alvaroaleman, and welcome to the Go project! Thanks for the work on it @dmitshur and @jba! I saw that @jba's CL https://go-review.googlesource.com/c/tools/+/221877/ added some clarification. Is there more that we could do for this issue? Thanks.

@alvaroaleman
Copy link
Author

Hey thanks for the ping, I think this can be considered solved, thanks for the help everyone!

@golang golang locked and limited conversation to collaborators May 26, 2021
@rsc rsc unassigned jba Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants