redefining for loop variable semantics

[Russ Cox]

I just posted a GitHub discussion that may be of interest:
https://github.com/golang/go/discussions/56010

Best,

Russ

[Jan Mercl]

On Mon, Oct 3, 2022 at 4:21 PM Russ Cox <r...@golang.org> wrote:

> I just posted a GitHub discussion that may be of interest:
> https://github.com/golang/go/discussions/56010

Let me please ask for clarification about code like this:

        package main

       
        type t struct {
                // ...
        }
       
        func (p *t) foo() {
                // ...
        }
       
        func bar() []t {
                return nil // but actually some non-empty slice of t
        }
       
        func main() {
                for _, v := range bar() {
                        v.foo() // <-- line 17
                }
        }

(https://go.dev/play/p/3x5uGPgvgiS)

1) Currently line 17 allocates at most a single instance of t for all loop passes, with the proposal it may allocate one instance of t for every loop pass.

2) With the proposal, code snippets published anywhere, both existing and the future ones, online or printed, cannot be read and analysed correctly by a human if the go directive used in go.mod, or the go.mod per se is not available to the reader. Even if the go.mod is available, like when working within a repository, it defines the semantics of the for loop, but in a different file. In contrast to, for example, build tags, located conveniently near the top of the same file or implicitly in the [same] file name.

Are [some of] those observations correct or not? If so, are all such seen as acceptable by most people?

-j

[Alan Donovan]

On Mon, 3 Oct 2022 at 11:01, Jan Mercl <0xj...@gmail.com> wrote:

Let me please ask for clarification about code like this:

        package main

       
        type t struct {
                // ...
        }
       
        func (p *t) foo() {
                // ...
        }
       
        func bar() []t {
                return nil // but actually some non-empty slice of t
        }
       
        func main() {
                for _, v := range bar() {
                        v.foo() // <-- line 17
                }
        }

(https://go.dev/play/p/3x5uGPgvgiS)

1) Currently line 17 allocates at most a single instance of t for all loop passes, with the proposal it may allocate one instance of t for every loop pass.

Correct. If v.foo causes &v to escape then this program will perform an asymptotically larger number of heap allocations.

2) With the proposal, code snippets published anywhere, both existing and the future ones, online or printed, cannot be read and analysed correctly by a human if the go directive used in go.mod, or the go.mod per se is not available to the reader. Even if the go.mod is available, like when working within a repository, it defines the semantics of the for loop, but in a different file. In contrast to, for example, build tags, located conveniently near the top of the same file or implicitly in the [same] file name.

Correct. That's a really good point.

>  If so, are all such seen as acceptable by most people?

I think the purpose of the discussion is to learn the answer to that question.

 

[Russ Cox]

The only thing I would add to Alan's answers is:

On Mon, Oct 3, 2022 at 11:01 AM Jan Mercl <0xj...@gmail.com> wrote:

2) With the proposal, code snippets published anywhere, both existing and the future ones, online or printed, cannot be read and analysed correctly by a human if the go directive used in go.mod, or the go.mod per se is not available to the reader. Even if the go.mod is available, like when working within a repository, it defines the semantics of the for loop, but in a different file. In contrast to, for example, build tags, located conveniently near the top of the same file or implicitly in the [same] file name.

You're absolutely right, and these reasons are exactly why we do not want to do redefinitions in general. The discussion aims to establish whether it makes sense to make an exception for this case. The evidence we have so far suggests it may.

Best,

Russ

 

[David Chase]

What we find is that escape analysis is pretty good, though there are some cases where it creates a new allocation.  But if the allocation has substantial effect, it shows up in profiling and the fix is trivial, so I call this a lesser error -- easy to find, easy to fix.

--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-dev/CACKEwjF3_Xv1bAw4KVmrVWZg06kYQimmutV6T8NPQgPq6pZRdQ%40mail.gmail.com.

[Jan Mercl]

On Mon, Oct 3, 2022 at 6:09 PM Russ Cox <r...@golang.org> wrote:

> You're absolutely right, and these reasons are exactly why we do not want to do redefinitions in general. The discussion aims to establish whether it makes sense to make an exception for this case. The evidence we have so far suggests it may.

Thanks all for the answers.

My personal opinion is that the aggregated cost is too high.

What about making the redefinition explicitly visible by some syntactic construct like (analogically for the three item for loop):

        for var <one-or-two-variables> := range expression { ... }

or  perhaps something like

        for <one-or-two-variables> ::= range <expression> { ... }

(I'd prefer the var keyword because for me it's a hard to miss reminder of the new loop variable. Also, no changes to existing Go scanners.)

-j

[Hein Meling]

I’m not sure adding the var keyword or ::= is much different than doing x := x. Seems like both are easy to forget, and you need to teach people the difference.

[U Cirello]

I wonder if we could use a conditional compilation comment and provide per package migration support? 

Inside a full Go application, with millions of lines, it would be a herculean task to check every internal package in a single shot.

So the proposal works for external dependencies (as far as the external dependency is able to cope with its own size), but it doesn't quite cut for the application internally.

With the conditional compilation comment, I can divide and conquer the migration process and not be blocked from upgrade my own application to a newer Go version.

It could support two modes. I could mark my application 1.30 and exclude packages individually, or I could mark my application 1.20 and include packages individually.

It would give me two controlled ways to migrate the source code. I imagine gofix could help with that? 

In any case, there's one precedent, or rather a close enough precendent, which the forced import name at package level ("//import pkgname") so I believe this pattern could be leveraged for this flow. 

--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-dev/a6e721a5-402e-4303-84fd-b68c19331958n%40googlegroups.com.

[Russ Cox]

On Tue, Oct 4, 2022 at 1:11 PM U Cirello <ulderi...@gmail.com> wrote:

I wonder if we could use a conditional compilation comment and provide per package migration support? 

I think the way a per-package migration would look would be a tool that, with the compiler's help,

identifies loops that might be changing. This will not be anywhere near all the loops in a program.

And then it would rewrite just those loops to declare the variable ahead of the loop, to preserve

the old semantics. Then updating to Go 1.30 would be a no-op, at which point you could revert 

those changes in incremental steps.

Best,

Russ

[Juliusz Chroboczek]

> What about making the redefinition explicitly visible by some syntactic
> construct like (analogically for the three item for loop):
>
> for var <one-or-two-variables> := range expression { ... }
>
> or perhaps something like
>
> for <one-or-two-variables> ::= range <expression> { ... }

Please don't. Just pick one. Please.

(I have a preference for the Scheme-like semantics, but I've spent years
of my life switching between Scheme and Common Lisp, so I know I can
live with either. But let's please not have two similar but subtly
different loop constructions in the same language.)