#14527: Warn on recursive bindings -------------------------------------+------------------------------------- Reporter: chrisdone | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Description changed by chrisdone: Old description:

When you accidentally write something like `let x = .. x ...` it can take hours to realize where you made your mistake. This hits me once in a while, and my colleagues often.

I'd propose e.g. `-Wrecursive-bindings` that says:

{{{ warning: [-Wrecursive-bindings] Recursive binding for `x` in

let x = length x }}}

This applies to `let`, `where` and top-level pattern bindings.

I believe that in practice, I only actually use real recursive bindings once in a while. So I might be bold enough to encourage enabling it in `-Wall` for a future major GHC release.

With the compromise that if you have the warning enabled but in one specific place, you want a recursive binding, you can use the `~` tilde to say "I really mean it", e.g.

{{{let ~ones = 1 : ones}}}

That seems like a nice balance to say "I know what I'm doing in this case". So the warning could be more helpful, like:

{{{ warning: [-Wrecursive-bindings] Recursive binding for `ones` in

let ones = length ones

If intentional, use the tilde marker on the name like this:

let ~ones = length ones

}}}

In Intero if I were to implement a prototype of this check, I'd probably do this, after renaming:

1. Use SYB to collect all variable bindings from the pattern. 2. Use SYB to listify all mentions of any of these variables in the RHS and any guards or where clauses.

If the list is non-empty, then trigger the error. A transformation function {{{[Name] -> Pat -> Pat}}} would provide the AST with the offending name(s) tilded as {{{~x}}} for use in the error message.

If there's general agreement, I could implement this change.

New description: When you accidentally write something like `let x = .. x ...` it can take hours to realize where you made your mistake. This hits me once in a while, and my colleagues often. I'd propose e.g. `-Wrecursive-bindings` that says: {{{ warning: [-Wrecursive-bindings] Recursive binding for `x` in let x = length x }}} This applies to `let`, `where` and top-level pattern bindings. I believe that in practice, I only actually use real recursive bindings once in a while. So I might be bold enough to encourage enabling it in `-Wall` for a future major GHC release. With the compromise that if you have the warning enabled but in one specific place, you want a recursive binding, you can use the `~` tilde to say "I really mean it", e.g. {{{let ~ones = 1 : ones}}} That seems like a nice balance to say "I know what I'm doing in this case". So the warning could be more helpful, like: {{{ warning: [-Wrecursive-bindings] Recursive binding for `ones` in let ones = length ones If intentional, use the tilde marker on the name like this: let ~ones = length ones }}} In Intero if I were to implement a prototype of this check, I'd probably do this, after renaming: 1. Use SYB to collect all variable bindings from the pattern. 2. Use SYB to listify all mentions of any of these variables in the RHS and any guards or where clauses. If the list is non-empty, then trigger the error. A transformation function {{{[Name] -> Pat -> Pat}}} would provide the AST with the offending name(s) tilded as {{{~x}}} for use in the error message. If there's general agreement, I could implement this change. **EDIT**: mutually recursive bindings apply here too. So {{{let x = y; y = x}}} by a regular occurs check. -- -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14527#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14527: Warn on recursive bindings -------------------------------------+------------------------------------- Reporter: chrisdone | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by nh2): **GHC 7.10 already had this feature**, but it disappeared with 8.0, and it's not clear to me if intentionally or accidentally. It would be great to find out. In 7.10, you could simply used a BangPattern to forbid recursive variable bindings. Example {{{ Recursive bang-pattern or unboxed-tuple bindings aren't allowed: !x = x }}} And it said in the manual: {{{ a bang-pattern binding must be non-recursive }}} It was removed for GHC 8.0 in these commits: * https://github.com/ghc/ghc/commit/e7985ed23ddc68b6a2e4af753578dc1d9e8ab4c9 #diff-47b9f40b0ede605e7b6bcb330b43ad53L1678 * https://github.com/ghc/ghc/commit/46a03fbec#diff- 02e7842997523ff8a5ad0312df2edfe2L10348 The question is: Why was this removed? Was it accidental, or did people find a legitimate use case for strict recursive variable let bindings? If not, we could simply re-introduce that, without a language extension (though we might still add the proposed language extension to get warnings even without bangs). -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14527#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14527: Warn on recursive bindings -------------------------------------+------------------------------------- Reporter: chrisdone | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by adamse): I cannot remember removing this warning intentionally. It might have been part of the bigger plan around {{{-XStrict}}} and the changes to the semantics of bang patterns it introduced. I'll dig around and see if I can find out if it was unintentional. Sorry for not responding on IRC. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14527#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14527: Warn on recursive bindings -------------------------------------+------------------------------------- Reporter: chrisdone | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by chrisdone): For context, Niklas and I were discussing this problem and I mentioned the !x restriction, only to discover that the restriction wasn't present on GHC 8. So Niklas digged and found the commit, but we weren't sure why. However, it's not quite true that GHC had ''this'' feature. Certainly, getting {{{!x}}} to disable recursion back into GHC should be much easier to convince GHC maintainers to add it back, assuming it was removed accidentally. Maybe that belongs in a separate but related ticket. More generally speaking, self-referential variables in my experience are almost always a mistake and ''once in a while'' an intentional thing for me. So it would actually be nice if GHC warned about it and I had to put an explicit `~` in the ''few times'' I wanted it, rather than `!` ''everywhere'', just to avoid the problem. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14527#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14527: Warn on recursive bindings -------------------------------------+------------------------------------- Reporter: chrisdone | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by goldfire): In the levity polymorphism work, there were some annoying interactions between the treatment of strict and unlifted bindings and levity polymorphism. I don't recall the details, but it was necessary to edit the code in this area. As I was doing so, it seemed the code was confused around the restrictions on unlifted bindings vs. strict ones. There is no need to prevent strict recursive bindings, and so we don't. In the end, though, I don't think that disallowing strict recursive bindings is the real answer to this need (which I've shared) -- the warning route originally proposed would be far better. I agree with Ben that posting a proposal is the best next step. If I may be so bold as to comment prematurely, I would likely support such a proposal. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14527#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14527: Warn on recursive bindings -------------------------------------+------------------------------------- Reporter: chrisdone | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by nomeata): Just for clarification. Does

This applies to let, where and top-level pattern bindings.

Mean it applies to {{{ let x = ones : x }}} and {{{ let f = \n -> if n == 0 then 1 else n * f (n-1) }}} but not {{{ let f n = if n == 0 then 1 else n * f (n-1) }}} right? (I think I like the explicit `~` on recursive value bindings.) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14527#comment:11 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14527: Warn on recursive bindings -------------------------------------+------------------------------------- Reporter: chrisdone | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj):

So I'm afraid it's less straight-forward than I hoped.

It's only a warning so it's ok to have ad-hoc (but still well specified) conditions. For example: you get a warning if * The binding is recursive (or part of a recursive group) * It is a pattern or simple variable binding; that is there are no argument on the LHS * The LHS does not have a top-level tilde * The RHS is not a syntactic value. Then define syntactic values to be: literal, constructor application, lambda, `\case`, etc. (This is the slightly ad-hoc bit.) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14527#comment:13 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14527: Warn on recursive bindings -------------------------------------+------------------------------------- Reporter: chrisdone | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by chrisdone): Right, I was aiming for {{{ones = 1 : ones}}} to be a warning. How about we rephrase it as "function-like": * The RHS is not a function-like: either a lambda or a lambda-case. (If the RHS is a literal then by-definition it's not recursive and wouldn't trigger the warning, I think.) With an ad-hoc bit like that, I'm more convinced that this could be practical. I'll consider making a proposal for it. A draft implementation could be run against Stackage to see how many false-positives it flags up. Here and reddit provided enough concerns -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14527#comment:15 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler