Skip to content
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

spec: add "with" conditional branch #68968

Closed
1 of 4 tasks
jkassis opened this issue Aug 20, 2024 · 26 comments
Closed
1 of 4 tasks

spec: add "with" conditional branch #68968

jkassis opened this issue Aug 20, 2024 · 26 comments
Labels
LanguageChange Suggested changes to the Go language LanguageChangeReview Discussed by language change review committee Proposal Proposal-FinalCommentPeriod
Milestone

Comments

@jkassis
Copy link

jkassis commented Aug 20, 2024

Go Programming Experience

Experienced

Other Languages Experience

So many

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?

I don't think so.

Does this affect error handling?

No.

Is this about generics?

No.

Proposal

create an "with" branch for conditionals. the scope of with includes the scope of "if-else" so that variables declared and assigned in the scope of "if-else" appear in the scope of "with" and variable declared in the scope of "with" appear in the scope of the the "if-else". in some cases, this removes the need for a nested "if-else" block.

this...

if a, err := f(b); err != nil {
  ...
} else {
  a.foo()
  if b, ok := bar(a); ok {
    ...
  } 
}

would be...

if a, err := f(b); err != nil {
  ...
} with {
  a.foo()
} else if b, ok := bar(a); ok {
    ...
}

for complex blocks...\

else if func () bool {... ; return false }()

becomes

with{...}

Language Spec Changes

addition of an "always" block to if-else sequences.

Informal Change

always blocks allow you to perform operations on variables declared and assigned in if-else statements.

Is this change backward compatible?

yes.

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

no.

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

easier. cleaner syntax. less nesting is always good.

Cost Description

trival.

Changes to Go ToolChain

No response

Performance Costs

No response

Prototype

No response

@jkassis jkassis added LanguageChange Suggested changes to the Go language LanguageChangeReview Discussed by language change review committee Proposal labels Aug 20, 2024
@gopherbot gopherbot added this to the Proposal milestone Aug 20, 2024
@randall77
Copy link
Contributor

How does "always" differ from "else true"?

@ianlancetaylor
Copy link
Contributor

I don't understand the example. It boils down to

    if c1 {
        ...
    } else {
        if c2 {
            ...
        }
        f()
    }

This converts to

    if c1 {
        ...
    } else if c2 {
        ...
    } always {
        f()
    }

For the converted code to be the same as the original code, it must be the case that the always block is only executed if c1 is false, and is executed whether c2 is true or false. I don't understand what always means.

@timothy-king
Copy link
Contributor

I believe that the intent is to add a new keyword always. That will not be backwards compatible. Preexisting variables/functions can be named always and those would no longer be permitted. If the intent is not a new keyword, can you explain how always would work.

@thediveo
Copy link

I'm also finding the OP proposal confusing. It breaks the identation logic as can be seen in the example. It is unexpected in that it looks much like a Python finally but seems to apply only to its immediate preceding block...?

@jkassis
Copy link
Author

jkassis commented Aug 22, 2024

the proposal is for a new keyword, but one that is only valid in the context of an if / else block and not as a "condition" of "if" or "else if"... thus it doesn't conflict with variables named "always".

@ianlancetaylor the example does not reduce to that simplified conditional because conditions in go can be used to declare and assign variables within the scope of the if/else block.

if a := fn1() ; a {
  fn2(a)
}

creates variable "a" that is valid for the scope of the conditional block.

in this...

if a := fn1() ; a {
  fn2(a)
}
fn3(a)

fn3(a) is invalid because the scope of a has already disappeared.

the ability to declare variables for only the scope of a sequence of if/else conditionals leads to clean, sequential, maintainable, readable code until the programmer needs to perform some work on or with a.

the proposal is not for a "finally" but for an "always" such that

if a, err := f(b); err != nil {
  ...
} else if b, ok := bar(a); ok {
  ...
} always {
  a.foo()
} else if c, err := a.bar(); err !=nil {
  ...
} else {
  return c
}

would also be valid.

this allows us to "do some work" with "a" (which entered the scope through a conditional assignment) before proceeding with the subsequent branches of the conditional and without introducing a nested conditional scope in an "else" block.

@jkassis
Copy link
Author

jkassis commented Aug 22, 2024

fwiw, i'm not married to "always". "next" or "do". "and" makes sense, but conflates with the boolean operator. perhaps "with", which is pythonic i suppose.

@ianlancetaylor
Copy link
Contributor

@jkassis Thanks, but that doesn't answer my question.

My question is: when does the always clause execute?

Your original post says that this

if a, err := f(b); err != nil {
  ...
} else {
  if b, ok := bar(a); ok {
    ...
  } 
  a.foo()
}

is equivalent to this:

if a, err := f(b); err != nil {
  ...
} else if b, ok := bar(a); ok {
  ...
} always {
  a.foo()
}

The first code sequence calls a.foo() if f(b) returns a nil error. It calls a.foo whatever bar(a) returns.

If the second code sequence is equivalent, then the always clause is not executed if the first condition (err != nil) is true, and the always clause is executed if the second condition (ok) is true.

So when precisely is the always clause executed?

@jkassis
Copy link
Author

jkassis commented Aug 23, 2024

@ianlancetaylor ... oh my goodness. i see your point and i did make a mistake in my example. please forgive. this example is to the point...

if a, err := f(b); err != nil {
  ...
} else {
  a.foo()
  if b, ok := bar(a); ok {
    ...
  } 
}

would be...

if a, err := f(b); err != nil {
  ...
} with {
  a.foo()
} else if b, ok := bar(a); ok {
    ...
}

@jkassis
Copy link
Author

jkassis commented Aug 23, 2024

noting that i changed the "always" keyword to "with" as per the evolving conversation.

@zwell
Copy link

zwell commented Aug 26, 2024

Can there be multiple "with" keywords? Or does it depend on where it is located?

@jkassis
Copy link
Author

jkassis commented Aug 26, 2024

definitely, though a with after a with doesn't make much sense.

@jfkfrb
Copy link

jfkfrb commented Aug 26, 2024

here's a practical example...

	// start the client pool
	var clientPool *ClientPool
	if poolID, err := GetPodIdx(s.Name); err != nil {
		s.ErrorCheck(http.StatusInternalServerError, fmt.Errorf("could not get poolID from podName: %v", err))
		return
	} else if clientPool = ClientPoolMake(s.Ctx, netConf, 1024, poolID); false {
	} else if err := clientPool.Start(); err != nil {
		s.ErrorCheck(http.StatusInternalServerError, fmt.Errorf("could not start client pool: %v", err))
		return
	}

becomes...

	// start the client pool
	var clientPool *ClientPool
	if poolID, err := GetPodIdx(s.Name); err != nil {
		s.ErrorCheck(http.StatusInternalServerError, fmt.Errorf("could not get poolID from podName: %v", err))
		return
	} with {
	        clientPool = ClientPoolMake(s.Ctx, netConf, 1024, poolID)
	} else if err := clientPool.Start(); err != nil {
		s.ErrorCheck(http.StatusInternalServerError, fmt.Errorf("could not start client pool: %v", err))
		return
	} 

@jkassis
Copy link
Author

jkassis commented Aug 27, 2024

obviously, you can also put more than one statement in the with clause, which is an extra advantage.

@magical
Copy link
Contributor

magical commented Aug 27, 2024

You can put more than one statement in an else if clause too, sort of...

	if a, b := 1, 3; a == b {
		panic("error")
	} else if func() bool {
		a++
		b--
		return false
	}() {
	} else if a == b {
		print("ok")
	}

@jkassis
Copy link
Author

jkassis commented Aug 27, 2024

that's true, but less readable imo.

@ianlancetaylor
Copy link
Contributor

This is syntactic sugar for a nested else clause. The emoji voting is not in favor. Therefore, this is a likely decline. Leaving open for four weeks for final comments.

@jkassis
Copy link
Author

jkassis commented Sep 7, 2024

How much emoji is required?

with{...} vs else if func () bool {... ; return false }(){} gives me butterfly-mojis!

As a former Google Borg-man, I'm sure others would feel the same.

@jkassis
Copy link
Author

jkassis commented Sep 7, 2024

@rebeccajae can you give me some ❤️

@jkassis
Copy link
Author

jkassis commented Sep 7, 2024

@adamrogoyski can you give me some ❤️ too?

@ianlancetaylor
Copy link
Contributor

How much emoji is required?

There is no specific requirement, but this proposal is all thumbs-down and no thumbs-up.

By way of comparison, in Go 1.21 we had a fairly minor change to the language: adding a clear builtin function. That proposal, #56351, had 122 thumbs up and 4 thumbs down.

@jkassis jkassis changed the title proposal: spec: always branch for conditionals spec: add "with" conditional branch Sep 8, 2024
@jkassis
Copy link
Author

jkassis commented Sep 8, 2024

wow. so many thumbs down. i suspect because the original example was in error. i updated the proposal title and content, but suspect i won't get the votes. can you please point me to the relevant package. i'll provide the PR.

@jkassis
Copy link
Author

jkassis commented Sep 8, 2024

was looking through the "clear" proposal... how would i locate the PR for code associated with that?

@gophun
Copy link

gophun commented Sep 8, 2024

@jkassis This is getting off-topic, but you can search for the gopherbot comments saying "Change https://go.dev/cl/... mentions this issue". In this case the main change is: https://go-review.googlesource.com/c/go/+/453395

@ianlancetaylor
Copy link
Contributor

This proposal is syntactic sugar, so several places need changing. The compiler parser in cmd/compile/internal/syntax. The internal IR may need changing in cmd/compile/internal/ir, I'm not sure. Something will need to translate the new syntax into existing constructs somewhere. Then there is separate code that needs changing in go/parser, go/ast, go/print, and probably go/types.

Note that writing a pull request for this proposal is not going to make us any more likely to accept it. We are currently path to declining this proposal.

@ianlancetaylor
Copy link
Contributor

No change in consensus.

@ianlancetaylor ianlancetaylor closed this as not planned Won't fix, can't repro, duplicate, stale Oct 2, 2024
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 LanguageChangeReview Discussed by language change review committee Proposal Proposal-FinalCommentPeriod
Projects
None yet
Development

No branches or pull requests