cmd/compile: compiler error for valid code

[Jorropo]

reported by @ivanjaros in #72860 (comment)

I just updated my Go version to 1.25 and I immediately got compilation error for the following code - which is correct and should compile, but this new change makes it fail:

func RetrieveOauthCodeGrantHandler(ctx context.Context, req RetrieveOauthCodeGrantRequest) (res *RetrieveOauthCodeGrantResponse, err error) {
	errOut := func(errType, info string) {
		res = &RetrieveOauthCodeGrantResponse{
			Failure: &RetrieveOauthCodeGrantResponse_Failure{
				Error:            errType,
				ErrorDescription: info,
				ErrorURI:         "",
			},
		}
		err = nil
	}

	if err := req.Validate(); err != nil {
		errOut("invalid_request", errors.String(err))
		return
	}

	...
}

I had to rename the variable to e:

	if e := req.Validate(); e != nil {
		errOut("invalid_request", errors.String(e))
		return
	}

I think this change has not been implemented correctly if this is not compiling. I usually don't write my code like this but this is a special case where it had to be done like this and i caught it right away by accident(as I usually don't write code like this).

[Jorropo]

cc @ivanjaros can you please post the compiler error and ideally a repro thanks ?

[ivanjaros]

The error is:

	if err := req.Validate(); err != nil { // 2dn error line: "inner declaration of var err error"
		errOut("invalid_request", errors.String(err))
		return // 1st error line: "result parameter err not in scope at return"
	}

The repo is private but there is nothing special. It's just those few lines of code I posted above, which you have shared.

I doubt it needs to be explicitly stated but for the sake of clarity, note that this is because of/in the context of named return values.

[Jorropo]

The error looks correct, if err := ... shadows err (the named return) so when you return the named return variable is not in scope (given it is shadowed).
I don't know why it used to compile before, I've tried making a small repro but 1.23 and 1.24 also compile error:

func f() (b, a int) {
	af := func(b int) { a = b }

	if a := 42; a == 42 {
		af(a)
		return
	}
	return 42, 0
}

[TapirLiu]

Looks like #54019. IMHO, compilers should not do vet jobs.

@ivanjaros @Jorropo Can you confirm that the code compiles before Go 1.25?

[ianlancetaylor]

Note that this is mentioned in the language spec. It's the "implementation restriction" at https://go.dev/ref/spec#Return_statements.

[Jorropo]

@TapirLiu I'm not sure what you want me to do, I don't have access to the code

The repo is private

and I am not able to guess what magic incantation made 1.24 accept this as valid code #75069 (comment)

[TapirLiu]

Okay, then @ivanjaros solely again.

[ivanjaros]

There is no magic here guys. It's very simple.

The function has "err" as named return value, ie func foo() (err error).

Inside, there is basic function within the scope of foo that simply mutates the return value, as it has access to it.

func foo() (err error) {
  bar := func(someVarName string) {
    err = errors.New(someVarName) // <-- this mutates the return value of foo as it does not declare a new local err variable
  }

  bar("ooops")
  return
}

The code above will simply return error with value "ooops".

Now we can expand this and use an if block, as in the original issue:

func foo() (err error) {
  bar := func(someVarName string) {
    err = errors.New(someVarName) // <-- this mutates the return value of foo as it does not declare a new local err variable
  }

  // right here, before this block, the err is still referencing the foo's return value
  if err := baz(); err != nil {
    // but in here, the err is from the if block and the foo's error is not being referenced any more
    bar(err.Error()) // this will call bar() which will mutate the foo's err and not this block's err, as they are completely separate contexts
    return
  }

  // do something else...
  return
}

It is no different than something like this, except of an if block, it is a for loop block:

func foo() (err error) {
  bar := func(someVarName string) {
    err = errors.New(someVarName)
  }

  for i := 0; i < 10; i++ {
    err := baz()
    if err != nil {
      bar(err.Error())
      return // or break, same result
    }
  }

  return
}

If we translate this into a version without named return values, the compilation error makes perfect sense:

func foo() error {
  var err error
  bar := func(someVarName string) {
    err = errors.New(someVarName) // mutates the error variable declared above
  }

  if err := baz(); err != nil {
    // like before, we are declaring new variable with the same name as is in the scope of foo, outside of this block 
    bar(err.Error()) // mutates the "original" error
    return err // this returns the error from this block statement and not the original err as we have overwritten its declaration in this code block. this is why the compiler is having trouble, it needs to distinguish whether the function has named return values or not
  }

  // do something else...
  return err // returns the original error variable
}

As said in the last example, the problem here is that the compiler, with this new patch, is making no distinction between a function with named return value and one without.

[ianlancetaylor]

@ivanjaros Please show us a complete, standalone, program that demonstrates the problem. We are looking for a program that compiles and run successfully with old Go versions, and fails to compile with Go 1.25.

As far as I can tell, the partial programs that you are showing us will fail to compile with all Go versions. In order to find this bug, we need you to show us a complete program that compiles with old Go versions but not with Go 1.25.

Thanks.