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

proposal: nil checks by default #21769

Closed
Jesusgeek opened this issue Sep 5, 2017 · 9 comments
Closed

proposal: nil checks by default #21769

Jesusgeek opened this issue Sep 5, 2017 · 9 comments
Labels
FrozenDueToAge LanguageChange Suggested changes to the Go language Proposal v2 An incompatible library change
Milestone

Comments

@Jesusgeek
Copy link

I propose to evaluate to nil checks automatically when a non-bool variable is used, and no comparison operatores with it, e.g.:

err := Something()
if err {
    // Handle Error
}

Currently

Right now it defaults to a "non-bool err (type %T) used as if condition" compile error.

Reason

Correct me if I am wrong, but it seems easier to read/understand than the usual if err != nil, especially considering the amount it is being used. An option to leave the nil-check out, seems very appealing to me.

@gopherbot gopherbot added this to the Proposal milestone Sep 5, 2017
@Jesusgeek Jesusgeek changed the title proposal: nil checks as default, when none-boolean proposal: nil checks as default (when non-bool type) Sep 5, 2017
@dominikh
Copy link
Member

dominikh commented Sep 5, 2017

This goes against two of Go's strong points.

  1. no implicit type conversions. if checks booleans, and nil isn't implicitly converted to a bool. You could argue that this could be made syntactic sugar instead, saying that if can operate both on booleans, and nilable values. But that raises more concerns: should bool(nilable) be valid code? Should for receive the same capability?

  2. Aside from error checking, nil isn't a very common value in Go. Functions that return nil on failure also return an error or a boolean. In rarer scenarios, where nil is a common value, it usually doesn't require special treatment.

    That's in part why nil pointer dereferences are a lot less common in Go, compared to say Java or Ruby. We don't pass nils around for multiple stack frames to blow up eventually.

Your example focuses on if err != nil, which suggests to me that the real intention is to slightly reduce the amount of typing required for error checking. IMO that is not worth the added complexity and other downsides, and we should focus on other proposals that are trying to improve error handling.

@Jesusgeek
Copy link
Author

Thanks, but my main intend is readability most of all, I really don't mind the typing.
The error checking pattern is probably the most used pattern in go, that's why I picked it.

But in general following seems rather logical/natural/easy to read ...

if something {
    // do something with something
}

... whereas I am seeing a lot of double negation (is some error not nothing).
Really readability more than anything!

@Jesusgeek Jesusgeek changed the title proposal: nil checks as default (when non-bool type) proposal: nil checks by default Sep 5, 2017
@robpike
Copy link
Contributor

robpike commented Sep 5, 2017

This has come up before and a related situation in C and C++, where a pointer can be a boolean, is notorious as the cause of many bad bugs, such as

bool flag = "false"

Even if you restricted the idea to interfaces and not also pointers, which would be inconsistent, I'm sure this proposal would introduce similar problems, especially during refactoring.

I am strongly disposed against this proposal.

@dsnet dsnet added LanguageChange Suggested changes to the Go language v2 An incompatible library change labels Sep 6, 2017
@Jesusgeek
Copy link
Author

I find this a rather strong argument, it can really leads to bugs in languages like C/C++, PHP or Python.

One possible solution

Literal use of special non-bool variables like "false", "true", "nil", "0" and possibly 0, for boolean evaluation, could simply raise a warning/error. (Someone using them like this should be spanked anyways)

var someFlag bool
var someText string

someText = "false" // ok
someFlag = "false" // warning/error

if "false"  { // warning/error
if someText { // seems pretty ok/understandable
if someFlag { // seems pretty ok/hard to misuse

func getSomeText() (string) {
    return "false" // ok, since non-bool expected
}
if getSomeText() { // evaluates to true, since text was returned, seems ok/clear, not?

But short variable declaration, might not be as dummy-friendly, don't know how one could tackle e.g.

someFlag := "false"

@OneOfOne
Copy link
Contributor

OneOfOne commented Sep 6, 2017

Please don't.

@bcmills
Copy link
Contributor

bcmills commented Sep 12, 2017

The most subtle version I've seen of the bug @robpike describes looks like this:

var enabled = flag.Bool("enabled", false, "if true, break everything")

…  // Some distance later...

if enabled {  // seems pretty ok/hard to misuse[!?]
	experiment.breakEverything()
}

The bug is the failure to dereference the enabled variable, which is a *bool: the condition should correctly be if *enabled, but it's easy to miss the forgotten * because the expression would be correct for a variable of type bool.

@Jesusgeek
Copy link
Author

Jesusgeek commented Sep 12, 2017

As the very first sentence of this proposal states, I would only allow this for non-bool variables/pointers.

In my example someFlag is a bool, and it's check is not for a nil type, but true/false, which is ok.

var someFlag bool
...
if someFlag { // hard to misuse

In your example the check is of type *bool which would be ...

  1. not eligible for nil checking
  2. a non-bool, so it would throw the same error as always

As mentioned above I would simply not allow it for bool, *bool, etc., why should you?
It is a feature/sugar, so it can throw an error for certain types, and no one has to use it.

But within the scope I provided (1. no (*(*(*)))bool types, 2. not enabled for direct/literal use of special dummy values like "true", "false", etc.), it seems pretty safe.

Please prove me wrong!

@cznic
Copy link
Contributor

cznic commented Sep 12, 2017

As the very first sentence of this proposal states, I would only allow this for non-bool variables/pointers.

Actually, quoting: "I propose to evaluate to nil checks automatically when a non-bool variable is used, and no comparison operatores with it, e.g.:"

Pointer types are non-bool types. So given opt := flag.Bool(...), both if *opt and if opt is valid under the proposal.

What was probably meant is to exclude types having underlying type bool and pointers to such types. Still, I see no improvement of the language by adopting the proposal. Quite the opposite.

@ianlancetaylor
Copy link
Contributor

There are several counter-arguments above. Proposal declined.

@golang golang locked and limited conversation to collaborators Mar 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge LanguageChange Suggested changes to the Go language Proposal v2 An incompatible library change
Projects
None yet
Development

No branches or pull requests

9 participants