cmd/vet: tests require Perl for errchk and fail with latest Perl

[genehack]

Please answer these questions before submitting your issue. Thanks!

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

go version go1.8.1 linux/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/genehack/go"
GORACE=""
GOROOT="/opt/go"
GOTOOLDIR="/opt/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build831712468=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

Grabbed the source tarball. Grabbed go-1.4 binary tarball and expanded under $HOME. Built 1.8.1 source per instructions (cd src; ./all.bash). Got test suite failure due to a change in regex syntax that became a fatal error in 5.24. (I'm running perl-5.25.11, currently; once I submit this I will replicate with 5.24, 5.22, and (hopefully!) not replicate with 5.20.)

What did you expect to see?

Fully passing test suite.

What did you see instead?

ok      cmd/vendor/golang.org/x/arch/x86/x86asm 0.195s
--- FAIL: TestVet (1.40s)
    --- FAIL: TestVet/5 (0.24s)
        vet_test.go:114: files: ["testdata/copylock_func.go" "testdata/nilfunc.go" "testdata/unsafeptr.go"]
        vet_test.go:163: vet output:
                Unescaped left brace in regex is illegal here in regex; marked by <-- HERE in m/call of fntab.0. copies lock value: struct{ <-- HERE lock sync.Mutex} contains sync.Mutex/ at /opt/go/test/errchk line 128.
        vet_test.go:164: exit status 1

[genehack]

Update: I read the perldelta wrong: unescaped left-brace is only deprecated in 5.24. It will become fatal in 5.26 (which is expected out soon, within a few months). So, currently, this bug will only show up for folks running Perls from the 5.25.x development releases.

[ianlancetaylor]

The actual use of perl is in the script test/errchk.

[bradfitz]

@ianlancetaylor, we have a Go version of errchk already in test/run.go. (hence my longer title about one view of the bug being that we depend on Perl)

[bradfitz]

That said, it might be just changing:

    $regexp = "$`$file:$n$'";

to:

    $regexp = "\\Q$`\\E$file:$n$'";

(or quotameta)

[ianlancetaylor]

Sorry, our subject line editing collided. I never saw yours.

[Leont]

Quite frankly, this entire test suite is confused whether the error lines it's dealing with are plain strings or regexes.

This particular error is caused by this line. This is clearly meant to be matched stringwise, as are many other strings (that work fine most of the time because the meta-character . matches itself as literal character .).

Do you really need these to be regexes? If not I would recommend just quotemeta()ing it early (this may involve unquoting a few cases that are currently hand-quoted, for example because they contain a *).

If you do need it, I would recommend consistently quoting all error strings.

One possible intermediate solution is to add a new syntax that will explicitly enable the regular expression syntax. Something like:

  foo(1) // ERROR /some \d+ regexp/

All of this you really want to fix regardless of what perl is doing.

[ianlancetaylor]

In general the ERROR lines that appear in the files in the test directory are always intended to be regular expressions. I am less familiar with the use in vet, but if vet wants to match the way that the test directory works then the ERROR lines there should also always be regexps. Thanks for identifying the line; probably the right solution is to backslash-quote the . characters.

[Leont]

Thanks for identifying the line; probably the right solution is to backslash-quote the . characters.

The primary problem here isn't the . characters, but the {s