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.

Sign up for GitHub

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

Jump to bottom

proposal: Go 2: reject (some) redundant expressions #27775

Closed
krhubert opened this issue Sep 20, 2018 · 6 comments
Closed

proposal: Go 2: reject (some) redundant expressions #27775

krhubert opened this issue Sep 20, 2018 · 6 comments
Labels
FrozenDueToAge LanguageChange Proposal v2 A language change or incompatible library change WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Proposal

Comments

@krhubert
Copy link

krhubert commented Sep 20, 2018 •
edited

Hi,

I love golang for simplicity and explicitness. But there are a few cases when a developer can write the same code in different ways. Why not restrict syntax (in Go2) only to one possible option.

package main

func main() {
	// compile error - negative buffer argument in make(chan error)
	_ = make(chan error, -1)
	// compile error - negative len argument in make([]error)
	_ = make([]error, -1)

	// Base on the above errors makes the language even more strict about syntax.

	// compile without error
	// should be an error -> zero len argument in make(chan error)
	_ = make(chan error, 0)

	// compile error -> missing len argument to make([]error)
	// should be no error here -> it's same as make([]error, 0) now.
        // NOTE: if we can make chan and map without giving the size why it's forbiden here?
	_ = make([]error)

        // compile without error
        // should be an error -> zero len argument in make([]error, 0)
        _ = make([]error, 0)

	// compile without error
	// should be an error -> zero cap argument in make([]error, 0, 0)
	_ = make([]error, 0, 0)

   	// compile without error
	// should be an error -> zero size argument in  make(map[string]error)
        _ = make(map[string]error, 0)

	// compile without error
	// should be an error -> all constant comparisions like this have no meaning.
	_ = 1 == 2
        _ = 1 != 2.0
	_ = true == true
	_ = true != false
       
       // compile without error
       // should be an error -> all expression with 0 should be simplified
       i := 8
       _ = i >> 0
       _ = i << 0
       _ = i + 0
       _ = i - 0
       _ = i * 0

        
	// compile without error
	// should be an error -> don't use constant boolean in if statment (use !a/a instead)
	if a := true; a == true {
	}
}
@gopherbot gopherbot added this to the Proposal milestone Sep 20, 2018
@DeedleFake
Copy link 

// compile without error
// should be an error -> all constant comparisions like this have no meaning.
_ = 1 == 2
_ = 1 != 2.0
_ = true == true
_ = true != false

I can agree with this one. I've always wondered why the compiler allows things like true == false. At the very least go vet should probably warn about it, as it's likely to be a bug.

@magical
Copy link
Contributor

magical commented Sep 20, 2018 

  // compile without error
  // should be an error -> all expression with 0 should be simplified
  i := 8
  _ = i >> 0
  _ = i << 0
  _ = i + 0
  _ = i - 0

I disagree that these expressions should always be simplified. Stylistically, it can be helpful to include an explicit zero in some cases involving repetitive statements.

For example,

buf[i+0] = ...
buf[i+1] = ...
buf[i+2] = ...
...

@bcmills
Copy link
Contributor

bcmills commented Sep 22, 2018

I've always wondered why the compiler allows things like true == false.

  • Variations based on build constraints?
  • Generated code?
  • Manual debugging?

It's not always apparent which identifiers are constants, or which constants are constant across all architectures and tags.

@bcmills bcmills added LanguageChange v2 A language change or incompatible library change labels Sep 22, 2018
@bcmills bcmills changed the title proposal: make the go language more strict about some syntaxes proposal: Go 2: reject (some) redundant expressions Sep 22, 2018
@bcmills
Copy link
Contributor

bcmills commented Sep 22, 2018

As a proposal this needs more detail.

  • What is the motivation for this proposal?
    • What harm do these expressions cause today, and why is it worth the cost of churn for existing code?
  • What are the exact rules you propose for which expressions should be rejected?
    • How do those rules interact with build constraints, unsafe.Sizeof, and other variations in constants?
  • Can all existing code be fixed automatically?

@bcmills bcmills added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Sep 22, 2018
@seebs
Copy link
Contributor

seebs commented Sep 22, 2018

i don't understand why most of these "should be" errors. i would absolutely use make([]foo, 0) if i wanted to have a non-nil thing but did not necessarily need it to have any contents, because "this is not nil" is itself data about the state of a thing. I can write code which distinguishes between a 0-length slice and a nil slice, therefore it's useful. Same for maps; why should I have to specify a non-zero size for a map?

I would tend to allow things like true == false because while they're unlikely to be reasonable in hand-written code, they might serve a useful function in machine-generated code, and the compiler can presumably optimize them effectively.

@ianlancetaylor
Copy link
Contributor

Per comments above, we aren't going to do this.

@golang golang locked and limited conversation to collaborators Oct 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge LanguageChange Proposal v2 A language change or incompatible library change WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Proposal
Development

No branches or pull requests
7 participants
@seebs @magical @DeedleFake @krhubert @ianlancetaylor @bcmills @gopherbot