The only disadvantage I can imagine with doing this is that exporting the field or adding a method that sets the offset to -1 could mean a confusing loss of performance for the user. But I think that's just how the prove pass currently works. If one modifies code to be too clever or to unintentionally break BCE, they can already see slow-downs due to added bounds checks.

We don’t do much cross-function (whole package) analysis, and it’s all before SSA: typechecking, escape analysis, inlining. Doing whole package analysis in SSA is possible but would make concurrent compilation much less effective. And it’s not really built for it. I’d say this is probably unlikely to happen.

I imagined that the answer would be no, but I still think it's worth writing this down in an issue :) For example, I think this makes using unsigned integers for offset/index fields reasonable, if BCE matters.

Can the compiler hoist the bound check outside the loop, if the integer variable is only increasing and cannot overflow? In this case, if it panics it will panic on the first iteration, and if the first iteration doesn't panic, it won't panic.

This would be a local optimization, and I think it would bring most of the performance benefit.

@cherrymui you mean rewriting the code to be like:

func (t *T) withIntField(data []byte) {
        if t.intOff < 0 {
                panic("out of bounds...")
        }
	for t.intOff < len(t.data) {
		_ = t.data[t.intOff] // no bounds check
		t.intOff++
	}
}

I'm not sure how that would work, though. If there's an actual out of bounds panic, what line would the panic point to? Presumably the original _ = t.data[t.intOff], even though the statement won't actually contain a bounds check.

Another workaround for users could be to do that transformation themselves, meaning that there wouldn't be a need for the field to be unsigned. That doesn't work at the moment, but could be a simple enough change to the prove pass, without needing to care about fields:

func withIntVar(i int, data []byte) {
        if i < 0 {
                panic("unreachable")
        }
        for i < len(data) {
                _ = data[i] // unexpected bounds check
                i++
        }
}

@mvdan yes. The compiler can annotate the panic as the same line number of _ = t.data[t.intOff]. In another word, the compiler compiles the statement _ = t.data[t.intOff] to two pieces of code with the same line number (which the compiler already do in various ways), one outside the loop, one inside the loop.

the original _ = t.data[t.intOff], even though the statement won't actually contain a bounds check.

Or say it this way: this statement does have a bound check (which is expected), just not in every iteration.

This sounds eerily familiar:
https://dl.acm.org/citation.cfm?id=378850

this statement does have a bound check (which is expected), just not in every iteration.

Sounds doable. I think I'll raise a separate issue about that non-field BCE failure I showed at the end of my last comment, as I don't think it's related to this issue or its potential fix.

Separated that into #28956.

@cherrymui if the loop iterations have observable side effects, I think it’d be strange to panic partway through at a line number previous to the loop.

@josharian to be clear, I don't mean to do anything with line numbers. Currently the compiler generates a bound check, with the same line number, runs on every iteration. What I mean is that the compiler generates a bound check, with the same line number, runs only once.

Side effect is a good point, though. I can see it is harder to transform code like

	for t.intOff < len(t.data) {
		sideEffect() // can be a lot of code
		_ = t.data[t.intOff]
		t.intOff++
	}

It'd probably need to unroll the loop once, which is more complex and may or may not be beneficial when the iteration number is small.