Value shadowing

Value shadowing

[David Allsopp]

[ There's been at least one recent(ish) discussion on this - [1] ]

Suppose I have this piece of code:

let foo xs =
  match xs with
    x::xs -> if x
             then xs (* Return the tail of the list *)
             else xs (* Return the entire list *)
  | []    -> raise Exit

Now, the comments show what's supposed to happen in this function but that's
obviously not how it will behave because the entire list [xs] on line 1 is
shadowed by the tail of the list [xs] on line 3 so in both cases the tail of
the list will be returned. The type checker can do nothing as both have type
[bool list].

What would general opinion be if OCaml were to have a warning in this
instance - "[xs] on line 3 shadows [xs] on line 1 with the same type"?

As noted in the thread below, I too find

let x = _
in
  let x = _
  in
    ...

a useful style and would be greatly irritated by a warning on all shadowing
- but I think that in most cases for me the type of each [x] is different.
I've been stung a couple of times recently by non-contrived versions of the
function [foo] (through careless coding, of course - but that's what
warnings and type-checkers are about otherwise we'd all be using BCPL!)

Thoughts?


David


[1]
http://caml.inria.fr/pub/ml-archives/caml-list/2007/02/f60c7ea8cc0ebdbf9d1d5
6f0623b45a2.en.html

Re: [Caml-list] Value shadowing

[Brighten Godfrey]

On Aug 13, 2008, at 1:54 AM, David Allsopp wrote:

> [ There's been at least one recent(ish) discussion on this - [1] ]
>
> Suppose I have this piece of code:
>
> let foo xs =
>   match xs with
>     x::xs -> if x
>              then xs (* Return the tail of the list *)
>              else xs (* Return the entire list *)
>   | []    -> raise Exit
>
> Now, the comments show what's supposed to happen in this function  
> but that's
> obviously not how it will behave because the entire list [xs] on  
> line 1 is
> shadowed by the tail of the list [xs] on line 3 so in both cases  
> the tail of
> the list will be returned. The type checker can do nothing as both  
> have type
> [bool list].
>
> What would general opinion be if OCaml were to have a warning in this
> instance - "[xs] on line 3 shadows [xs] on line 1 with the same type"?
>
> As noted in the thread below, I too find
>
> let x = _
> in
>   let x = _
>   in
>     ...
>
> a useful style and would be greatly irritated by a warning on all  
> shadowing
> - but I think that in most cases for me the type of each [x] is  
> different.

I also find that style useful.  Sometimes the type changes, but I can  
recall useful cases where the type doesn't change, e.g., a sequence  
of various operations on a list.  Mock-up:

     let lst = generate_list_somehow () in
     let lst = List.filter (fun x -> x > 0) in
     let lst = List.sort compare lst in
     ...

~Brighten 

RE: [Caml-list] Value shadowing

[David Allsopp]

> > a useful style and would be greatly irritated by a warning on all  
> > shadowing
> > - but I think that in most cases for me the type of each [x] is  
> > different.
>
> I also find that style useful.  Sometimes the type changes, but I can  
> recall useful cases where the type doesn't change, e.g., a sequence  
> of various operations on a list.  Mock-up:
>
>      let lst = generate_list_somehow () in
>      let lst = List.filter (fun x -> x > 0) in
>      let lst = List.sort compare lst in
>      ...

Although in this case you can use a function composition operator instead -
so

let lst = [5; 4; 3; 2; 1; 0; -1; -2; -3; -4; -5]
in
  let lst = List.filter (fun x -> x > 0) lst
  in
    let lst = List.map (fun x -> -2 * x) lst
    in
      let lst = List.sort compare lst
      in
        lst

becomes [assuming let ($$) f g x = f (g x)]

let lst = [5; 4; 3; 2; 1; 0; -1; -2; -3; -4; -5]
in
  let filter = List.filter (fun x -> x > 0)
  in
    let double = List.map (fun x -> -2 * x)
    in
      let sort = List.sort compare
      in
        (sort $$ double $$ filter) lst




David

Re: [Caml-list] Value shadowing (tangent)

[Brighten Godfrey]

Going off on a tangent here...

On Aug 13, 2008, at 2:56 AM, David Allsopp wrote:
>
> let lst = [5; 4; 3; 2; 1; 0; -1; -2; -3; -4; -5]
> in
>   let filter = List.filter (fun x -> x > 0)
>   in
>     let double = List.map (fun x -> -2 * x)
>     in
>       let sort = List.sort compare
>       in
>         (sort $$ double $$ filter) lst

I've seen little of other people's OCaml code, so out of curiosity,  
do you or others actually write code formatted like the above, as  
opposed to the more compact and (I think) readable

     let lst = [5; 4; 3; 2; 1; 0; -1; -2; -3; -4; -5] in
     let filter = List.filter (fun x -> x > 0) in
     let double = List.map (fun x -> -2 * x) in
     let sort = List.sort compare in
     (sort $$ double $$ filter) lst

?

~Brighten

RE: [Caml-list] Value shadowing (tangent)

[David Allsopp]

> Going off on a tangent here...

Indeed :o)

> > let lst = [5; 4; 3; 2; 1; 0; -1; -2; -3; -4; -5]
> > in
> >   let filter = List.filter (fun x -> x > 0)
> >   in
> >     let double = List.map (fun x -> -2 * x)
> >     in
> >       let sort = List.sort compare
> >       in
> >         (sort $$ double $$ filter) lst
>
> I've seen little of other people's OCaml code, so out of curiosity,  
> do you or others actually write code formatted like the above, as  
> opposed to the more compact and (I think) readable
>
>      let lst = [5; 4; 3; 2; 1; 0; -1; -2; -3; -4; -5] in
>      let filter = List.filter (fun x -> x > 0) in
>      let double = List.map (fun x -> -2 * x) in
>      let sort = List.sort compare in
>      (sort $$ double $$ filter) lst

I similarly learned OCaml without reference to anyone else's code - I find
the indentation invaluable when reading the code for spotting the scope. I
would normally have written the above as

let lst = [5; 4; 3; 2; 1; 0; -1; -2; -3; -4; -5]
and filter = List.filter (fun x -> x > 0)
and double = List.map (fun x -> -2 * x)
and sort = List.sort compare
in
  (sort $$ double $$ filter) lst

Which is actually less to type than yours but I was adapting the previous
let lst = .. let lst = .. which is why I ended up with a very indented
version in my previous post.

I've not seen it anywhere else but then I haven't looked!

<irrelevance>
I don't know how the OCaml compiler actually implements displays, but the
"quick" approach means that using let .. and .. in instead of nested lets
will compile faster as name resolution will be faster :o)
</irrelevance>


David

Re: [Caml-list] Value shadowing (tangent)

[Brighten Godfrey]

On Aug 13, 2008, at 3:49 AM, Brighten Godfrey wrote:

> Going off on a tangent here...
>
> On Aug 13, 2008, at 2:56 AM, David Allsopp wrote:
>>
>> let lst = [5; 4; 3; 2; 1; 0; -1; -2; -3; -4; -5]
>> in
>>   let filter = List.filter (fun x -> x > 0)
>>   in
>>     let double = List.map (fun x -> -2 * x)
>>     in
>>       let sort = List.sort compare
>>       in
>>         (sort $$ double $$ filter) lst
>
> I've seen little of other people's OCaml code, so out of curiosity,  
> do you or others actually write code formatted like the above, as  
> opposed to the more compact and (I think) readable
>
>     let lst = [5; 4; 3; 2; 1; 0; -1; -2; -3; -4; -5] in
>     let filter = List.filter (fun x -> x > 0) in
>     let double = List.map (fun x -> -2 * x) in
>     let sort = List.sort compare in
>     (sort $$ double $$ filter) lst
>
> ?

P.S. Sorry David, on second read I sounded pretty elitist there!  I  
realize this is all subjective and my habits are no more valid than  
others'.  I am curious, though, what styles people have adopted.

Thanks,
~Brighten

Re: [Caml-list] Value shadowing (tangent)

[Daniel Bünzli]

Le 13 août 08 à 13:04, Brighten Godfrey a écrit :

> I realize this is all subjective and my habits are no more valid  
> than others'.  I am curious, though, what styles people have adopted.

Follow the guidelines http://caml.inria.fr/resources/doc/guides/guidelines.en.html#id2269166

Daniel

Re: [Caml-list] Value shadowing (tangent)

[Stefano Zacchiroli]

On Wed, Aug 13, 2008 at 01:17:17PM +0200, Daniel Bünzli wrote:
> Follow the guidelines http://caml.inria.fr/resources/doc/guides/guidelines.en.html#id2269166

Out of curiosity: does anybody knows for how long such a document has
been available? I was looking for something like that a while ago, and I
don't remind it's availability ...

Cheers.

-- 
Stefano Zacchiroli -*- PhD in Computer Science \ PostDoc @ Univ. Paris 7
zack@{upsilon.cc,pps.jussieu.fr,debian.org} -<>- http://upsilon.cc/zack/
I'm still an SGML person,this newfangled /\ All one has to do is hit the
XML stuff is so ... simplistic  -- Manoj \/ right keys at the right time

Re: [Caml-list] Value shadowing (tangent)

[Daniel Bünzli]

Le 14 août 08 à 01:05, Stefano Zacchiroli a écrit :

> Out of curiosity: does anybody knows for how long such a document has
> been available?

According to this [1] page found on the internet archive. 2 september  
1998 for the french version, 22 january 2000 for the english  
translation.

Best,

Daniel

[1] http://web.archive.org/web/20000529150849/http://caml.inria.fr/FAQ/pgl-eng.html

Re: [Caml-list] Value shadowing (tangent)

[Vincent Hanquez]

On Wed, Aug 13, 2008 at 03:49:23AM -0700, Brighten Godfrey wrote:
> Going off on a tangent here...
>
> On Aug 13, 2008, at 2:56 AM, David Allsopp wrote:
>>
>> let lst = [5; 4; 3; 2; 1; 0; -1; -2; -3; -4; -5]
>> in
>>   let filter = List.filter (fun x -> x > 0)
>>   in
>>     let double = List.map (fun x -> -2 * x)
>>     in
>>       let sort = List.sort compare
>>       in
>>         (sort $$ double $$ filter) lst
>
> I've seen little of other people's OCaml code, so out of curiosity, do 
> you or others actually write code formatted like the above, as opposed to 
> the more compact and (I think) readable
>
>     let lst = [5; 4; 3; 2; 1; 0; -1; -2; -3; -4; -5] in
>     let filter = List.filter (fun x -> x > 0) in
>     let double = List.map (fun x -> -2 * x) in
>     let sort = List.sort compare in
>     (sort $$ double $$ filter) lst

I write the compact way too, which i found much more readable.
I really dislike the wavy effect, of the former example, it has on code.

however i understand why some people do it the first way. after the "in"
you're in some sort of new scope (previous scope augmented by your
let binds)

-- 
Vincent

Re: [Caml-list] Value shadowing (tangent)

[Pierre Etchemaïté]

Le Wed, 13 Aug 2008 12:05:46 +0100, Vincent Hanquez <tab@snarc.org> a écrit :

> however i understand why some people do it the first way. after the "in"
> you're in some sort of new scope (previous scope augmented by your
> let binds)

On the other hand, the 'let' scope will end exactly at the same place as
the englobing scope. Since you can't close one without closing the
other, it doesn't make a lot of sense (both practically and mentally)
to distinguish them, thru indentation of otherwise.

I guess that's what 'justifies' the recommended indentation.

Best regards,
Pierre.

RE: [Caml-list] Value shadowing (tangent)

[David Allsopp]

> > however i understand why some people do it the first way. after the "in"
> > you're in some sort of new scope (previous scope augmented by your
> > let binds)
>
> On the other hand, the 'let' scope will end exactly at the same place as
> the englobing scope. Since you can't close one without closing the
> other, 

That's not true. 

let x =
  let y =
    let z = ()
    in
      ()
  in
    () (* z no longer in scope *)
and a = ()
in
  (* y and z no longer in scope *)


> it doesn't make a lot of sense (both practically and mentally) to
> distinguish them, thru indentation of otherwise.

Depends on whether you like to initialise non-trivial values with separate
one-off functions or with nested lets. Personally, I prefer the latter but
that's a matter of style/taste, not sense.


David

Re: [Caml-list] Value shadowing (tangent)

[Pierre Etchemaïté]

Le Sun, 17 Aug 2008 09:07:10 +0100, "David Allsopp" <dra-news@metastack.com> a écrit :

> > On the other hand, the 'let' scope will end exactly at the same place as
> > the englobing scope. Since you can't close one without closing the
> > other, 
> 
> That's not true. 
> 
> let x =
>   let y =
>     let z = ()
>     in
>       ()
>   in
>     () (* z no longer in scope *)
> and a = ()
> in
>   (* y and z no longer in scope *)

Now you're using lets within the _definition_ part of previous lets,
not within their scope. Even in the other style, that code would
require indentation like this:

let x =
  let y =
    let z = () in
    () in
  ()
and
  a = () in
(* ... *)

> Personally, I prefer the latter but
> that's a matter of style/taste, not sense.

I'm saying that "standard" indentation style makes sense, not that your
indentation style doesn't. Beware of xor-mode thinking :)

Best regards,
Pierre.

Re: [Caml-list] Value shadowing

[Jim Farrand]

On Wed, 13 Aug 2008 02:15:53 -0700
Brighten Godfrey <pbg@cs.berkeley.edu> wrote:

> I also find that style useful.  Sometimes the type changes, but I
> can recall useful cases where the type doesn't change, e.g., a
> sequence of various operations on a list.  Mock-up:
> 
>      let lst = generate_list_somehow () in
>      let lst = List.filter (fun x -> x > 0) in
>      let lst = List.sort compare lst in

Agreed.

I have a slightly different proposal than using return types: short
pragmas for switching off specific instances of a warning.

I always want as many warnings as possible switched on because I find
them really useful.  The problem is they are warnings for a reason:
They don't ALWAYS represent errors.

Once my code has a warning in it, the usefulness of warnings is greatly
reduced as I'm less likely to spot the addition of another warning in
the compiler output.

If I could disable specific instances of a warning, which I've looked
at and decided to ignore, I'd get the best of both worlds.

>      (* @disableshadowwarning *)
>      let lst = generate_list_somehow () in
>      let lst = List.filter (fun x -> x > 0) in
>      let lst = List.sort compare lst in
>      (* @enableshadowwarning *)

No doubt, someone can think of a better syntax

Regards,
Jim

Re: [Caml-list] Value shadowing

[Richard Jones]

On Wed, Aug 13, 2008 at 09:54:36AM +0100, David Allsopp wrote:
> Suppose I have this piece of code:
> 
> let foo xs =
>   match xs with
>     x::xs -> if x
>              then xs (* Return the tail of the list *)
>              else xs (* Return the entire list *)
>   | []    -> raise Exit

I'd find it very counter-intuitive if OCaml behaved like this, and
annoying if it gave a warning.  Just name the variables to be
different!

Rich.

-- 
Richard Jones
Red Hat

RE: [Caml-list] Value shadowing

[David Allsopp]

> On Wed, Aug 13, 2008 at 09:54:36AM +0100, David Allsopp wrote:
> > Suppose I have this piece of code:
> > 
> > let foo xs =
> >   match xs with
> >     x::xs -> if x
> >              then xs (* Return the tail of the list *)
> >              else xs (* Return the entire list *)
> >   | []    -> raise Exit
>
> I'd find it very counter-intuitive if OCaml behaved like this, and
> annoying if it gave a warning.  Just name the variables to be
> different!

You seem to have missed my point that I wrote the above *in error* so "Just
name the variables to be different!" is a clairvoyant response in this
case... I'm aware that that's what has to change :o)

That said, I do see your point that my proposed warning would mean that you
could never match a list tail with the same name as the whole list - a
definite weakness of my suggestion that I hadn't spotted before.

Personally, I'd be happy to live with always using two names in this context
- but perhaps this should therefore be the job of a home-grown postprocessor
on .annot files, rather than a compiler feature request?


David

Re: [Caml-list] Value shadowing

[blue storm]

[-- Attachment #1: Type: text/plain, Size: 244 bytes --]

It could also be possible, with some more camlp4 sweetness, to allow for
local, explicit shadowing.
You'd write something like    | rebind x::xs -> , and it would suppress the
warning.

The problem is that it would make the extension invasive.

[-- Attachment #2: Type: text/html, Size: 301 bytes --]

Re: [Caml-list] Value shadowing

[Mauricio Fernandez]

On Wed, Aug 13, 2008 at 12:04:21PM +0100, David Allsopp wrote:
> > On Wed, Aug 13, 2008 at 09:54:36AM +0100, David Allsopp wrote:
> > > Suppose I have this piece of code:
> > > 
> > > let foo xs =
> > >   match xs with
> > >     x::xs -> if x
> > >              then xs (* Return the tail of the list *)
> > >              else xs (* Return the entire list *)
> > >   | []    -> raise Exit
> >
> > I'd find it very counter-intuitive if OCaml behaved like this, and
> > annoying if it gave a warning.  Just name the variables to be
> > different!
> 
> You seem to have missed my point that I wrote the above *in error* so "Just
> name the variables to be different!" is a clairvoyant response in this
> case... I'm aware that that's what has to change :o)

(Somewhat tangentially)

I often reuse variable names *in order to avoid errors*, e.g., I prefer
  let l = foo l in
to
  let l' = foo l in
when I have no need for the original l in subsequent code and both l and l'
have the same type. This prevents a potential bug (using l instead of l'
later).

Shadowing is useful when you could code in point-free style but decide to name
the intermediate values for clarity.

-- 
Mauricio Fernandez  -   http://eigenclass.org

Re: [Caml-list] Value shadowing

[blue storm]

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

Here is a draft of camlp4 extension warning on value shadowing :
  http://bluestorm.info/camlp4/dev/pf_shadow/list.php

You can use it as a preprocessor to your source file, and it will raise
warnings on value shadowing.
For example (more examples in the test.ml file), your input gives the
warning :
<W> File "input.ml", line 4, characters 6-8: shadowing binding 'xs' from
File "input.ml", line 2, characters 8-10

Recursive bindings are handled, but classes, types and modules are not : it
is a reasonable proof of concept, and if somebody is interested in more
exhaustiveness, i (or whoever) could probably extend it easily.

[-- Attachment #2: Type: text/html, Size: 870 bytes --]