[go-nuts] shadowing - a proposal to improve visibility / usage analysis

On 19.02.2013 08:28, Jan Mercl wrote:
- Short variable declaration is not related to "duck" typing.
This false wording is irrelevant to the proposal
- The proposal would make sane code illegal. The scenario where a zero
value is only conditionally altered before being returned/passed is
common.

- Breaks the Go 1 compatibility rules.
Proposal is strictly for Go 2
- j

On 2013/2/19 Johann Höchtl wrote:
Bumping, as I consider it a sane proposal

On 02/19/2013 09:39 PM, Rémy Oudompheng wrote:
It seems ridiculously strict to forbid people from writing:

result := Function()
DoSomethingWith(result)

It looks like perfectly valid code and I wouldn't want to litter it
with "_ := &result".
You should rather use a govet-like tool to detect shadowing and filter
out those you don't like.

On Tue, Feb 19, 2013 at 9:50 PM, Johann Höchtl wrote:

Sure, but in this case no shadowing happens. But take a look at this case:


var found bool
for idx := range []int{1, 3, 5} {
if found := answertoeverything(idx); found == true {
answer = "We got an answer"
break
}
}

if found {
fmt.Println("The answer is", answer)
} else {
fmt.Println("No answer was found")
}

The outer found get's never explicitly assigned to, yet it's legal and
sane code because of the default initialization promise. However, would the
compiler bark, the shadowing would become obvious.

On 02/19/2013 10:03 PM, Jan Mercl wrote:
On Tue, Feb 19, 2013 at 9:50 PM, Johann Höchtl wrote:

Sure, but in this case no shadowing happens. But take a look at this
case:


var found bool
for idx := range []int{1, 3, 5} {
if found := answertoeverything(idx); found == true {
answer = "We got an answer"
break
}
}

if found {
fmt.Println("The answer is", answer)
} else {
fmt.Println("No answer was found")
}

The outer found get's never explicitly assigned to, yet it's legal
and sane code because of the default initialization promise.
However, would the compiler bark, the shadowing would become obvious.


This is how block scoping must work for it to be useful. Imagine for
example code getting moved around. There's no reason for the compiler to
say anything about it.

On Wed, Feb 20, 2013 at 5:07 AM, Johann Höchtl wrote:
Ok, but when you remove the block, code like this is very likely to be
erroneous:

var i int

if i != 0 {
} else {
}

Of course, Go promises to be well defined but it's still likely to make no
sense as i get's never explicitly assigned to

On Tue, Feb 19, 2013 at 1:11 PM, minux wrote:


the behavior of your example really is well defined, and it might be the
programmer's intention,
or not, but in general (like the other thread about giving error for const
if conditions), the compiler
is impossible to know the real intent at this stage (and when they do,
maybe we don't even need to write it down).

--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On Tue, Feb 19, 2013 at 10:22 PM, Michael Jones wrote:

How about a "warn about shadowed variables" feature of a tool like vet?

What else could it do?

+ look for idiomatic ways to do things (go fmt can turn s[0:k] into s[:k]
for example)
+ warn about shadowed variables/return args
+ maybe whine about using fmt.Printf with fixed strings (silly waste of
time): Printf -> Print
:



--
Michael T. Jones | Chief Technology Advocate | mtj@google.com | +1
650-335-5765

--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

+1, this has come up many times.

On Tue, Feb 19, 2013 at 10:22 PM, Michael Jones wrote:

How about a "warn about shadowed variables" feature of a tool like vet?

What else could it do?

+ look for idiomatic ways to do things (go fmt can turn s[0:k] into
s[:k] for example)
+ warn about shadowed variables/return args
+ maybe whine about using fmt.Printf with fixed strings (silly waste
of time): Printf -> Print
:


On Tue, Feb 19, 2013 at 1:11 PM, minux wrote:



On Wed, Feb 20, 2013 at 5:07 AM, Johann Höchtl
wrote:

On 02/19/2013 10:03 PM, Jan Mercl wrote:

This is how block scoping must work for it to be useful.
Imagine for
example code getting moved around. There's no reason for
the compiler to
say anything about it.

Ok, but when you remove the block, code like this is very
likely to be erroneous:

var i int

if i != 0 {
} else {
}

Of course, Go promises to be well defined but it's still
likely to make no sense as i get's never explicitly assigned to

the behavior of your example really is well defined, and it
might be the programmer's intention,
or not, but in general (like the other thread about giving error
for const if conditions), the compiler
is impossible to know the real intent at this stage (and when
they do, maybe we don't even need to write it down).

--
You received this message because you are subscribed to the
Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from
it, send an email to golang-nuts+unsubscribe@googlegroups.com
For more options, visit https://groups.google.com/groups/opt_out.





--
Michael T. Jones | Chief Technology Advocate |mtj@google.com
--
You received this message because you are subscribed to the Google
Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it,
send an email to golang-nuts+unsubscribe@googlegroups.com
For more options, visit https://groups.google.com/groups/opt_out.

On 2013/2/19 Michael Jones wrote:
+ maybe whine about using fmt.Printf with fixed strings (silly waste of
time): Printf -> Print

It's not a waste of time. fmt.Print(string) produces an unecessary
extra memory allocation and reflection.
fmt.Printf(string) should be much more efficient.

On Tuesday, February 19, 2013 12:50:22 PM UTC-8, Johann Höchtl wrote:
Sure, but in this case no shadowing happens. But take a look at this case:

var found bool
for idx := range []int{1, 3, 5} {
if found := answertoeverything(idx); found == true {
answer = "We got an answer"
break
}
}

if found {
fmt.Println("The answer is", answer)
} else {
fmt.Println("No answer was found")
}

The outer found get's never explicitly assigned to, yet it's legal and
sane code because of the default initialization promise. However, would
the compiler bark, the shadowing would become obvious.

On 02/19/2013 10:31 PM, Kamil Kisiel wrote:
On Tuesday, February 19, 2013 12:50:22 PM UTC-8, Johann Höchtl wrote:
wrote:
Sure, but in this case no shadowing happens. But take a look at this
case:

var found bool
for idx := range []int{1, 3, 5} {
if found := answertoeverything(idx); found == true {
answer = "We got an answer"
break
}
}

if found {
fmt.Println("The answer is", answer)
} else {
fmt.Println("No answer was found")
}

The outer found get's never explicitly assigned to, yet it's legal and
sane code because of the default initialization promise. However, would
the compiler bark, the shadowing would become obvious.


But that's exactly the way I would want, and expect it to work. I don't
see why the compiler should produce an error here. Sure it's possible to
do it by accident, but there are many cases where you can make
intentional use of such constructs, such as limiting the scope of an err
variable.

--
You received this message because you are subscribed to the Google
Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send
an email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On Tuesday, February 19, 2013 1:34:04 PM UTC-8, Johann Höchtl wrote:
it
The compiler shouldn't give an error about the inner found, as a value
gets assigned to, but about the outer, which gets declared but is never
explicitly assigned to.

On 02/19/2013 10:40 PM, Kamil Kisiel wrote:
On Tuesday, February 19, 2013 1:34:04 PM UTC-8, Johann Höchtl wrote:
wrote:
<javascript:>>
litter it
shadowing and
The compiler shouldn't give an error about the inner found, as a value
gets assigned to, but about the outer, which gets declared but is never
explicitly assigned to.


I don't see what's wrong with that, the behaviour is explicitly defined
in the spec, the variable is initialized to the zero
value: http://golang.org/ref/spec#Program_initialization_and_execution

var found bool

is simply equivalent to

var found bool = false

--
You received this message because you are subscribed to the Google
Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send
an email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On 02/19/2013 09:34 PM, Johann Höchtl wrote:
The compiler shouldn't give an error about the inner found, as a value
gets assigned to, but about the outer, which gets declared but is never
explicitly assigned to.

On Tue, Feb 19, 2013 at 3:20 PM, Johann Höchtl wrote:

quick assignemnts, := , are meant to help the programmer and give the
language the feeling being a duck-typed, dynamic language yet with the
powers of static type checking. Unfortunately this feature comes at the
price of introducing nasty, hard to spot shadowing bugs.

Consider this program:

http://play.golang.org/p/nroI2nSKyG

package main

import "fmt"

func answertoeverything(idx int) bool {
return true
}

func main() {

var answer string
var found bool
for idx := range []int{1, 3, 5} {
if found := answertoeverything(idx); found == true {
answer = "We got an answer"
break
}
}

if found {
fmt.Println("The answer is", answer)
} else {
fmt.Println("No answer was found")
}
}

The variable "found" is hidden. This is clearly a mistake yet get's
unspotted by the current compiler. This is either a plain bug, or, if not,
I suppose to change the behaviour in that simply accessing a variable is
not enough to silence the compiler, but either assigning to it or taking
it'd address.

_ := silence

is seen very often and suffices to calm the compiler. I would suggest,
that only a proper assignment can silence the "introduced but not used"
error, or taking the address of such a variable as eg. in

callfunc(&notused)

Thus

_ := silence
would become
_ := &silence

If this variable get's actually used at a a later point, it is most
likely that it will not be used in a pointer context, in such as the
programmer can still benefit from the compilers analysis.

On 02/19/2013 10:20 PM, minux wrote:

On Wed, Feb 20, 2013 at 5:14 AM, Johann Höchtl wrote:

On 02/19/2013 10:06 PM, minux wrote:


On Tue, Feb 19, 2013 at 3:20 PM, Johann Höchtl
wrote:

quick assignemnts, := , are meant to help the programmer
and give
the language the feeling being a duck-typed, dynamic
language yet
with the powers of static type checking. Unfortunately this
feature
comes at the price of introducing nasty, hard to spot
shadowing bugs.

Consider this program:

http://play.golang.org/p/__nroI2nSKyG
<http://play.golang.org/p/nroI2nSKyG>

package main

import "fmt"

func answertoeverything(idx int) bool {
return true
}

func main() {

var answer string
var found bool
for idx := range []int{1, 3, 5} {
if found := answertoeverything(idx); found == true {
answer = "We got an answer"
break
}
}

if found {
fmt.Println("The answer is", answer)
} else {
fmt.Println("No answer was found")
}
}

The variable "found" is hidden. This is clearly a mistake
yet get's
unspotted by the current compiler. This is either a plain
bug, or,
if not, I suppose to change the behaviour in that simply
accessing a
variable is not enough to silence the compiler, but either
assigning
to it or taking it'd address.

_ := silence

is seen very often and suffices to calm the compiler. I would
suggest, that only a proper assignment can silence the
"introduced
but not used" error, or taking the address of such a
variable as eg. in

callfunc(&notused)

Thus

_ := silence
would become
_ := &silence

If this variable get's actually used at a a later point,
it is most
likely that it will not be used in a pointer context, in
such as the
programmer can still benefit from the compilers analysis.

suppose we adopt this change, please write the update to the
spec page and
explain it as simple and as clear as possible.

The current spec is very precise, I wholeheartedly admit I am not
adept enough for that.

in what cases does the compiler give an error for shadowing? do
you need to
add some exceptions to your rules?


It would / should give an error if an "outer" variable is declared
but never assigned a value to it (but the default value) and
shadowed by an "inner" one.

Let's see this example:
found := true
for idx := range []int{1, 3, 5} {
if found := answertoeverything(idx); found == true {
answer = "We got an answer"
break
}
}

if found {
fmt.Println("The answer is", answer)
} else {
fmt.Println("No answer was found")
}

found is indeed assigned a value, however, this program is not correct but
according to your rule, the compiler won't detect it.
if you change := to = in the inner if clause, the program will work as
intended.

In case the address of the outer variable is somewhere taken, my
guess is that flow analysis would become to difficult (whole program
flow optimization), in which the compiler wouldn't report an error.

On 02/19/2013 07:20 AM, Johann Höchtl wrote:
quick assignemnts, := , are meant to help the programmer and give the
language the feeling being a duck-typed, dynamic language yet with the
powers of static type checking. Unfortunately this feature comes at the
price of introducing nasty, hard to spot shadowing bugs.

On 02/19/2013 10:49 PM, Rui Maciel wrote:
Go's short variable declaration doesn't introduce bugs. What tends to
introduce bugs is people using short variable declarations expecting it
to do an assignment instead of actually declaring a variable. The
example you've presented below is one of those cases.

There is a simpler and better solution: use a short variable declaration
when you actually want to declare a variable, and use an assignment
operator when all you want to do is assign a value to a variable which
you've previously declared. This doesn't require any change to either
the language or the compiler, particularly one which is that cryptic.


Rui Maciel

On Tue, Feb 19, 2013 at 11:03 PM, Johann Höchtl wrote:

Still ... everything happens exactly the way it is told to behave. Which
still makes the end results not a desirable result.

On 02/19/2013 11:23 PM, Jan Mercl wrote:
On Tue, Feb 19, 2013 at 11:03 PM, Johann Höchtl
wrote:



Still ... everything happens exactly the way it is told to behave.
Which still makes the end results not a desirable result.


The compiler has no way to understand what is the 'desired' result in
this case. There are formally/structurally equivalent sane cases where
the desired result is the one obtained. This rules out the possibility
to make it an compiler error. Then there are cases, where a programmer
makes a mistake which misses the desired result. The compiler can never
ever discriminate the two situations and thus it is not rational to ask
it for doing that.

On 02/19/2013 10:03 PM, Johann Höchtl wrote:
See it this way. I can carry a gun in my hand aiming towards a target. I
pull the trigger and hit the target. Everything happens exactly the whay
it is expected to happen.

Suddenly an inner block jumps in ... the instructor. Me, a gun in my
hand, the instructor in between and on the other side the target. I pull
the trigger.

Still ... everything happens exactly the way it is told to behave. Which
still makes the end results not a desirable result. Adding an "inner
block", which by itself is behaving in a fully specified way, influences
the whole.

Somewhat odd I admit, but you may get what I mean?

On Wed, Feb 20, 2013 at 8:40 AM, Jay Weisskopf wrote:

I wasted a lot of time on a bug today that was caused by shadowing (= was
mistakenly :=). I've seen some people mentioning the "go vet" command, but
I can't find much information on it. It warns about shadowed variables, or
was someone just throwing that out as an idea?

On 20.02.2013 08:53, Jan Mercl wrote:
On Wed, Feb 20, 2013 at 8:40 AM, Jay Weisskopf wrote:

I wasted a lot of time on a bug today that was caused by shadowing
(= was mistakenly :=). I've seen some people mentioning the "go
vet" command, but I can't find much information on it. It warns
about shadowed variables, or was someone just throwing that out as
an idea?


AFAIK, it does not warn about this. IMO it should not warn about this.
I use some "scripts" which demand `$ go vet` says nothing in the case
of valid code (cca a pre commit hook). It would be sad if it will
start to rant about code w/o any troubles. I think I have some
(working) code which would not pass.

-j

PS:

$ go vet -h
or
$ go help vet

->

$ godoc vet

-j

--
You received this message because you are subscribed to the Google
Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send
an email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On Wednesday, 20 February 2013 08:53:44 UTC+1, Jan Mercl wrote:

On Wed, Feb 20, 2013 at 8:40 AM, Jay Weisskopf <jays...@gmail.com<javascript:>

AFAIK, it does not warn about this. IMO it should not warn about this. I
use some "scripts" which demand `$ go vet` says nothing in the case of
valid code (cca a pre commit hook). It would be sad if it will start to
rant about code w/o any troubles. I think I have some (working) code which
would not pass.

-j

PS:

$ go vet -h
or
$ go help vet

->

$ godoc vet

-j

On Wed, Feb 20, 2013 at 2:17 AM, Jan Mercl wrote:

Yes, that's maybe a good approach.

-j

--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On Feb 20, 2013 8:18 AM, "Michael Jones" wrote:

that was my intention...


--
Michael T. Jones | Chief Technology Advocate | mtj@google.com | +1
650-335-5765

--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On Thu, Feb 21, 2013 at 7:22 AM, George Shammas wrote:
The flag should be the other way around.

Most people who complain about shadowing are either new to the language or
don't understand block scoping. So why make it a flag to enable the check
when they wouldn't even to understand what the flag does without researching
it.

On Thu, Feb 21, 2013 at 4:37 PM, Ian Lance Taylor wrote:
Any such flag effectively creates a new dialect of the language. For
example, there are multiple dialects of C, including the language
described by the standard and the language accepted by GCC with the
-Wall -Werror options. I think we would want very compelling reasons
to split Go into multiple dialects at this point.

I agree that short declarations using :=, redeclarations using :=,
block scoping, and for and if statements introducing a new block scope
can in combination lead to situations that can be confusing. There
may be some way to improve matters. But I don't think that adding a
compiler flag is it.

Ian

--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On 21.02.2013 16:43, Patrick Mylund Nielsen wrote:
I agree about gc, but I think it would be reasonable to have 'go vet'
warn about this by default. If you ARE shadowing intentionally, a
warning from vet isn't going to alarm you.

On Thu, Feb 21, 2013 at 4:37 PM, Ian Lance Taylor wrote:
Any such flag effectively creates a new dialect of the language. For
example, there are multiple dialects of C, including the language
described by the standard and the language accepted by GCC with the
-Wall -Werror options. I think we would want very compelling reasons
to split Go into multiple dialects at this point.

I agree that short declarations using :=, redeclarations using :=,
block scoping, and for and if statements introducing a new block scope
can in combination lead to situations that can be confusing. There
may be some way to improve matters. But I don't think that adding a
compiler flag is it.

Ian

--
You received this message because you are subscribed to the Google
Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it,
send an email to golang-nuts+unsubscribe@googlegroups.com
For more options, visit https://groups.google.com/groups/opt_out.



--
You received this message because you are subscribed to the Google
Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send
an email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On Fri, Feb 22, 2013 at 3:51 PM, Johann Höchtl wrote:

On 21.02.2013 16:43, Patrick Mylund Nielsen wrote:

I agree about gc, but I think it would be reasonable to have 'go vet' warn
about this by default. If you ARE shadowing intentionally, a warning from
vet isn't going to alarm you.

I would be very much in favor of such a flag. It would be helpful (even
if only for beginners) and won't harm otherwise.

--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.




--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On Thursday, 21 February 2013 16:37:30 UTC+1, Ian Lance Taylor wrote:
Any such flag effectively creates a new dialect of the language.

On 2/20/2013 2:04 AM, Job van der Zwan wrote:
What about an optional flag to check for shadowing?

On 02/20/2013 09:14 PM, John Nagle wrote:
No, either it should be allowed, or it should not be allowed.
Making it optional splits the language.

On 02/19/2013 08:20 AM, Johann Höchtl wrote:

[...] The variable "found" is hidden. This is clearly a mistake yet get's
unspotted by the current compiler. [...]

On 2/20/2013 8:43 AM, Norbert Roos wrote:
I would propose another radical solution for this: Simply that shadowing
variables is not allowed. When you are about to shadow a variable, use a
different variable name.

On Wed, Feb 20, 2013 at 5:43 PM, Norbert Roos wrote:

I would propose another radical solution for this: Simply that shadowing
variables is not allowed.
Good bye block scoping.
When you are about to shadow a variable, use a
different variable name.

I mean this really serious; compared to C, Go made some useful restrictions,
limiting the programmer's freedom in some way, like the mandatory use of { }
in all if's anf for's. Forbidding variable shadowing could be one more such
thing. I could live with that restriction very well.

Good bye block scoping.

On Wed, Feb 20, 2013 at 10:21 PM, Jan Mercl wrote:
get's
I cannot recall a single one of the imperative mainstream languages
which doesn't have block scoping. Not an argument, of course. Just
makes one wonder why it is like that.

-j

--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Block scoping does not disappear if you can't declare new variables with
names identical to variables in outer scopes.

On Thu, Feb 21, 2013 at 2:58 PM, Jan Mercl wrote:

On Thu, Feb 21, 2013 at 2:38 PM, Patrick Mylund Nielsen
wrote:
shadowing
package main
var a int
func main() {
var a int // main.go:4: a redeclared in this block previous
declaration at main.go:2

...
}

Well, is it just me who doesn't call the above 'block scoping' anymore?

-j

On Thu, Feb 21, 2013 at 3:09 PM, Patrick Mylund Nielsen wrote:
A "var oa int" would still be in block scope.

On Thu, Feb 21, 2013 at 6:34 AM, Jan Mercl wrote:

On Thu, Feb 21, 2013 at 3:09 PM, Patrick Mylund Nielsen
wrote:
I understand that variables still have scope. But with an effectively
flat "namespace", it's only one half left of the 'block scoping'.
Which leads to some funny things like: In a valid and correct program
using this "half scoping", one can move every single variable declared
to become a TLD and _nothing happens_ to the validity, correctness and
behavior of the program. Pretty BASIC like it is, if one imagines
that, right? But I cannot call BASIC a block scoped language ;-)

-j

PS: I've ignored some minor details about e.g. file vs package scope.

--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On Feb 21, 2013 8:52 PM, "Thomas Bushnell, BSG" wrote:
That's not actually true, because presumably the shadowing opponents

On Thu, Feb 21, 2013 at 9:18 PM, Jan Mercl wrote:

still want recursion to work.

Oh, you're right of course, I forgot to consider recursive calls. Even
worse, I failed to consider a variable declared inside a loop too.

-j