Skip to content

proposal: Go 2: make := always shadow variables #65700

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
2 of 4 tasks
Manbeardo opened this issue Feb 14, 2024 · 6 comments
Closed
2 of 4 tasks

proposal: Go 2: make := always shadow variables #65700

Manbeardo opened this issue Feb 14, 2024 · 6 comments
Labels
LanguageChange Suggested changes to the Go language Proposal v2 An incompatible library change
Milestone

Comments

@Manbeardo
Copy link

Manbeardo commented Feb 14, 2024

Go Programming Experience

Experienced

Other Languages Experience

TypeScript, Java, Kotlin, Flow, Hack

Related Idea

  • Has this idea, or one like it, been proposed before?
  • Does this affect error handling?
  • Is this about generics?
  • Is this change backward compatible? Breaking the Go 1 compatibility guarantee is a large cost and requires a large benefit

Has this idea, or one like it, been proposed before?

There are other proposals that involve changing the behavior of :=, but I haven't found any that would make shadowing more permissive:

Does this affect error handling?

Shadowing variables in the same block would make multiline err checks easier to add/remove/reorder.

Example:

func myFunc() error {
    err := mutateStateA()
    if err != nil {
        return fmt.Errorf("mutating state A: %w", err)
    }

    err := mutateStateB()
    if err != nil {
        return fmt.Errorf("mutating state B: %w", err)
    }

    return nil
}

Because err is shadowed, the statements don't need to be edited depending on whether they're the first to declare err.

Is this about generics?

No

Proposal

When a variable that has already been declared is listed on the LHS of :=, always shadow it.

There are two primary use cases that this would simplify. The first is multiline-form error checks (see the error handling section). The second (and much more compelling) use case is for handling type-changing transformations to collections. When using functional programming paradigms, composing operations is one of the biggest pain points with the current implementation of generics. Being able to repeatedly declare the same variable with different types would help alleviate that pain without adding new syntax to support composition.

func fetchThings(ctx context.Context, things []string) ([]*DecoratedThing, error) {
    things, err := functional.Map(FetchThing)(things)
    // things.(type) == []*ThingFromDB
    if err != nil {
        return nil, fmt.Errorf("fetching from DB: %w", err)
    }

    things, err := functional.Map(AddSystemDataToThing)(things)
    // things.(type) == []*ThingWithSystemData
    if err != nil {
        return nil, fmt.Errorf("adding system data: %w", err)
    }

    things, err := functional.Map(func(thing *ThingWithSystemData) (*DecoratedThing, error) {
        return addContextDataToThing(ctx, thing)
    })(things)
    // things.(type) == []*DecoratedThing
    if err != nil {
        return nil, fmt.Errorf("adding context data: %w", err)
    }

    return things, nil
}

The example is a little bit contrived, but it can be genuinely difficult to come up with succinct and descriptive names for the intermediate steps when composing several collection operations together. Shadowing the original variable name has a couple other advantages:

  • You can't accidentally reference the output of the wrong step
  • Operations can safely be reordered by moving their lines without changing any names

Language Spec Changes

Under "Declarations and scope"

this paragraph:

A declaration binds a non-blank identifier to a constant, type, type parameter, variable, function, label, or package. Every identifier in a program must be declared. No identifier may be declared twice in the same block, and no identifier may be declared in both the file and package block.

becomes

A declaration binds a non-blank identifier to a constant, type, type parameter, variable, function, label, or package. Every identifier in a program must be declared. No identifier may be declared more than once in the same block unless it is a variable identifier. No identifier may be declared in both the file and package block.

Under "Declarations and scope"

this paragraph:

An identifier declared in a block may be redeclared in an inner block. While the identifier of the inner declaration is in scope, it denotes the entity declared by the inner declaration.

becomes

An identifier declared in a block may be redeclared in an inner block. Variable identifiers may also be redeclared within the same block. While the identifier of the most recent declaration is in scope, it denotes the entity declared by the most recent declaration.

Under "Short variable declarations"

this paragraph is removed:

Unlike regular variable declarations, a short variable declaration may redeclare variables provided they were originally declared earlier in the same block (or the parameter lists if the block is the function body) with the same type, and at least one of the non-blank variables is new. As a consequence, redeclaration can only appear in a multi-variable short declaration. Redeclaration does not introduce a new variable; it just assigns a new value to the original. The non-blank variable names on the left side of := must be unique.

Informal Change

No response

Is this change backward compatible?

No.

Because shadowing changes the variable type, this change could result in some variable types being downgraded from interfaces to concrete types:

func myFunc() (string, error) {
    var thing MyInterface
    thing, err := getThing()
    if err != nil {
        return "", err
    }
    // before: MyInterface
    // after:  *Thing
    return reflect.TypeOf(&thing).Elem().Name(), nil
}

Because shadowing would change access semantics, this change could also break some corner cases with pointers and goroutines:

func convertCase(strs map[string]struct{}) (map[string]struct{}, error) {
    inputChan := make(chan string)
    resultChan := make(chan string)
    var converter Converter
    go func() {
        for str := range inputChan {
            resultChan <- converter.Convert(str) 
        }
        close(resultChan)
    }()
    converter = SnakeCaseConverter()
    // old behavior assigns to the variable referenced in the first goroutine
    // new behavior creates a shadowed variable that is never accessed
    converter, err := UpperCaseConverter(converter)
    if err != nil {
        return nil, err
    }
    wg := sync.WaitGroup{}
    for str := range strs {
        wg.Add(1)
        go func() {
            inputChan <- str
            wg.Done()
        }()
    }
    go func() {
        wg.Wait()
        close(inputChan)
    }()
    out := map[string]struct{}{}
    for str := range resultChan {
        out[str] = struct{}{}
    }
    return out, nil
}

It's fairly subtle and actual incompatibilities might be rare enough to justify adding it, similar to the 1.22 changes to for variables.

Orthogonality: How does this change interact or overlap with existing features?

No response

Would this change make Go easier or harder to learn, and why?

In my opinion, this would make Go easier to learn because it would reduce the number of possible outcomes from using :=. After this change, you can say that variables on the LHS of := are always shadowed if they already exist in the current scope.

With the current behavior, you have to find which block each variable on the LHS was originally declared in to determine whether := will shadow or reuse.

Cost Description

The biggest adoption cost would likely be in the changes that would need to be made to the types package. The Scope.Lookup and Scope.LookupParent methods both assume that a variable can only be declared once per block, so they would need to be deprecated and redefined as returning either the first or last declaration of the named identifier.

Changes to Go ToolChain

any tool that uses types would probably need to be updated

Performance Costs

The run time cost should be fairly small since stack locations could still be reused if the shadowed variable doesn't escape and the new variable has the same size.

Depending on implementation details, compiler performance cost could go either way. There's potential for it to be a perf improvement since the logic of parsing := statements wouldn't require doing a lookup against the set of declared variables any more.

Prototype

No response

@Manbeardo Manbeardo added LanguageChange Suggested changes to the Go language Proposal v2 An incompatible library change labels Feb 14, 2024
@gopherbot gopherbot added this to the Proposal milestone Feb 14, 2024
@Manbeardo
Copy link
Author

Manbeardo commented Feb 14, 2024

I hit enter for a line break on the "Performance Costs" part of the form while writing this and it submitted the incomplete form. Still writing some sections... Should be ready for discussion now.

@Manbeardo Manbeardo changed the title proposal: Go 2: allow := to shadow variables in the same block proposal: Go 2: always shadow variables with := Feb 14, 2024
@Manbeardo Manbeardo changed the title proposal: Go 2: always shadow variables with := proposal: Go 2: make := always shadow variables Feb 14, 2024
@ianlancetaylor
Copy link
Contributor

We are extremely unlikely to adopt a language change that is not strictly backward compatible. The change to loop variable scope in 1.22 was an exception adopted only because a study of many examples showed that it fixed a lot of code while breaking almost none. I don't see how that kind of argument applies here.

@thediveo
Copy link
Contributor

This is a very dangerous incompatible breaking change: there is probably a very large corpus out there that, for instance, uses err variables as a func-wide status that gets handled in defer handlers. And then there are named return variables/values, same ballpark. I know I'm guilty of this, but have seen this in quite some other places as well. Even as an opt-in this would apply to a single module and thus break other, already existing packages in these modules.

@Manbeardo
Copy link
Author

FWIW, a transition could be made more safely across two releases by first removing the partially-declaring mode of := so that anything with potential to introduce bugs would disallowed.

Even without adding same-block shadowing, the partially-declaring mode sticks out to me as a bit of a complexity wart in the language. In my experience, it's almost exclusively used with one new variable and one reused err variable.

@ianlancetaylor
Copy link
Contributor

first removing the partially-declaring mode of := so that anything with potential to introduce bugs would disallowed.

But that would break an enormous amount of existing working code. That is a non-starter.

We simply aren't going to adopt this proposal. I'm going to close it. Sorry.

@ianlancetaylor ianlancetaylor closed this as not planned Won't fix, can't repro, duplicate, stale Feb 14, 2024
@alexmozaidze
Copy link

alexmozaidze commented Sep 19, 2024

Damn, this is unfortunate. Shadow-chaining would be an awesome feature to have, it would make things not require explicit names, and would make it a fairly good emulation of a chain operator some languages have

thing := 123 // type: int
thing := doSomething(thing) // type: string 
thing := doAnotherOperation(thing) // type: SomeStruct
return thing

It would be at least pretty cool for v2 of Go (if it ever comes).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LanguageChange Suggested changes to the Go language Proposal v2 An incompatible library change
Projects
None yet
Development

No branches or pull requests

5 participants