Opened 8 years ago

Last modified 3 years ago

#5288 new feature request

Less noisy version of -fwarn-name-shadowing

Reported by: batterseapower Owned by:
Priority: low Milestone:
Component: Compiler Version: 7.0.3
Keywords: Cc: malcolm.wallace@…, nathan.collins@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Incorrect warning at compile-time Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description (last modified by batterseapower)

I would like a flag that warns about name-shadowing in more restricted circumstances than the current flag. I.e. I would like examples like these not to produce a warning:

foo x = ..
  where
    bar x = ...
baz z = do
  z <- .... z ...
  return z

But I would like these to produce one:

foo x = ..
  where
    x = ... x ...
baz z = mdo
  z <- .... z ...
  return z

Basically warn when a definition shadows *itself*. My motivation is that code like my first two examples is almost never an error in my experience, but my last two examples almost always are examples of my accidentally building a loop that evaluates to _|_.

Change History (17)

comment:1 Changed 8 years ago by batterseapower

Description: modified (diff)

comment:2 Changed 8 years ago by malcolm.wallace@…

Cc: malcolm.wallace@… added

comment:3 Changed 8 years ago by simonpj

I don't understand what you mean by saying "a definition shadows itself".

And while I sometimes make silly loops too (of the form x = ...x...), they may or may not involve shadowing.

Maybe you can write a more precise spec, and argue why it's useful?

Simon

comment:4 Changed 8 years ago by batterseapower

My thinking is that when I write:

foo rn = ... rn ...
  where
    rn = ... rn ...

My intention is almost invariably to bind a slightly-modified version of rn. However, in the code above I have made a mistake and reused the old name for the new version. What I should have written was:

foo rn = ... rn' ...
  where
    rn' = ... rn ...

The fact that the "rn = ... rn ..." binding shadowed an existing binding called rn is the clue that I was trying to slightly-modify an existing binding. I don't want to warn if "rn = ... rn ..." does not cause any shadowing because no such clue exists.

Does that make more sense?

I used to write programs with this error at the rate of perhaps 1 program per 20 hours of coding, but I'm getting better at avoiding it.

comment:5 Changed 8 years ago by simonpj

I'm still hoping for a *specification*. Perhaps you mean "report shadowing only for a recursive binding"?

comment:6 Changed 8 years ago by igloo

Milestone: 7.4.1

comment:7 Changed 8 years ago by batterseapower

Simon: yes. But now that I think about it, perhaps a more useful warning is one that warns about ANY recursive pattern/variable binding? This would help me find the class of bugs caused by accidental value-recursive bindings in those situations that I need do. Those people who believe silent letrec is a misfeature (there are some) could even turn it on for their whole project and use "fix" to explicitly mark those places where they really intend to use letrec.

comment:8 Changed 8 years ago by igloo

Milestone: 7.4.17.6.1
Priority: normallow

comment:9 Changed 7 years ago by igloo

Milestone: 7.6.17.6.2

comment:10 Changed 5 years ago by thoughtpolice

Milestone: 7.6.27.10.1

Moving to 7.10.1.

comment:11 Changed 5 years ago by thoughtpolice

Milestone: 7.10.17.12.1

Moving to 7.12.1 milestone; if you feel this is an error and should be addressed sooner, please move it back to the 7.10.1 milestone.

comment:12 Changed 4 years ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

comment:13 Changed 4 years ago by thomie

Type of failure: None/UnknownIncorrect warning at compile-time

comment:14 Changed 4 years ago by thomie

Milestone: 8.0.1

comment:15 Changed 3 years ago by ntc2

+1 (and is there a better way to say "I like this idea" than adding a "+1" comment like this?)

comment:16 Changed 3 years ago by ntc2

Cc: nathan.collins@… added

comment:17 Changed 3 years ago by mpickering

I think there still needs to be a proper specification and then of course an implementation.

Note: See TracTickets for help on using tickets.