#9127 closed bug (fixed)

Don't warn about pattern-bindings of the form `let !_ = rhs`

Reported by: refold Owned by:
Priority: normal Milestone:
Component: Compiler Version: 7.8.2
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description

With GHC 7.8.2, code like

        let !_           = rqPostParams rq
        let !_           = rqParams rq

triggers the following warnings:

src/Snap/Internal/Test/RequestBuilder.hs:150:13: Warning:
    This pattern-binding binds no variables: !_ = rqPostParams rq

src/Snap/Internal/Test/RequestBuilder.hs:151:13: Warning:
    This pattern-binding binds no variables: !_ = rqParams rq

I think that let !_ = rhs shouldn't trigger a warning, just like let _ = rhs doesn't.

Change History (17)

comment:1 Changed 14 months ago by simonpj

Harump. Maybe let _ = rhs should trigger a warning! It's dead code after all.

But the let !_ = e in b means just e `seq` b so I suppose you could argue that it non-warnable.

Does anyone else care?

Simon

comment:2 follow-up: Changed 13 months ago by igloo

This sounds a little odd to me. What's the actual use-case here? A test that the parameter parser doesn't thrown an exception?

Unless there's a common reason for wanting to do this, I think it would be better for people who really want to do this to have to use seq or !_rqPostParams = ... instead.

comment:3 in reply to: ↑ 2 Changed 13 months ago by refold

Replying to igloo:

This sounds a little odd to me. What's the actual use-case here? A test that the parameter parser doesn't thrown an exception?

The code looks approximately like this:

foo :: RequestBuilder
foo = do rq <- rGet
         let !_ = rqPostParams rq
         ...
         rPut rq

where RequestBuilder is a state monad (StateT ...). So we're forcing some parts of the state before passing it along; let !_ = rhs is used as a nicer syntax for seq. IMO it's a valid idiom.

Last edited 13 months ago by refold (previous) (diff)

comment:4 follow-up: Changed 13 months ago by igloo

Ah, well, it'll probably do what you want. But if, for example, at a usage site GHC can see the expression that has been put in rqPostParams and thinks that it is cheap, then it may evaluate a copy of it rather than sharing the value. #2273 is an example of something similar.

To be sure, I think you'd need something like:

foo :: RequestBuilder
foo = do rq <- rGet
         let !rqPostParams' = rqPostParams rq
         ...
         rPut $! (rq { rqPostParams = rqPostParams' })

comment:5 in reply to: ↑ 4 Changed 13 months ago by refold

Replying to igloo:

Thanks, that's good to know.

comment:6 Changed 13 months ago by gzayas

  • Owner set to gzayas

comment:7 Changed 13 months ago by gzayas

  • Status changed from new to patch

comment:8 Changed 13 months ago by Joachim Breitner <mail@…>

In fbdebd30b9ff3ca76243791723b85959c6860083/ghc:

supress warning of bang wildcard pattern-binding (i.e. let !_ = rhs). This fixes #9127

comment:9 Changed 13 months ago by nomeata

Reviewed, validated and pushed, thanks Guido for your first contribution to GHC!

comment:10 Changed 13 months ago by nomeata

  • Resolution set to fixed
  • Status changed from patch to closed

comment:11 Changed 13 months ago by simonpj

I'd love to see this documented in the user manual, please. Look at sanity checking and -fwarn-unused-bindings.

There are really two things enabled by -fwarn-unused_bindings:

  • Warning about a named variable brought into scope but not used (unless exported or starting with underscore). e.g.
    let f x = rhs1 in True   -- Warning for unused f
    let (p,q) = rhs2 in p+1  -- Warning about unused q
    
  • Warning about a pattern binding that doesn't bind anything; e.g.
    Just _ = rhs1     -- Warning for unused binding
    (_, _) = rhs2     -- Warning for unused binding
    
    But no warning is given for a lone wild-card pattern or banged wild-card pattern:
    _  = rhs3  -- No warning
    !_ = rhs4  -- No warning; behaves like seq
    
    The former is not very different from _v = rhs3 which elicits no warning; and can be useful to add a type constraint, e.g. _ = x::Int. The latter is useful as an alternative way to force evaluation.

Could you add all that to the manual? Thanks

comment:12 Changed 13 months ago by simonpj

  • Owner gzayas deleted
  • Resolution fixed deleted
  • Status changed from closed to new

comment:13 Changed 13 months ago by gzayas

Simon, documentation changed as requested. Please have a look and let me know.
Thanks,
Guido

comment:14 Changed 13 months ago by gzayas

  • Status changed from new to patch

comment:15 Changed 13 months ago by simonpj

  • Resolution set to fixed
  • Status changed from patch to closed

Thank you. I edited a bit more and committed

commit aa18a46d85a4995f0daea63d44e627f11d03ce95
Author: Simon Peyton Jones <[email protected]>
Date:   Mon Jun 9 13:58:23 2014 +0100

    Improve documentation for -fwarn-unused-binds
Note: See TracTickets for help on using tickets.