[davecheney]

Compare string(a) with any string operand without copying the []byte operand when nothing between the conversion and the comparison can change it.

#5765 and #5773 needed both operands to be []byte conversions, but Go's gc compiler marks each operand on its own, and this change does the same.

The safety check from #5765 does not change, so a call, store, load, or slice between the conversion and the comparison keeps the copy.

On cortex-m-qemu with -opt=z, the size corpus code drops from 9923 to 9907 bytes.

[dgryski]

From a corrrectness standpoint the test corpus is happy with this change, but we should figure out what's happening with the AVR build (and also the binary size test failed CI).

[davecheney]

From a corrrectness standpoint the test corpus is happy with this change, but we should figure out what's happening with the AVR build (and also the binary size test failed CI).

The AVR build needs an exception, like math.go, because after inlining main.main is large enough it needs more stack than simavr provides.

[davecheney]

@dgryski @deadprogram after a bunch of side quests this is now ready for review.

[deadprogram]

Thanks @davecheney. I ran the tests locally. The following is edited from an automated review:

1. A string load (*s) or a string slice (s[:n]) between the conversion and the comparison can't write to the byte slice, and if either panics the comparison never runs. So adjacentComparison could perhaps let those through as well, as a follow up. Keeping it conservative for this PR seems fine too.

2. checkAllocs in tests/stringcompare/allocs_test.go checks for an exact TotalAlloc byte count. That relies on the conservative GC counting the requested size, not the rounded size. Should it check Mallocs only, so a GC change does not break this test?

WDYT?

[deadprogram]

My comments being just comments, this PR is looking good to me. Now merging, thank you @davecheney

[davecheney]

@deadprogram thanks for your comments

Thanks @davecheney. I ran the tests locally. The following is edited from an automated review:

1. A string load (*s) or a string slice (s[:n]) between the conversion and the comparison can't write to the byte slice, and if either panics the comparison never runs. So adjacentComparison could perhaps let those through as well, as a follow up. Keeping it conservative for this PR seems fine too.

fortunately I have #5937 waiting in the wings

2. checkAllocs in tests/stringcompare/allocs_test.go checks for an exact TotalAlloc byte count. That relies on the conservative GC counting the requested size, not the rounded size. Should it check Mallocs only, so a GC change does not break this test?

I'll take this under advisement. I'll send a PR to switch the memory size arithmetic.