x/tools/go/analysis/passes/shadow: is confused by newtypes

[ainar-g]

Reproducer

package main

import "fmt"

type x int

type y int

func main() {
	as := []int{1, 2, 3}
	bs := []int{10, 20, 30}

	for _, a := range as {
		v := x(a)

		for _, b := range bs {
			v := y(b)
			fmt.Println(v)
		}

		fmt.Println(v)
	}
}

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

Expected

$ shadow --strict ./main.go
.../main.go:17:4: declaration of "v" shadows declaration at line 14
$

Got

$ shadow --strict ./main.go 
$

Details

If I remove the type conversions, the result is as expected.

OS, Versions, And So On

Formalities

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

$ go version
go version go1.17.3 linux/amd64
$ shadow -V
shadow: unsupported flag value: -V=true
$ strings $( which shadow ) | grep -F -e '0.1.8' | grep -F -e 'shadow'
/home/ainar/go/pkg/mod/golang.org/x/tools@v0.1.8-0.20211117171249-771dabf43312/go/analysis/passes/shadow/shadow.go
/home/ainar/go/pkg/mod/golang.org/x/tools@v0.1.8-0.20211117171249-771dabf43312/go/analysis/passes/shadow/cmd/shadow/main.go

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

go env Output

$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/ainar/.cache/go-build"
GOENV="/home/ainar/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/ainar/go/pkg/mod"
GONOPROXY="REMOVED"
GONOSUMDB="REMOVED"
GOOS="linux"
GOPATH="/home/ainar/go"
GOPRIVATE="REMOVED"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/ainar/go/go1.17"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/ainar/go/go1.17/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.3"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3097813802=/tmp/go-build -gno-record-gcc-switches"

[dr2chase]

@matloob PTAL.

[timothy-king]

IIUC this is the expected behavior. From the code:

	// Don't complain if the types differ: that implies the programmer really wants two different things.

From the documentation "A shadowed variable is a variable declared in an inner scope
with the same name and type as a variable in an outer scope," [emphasis added].

In the example, y is not identical to x as they are different named types.

I am missing the history for why shadow looks for the types to be identical and not the Underlying type to be identical. I am guessing there was a good reason though.

[ianlancetaylor]

As I recall it's simply that if the shadowing (inner) variable x is a different type, then almost any use of the shadowing x that intended to use the shadowed (outer) x would get an error anyhow. (I suppose that slice indexing is an exception to that, though.)

[timothy-king]

@ianlancetaylor I think my question is essentially, why is the test on the type (current Analyzer code):

	if types.Identical(obj.Type(), shadowed.Type()) {

instead of the underlying type:

	if types.Identical(obj.Type().Underlying(), shadowed.Type().Underlying()) {

Both seem somewhat reasonable for a shadow checker to me. For very different types, a type checking error does seem quite likely.

[timothy-king]

Here is a modified version of the original example that gives me some pause for whether switching from type identity to Underlying type identity makes sense. Here we have x and y with the same underlying type but different methods.

package main

import "fmt"

type x int
func (v x) Foo() {	fmt.Println("x.Foo()", v)}

type y x
func (v y) Foo() {	fmt.Println("y.Foo()", v)}

func main() {
	as := []int{1, 2, 3}
	bs := []int{10, 20, 30}
	for _, a := range as {
		v := x(a)
		for _, b := range bs {
			v := y(b)
			v.Foo()
		}
		v.Foo()
	}
}

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

[ianlancetaylor]

I see. I think I did misunderstand this. I have no answer for your question. It's probably fine to change it.