]> Cypherpunks repositories - gostls13.git/commit
cmd/compile: disallow rewrite rules from declaring reserved names
authorDaniel Martí <mvdan@mvdan.cc>
Mon, 22 Mar 2021 10:35:02 +0000 (11:35 +0100)
committerDaniel Martí <mvdan@mvdan.cc>
Mon, 22 Mar 2021 16:02:08 +0000 (16:02 +0000)
commit5437b5a24ba52c06d7ff627f01ed1876558959d2
tree58e3cce28163221653b42863e04cb9f55a9ae982
parentbd8b3fe5be9e9a5a2579c013451c07d53b827c56
cmd/compile: disallow rewrite rules from declaring reserved names

If I change a rule in ARM64.rules to use the variable name "b" in a
conflicting way, rulegen would previously not complain, and the compiler
would later give a confusing error:

$ go run *.go && go build cmd/compile/internal/ssa
# cmd/compile/internal/ssa
../rewriteARM64.go:24236:10: b.NewValue0 undefined (type int64 has no field or method NewValue0)

Make rulegen complain early about those cases. Sometimes they might
happen to be harmless, but in general they can easily cause confusion or
unintended effect due to shadowing.

After the change, with the same conflicting rule:

$ go run *.go && go build cmd/compile/internal/ssa
2021/03/22 11:31:49 rule ARM64.rules:495 uses the reserved name b
exit status 1

Note that 24 existing rules were using reserved names. It seems like the
shadowing was harmless, as it wasn't causing typechecking issues nor did
it seem to cause unintended behavior when the rule rewrite code ran.

The bool values "b" were renamed "t", since that seems to have a
precedent in other rules and in the fmt package.

Sequential values like "a b c" were renamed to "x y z", since "b" is
reserved.

Finally, "typ" was renamed to "_typ", since there doesn't seem to be an
obviously better answer.

Passes all three of:

$ GOARCH=amd64 go build -toolexec 'toolstash -cmp' -a std
$ GOARCH=arm64 go build -toolexec 'toolstash -cmp' -a std
$ GOARCH=mips64 go build -toolexec 'toolstash -cmp' -a std

Fixes #45154.

Change-Id: I1cce194dc7b477886a9c218c17973e996bcedccf
Reviewed-on: https://go-review.googlesource.com/c/go/+/303549
Trust: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
15 files changed:
src/cmd/compile/internal/ssa/gen/ARM.rules
src/cmd/compile/internal/ssa/gen/ARM64.rules
src/cmd/compile/internal/ssa/gen/MIPS.rules
src/cmd/compile/internal/ssa/gen/MIPS64.rules
src/cmd/compile/internal/ssa/gen/PPC64.rules
src/cmd/compile/internal/ssa/gen/S390X.rules
src/cmd/compile/internal/ssa/gen/generic.rules
src/cmd/compile/internal/ssa/gen/rulegen.go
src/cmd/compile/internal/ssa/rewriteARM.go
src/cmd/compile/internal/ssa/rewriteARM64.go
src/cmd/compile/internal/ssa/rewriteMIPS.go
src/cmd/compile/internal/ssa/rewriteMIPS64.go
src/cmd/compile/internal/ssa/rewritePPC64.go
src/cmd/compile/internal/ssa/rewriteS390X.go
src/cmd/compile/internal/ssa/rewritegeneric.go