#11947 closed bug (fixed)

GHC mistakenly warns about type defaulting in the presence of -XTypeApplications

Reported by: RyanGlScott Owned by:
Priority: normal Milestone: 8.0.2
Component: Compiler (Type checker) Version: 8.0.1
Keywords: TypeApplications Cc: goldfire
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Incorrect warning at compile-time Test Case: typecheck/should_compile/T11947
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

This code:

{-# LANGUAGE RankNTypes #-}
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE TypeApplications #-}
module Main (main) where

theFloatDigits :: forall a. RealFloat a => Int
theFloatDigits = floatDigits (undefined @_ @a)

main :: IO ()
main = print (theFloatDigits @Double, theFloatDigits @Float)

erroneously produces this warning:

$ /opt/ghc/8.0.1/bin/runghc -Wall TheFloatDigits.hs

TheFloatDigits.hs:6:19: warning: [-Wtype-defaults]
    • Defaulting the following constraint to type ‘Double’ RealFloat a0
    • In the ambiguity check for ‘theFloatDigits’
      To defer the ambiguity check to use sites, enable AllowAmbiguousTypes
      In the type signature:
        theFloatDigits :: forall a. RealFloat a => Int
(53,24)

GHC's claim that a0 was defaulted to Double is clearly bogus, since theFloatDigits outputs different answers for Double and Float.

Change History (12)

comment:1 Changed 19 months ago by goldfire

This is correct behavior.

As described here, the ambiguity check for theFloatDigits does precisely this:

check :: forall a. RealFloat a => Int   -- type copied from theFloatDigits
check = theFloatDigits

When checking this declaration, a is indeed defaulted to Double, as reported. This is a consequence of the fact that theFloatDigits's type is indeed ambiguous.

A few ways forward here:

  1. Clarify the mechanism behind the ambiguity check in the manual. It's stated in there now, but we could make this more prominent and describe that GHC really does type-check a binding as above.
  2. Disable type defaulting in an ambiguity check.
  3. Do nothing.

comment:2 Changed 19 months ago by RyanGlScott

I suppose this is correct, but it hardly feels intuitive to me. The definition of theFloatDigits, as written above, doesn't seem ambiguous at all, given that we are explicitly applying the type a to undefined. The fact that GHC is emitting a warning at all appears to be byproduct of the particular implementation it uses to check for ambiguity.

That being said, it looks like option 2 would be the most sensible option? I can't envision a scenario in which not defaulting types in an ambiguity check would make a difference.

comment:3 Changed 19 months ago by goldfire

theFloatDigits isn't the thing that's ambiguous or not; it's theFloatDigits's type, forall a. RealFloat a => Int, which certainly is ambiguous, mentioning a type variable only in a constraint and not the type head.

comment:4 in reply to:  3 Changed 19 months ago by RyanGlScott

Replying to goldfire:

theFloatDigits isn't the thing that's ambiguous or not; it's theFloatDigits's type, forall a. RealFloat a => Int, which certainly is ambiguous, mentioning a type variable only in a constraint and not the type head.

Sorry, I was confused. You're absolutely right in that theFloatDigits's type signature is ambiguous. That is apparent if you try to typecheck something similar without defaulting rules like:

theIsSigned :: forall a. Bits a => Bool
theIsSigned = isSigned (undefined @_ @a)

This fails to compile without -XAllowAmbiguousTypes enabled, which makes sense.

theFloatDigits, on the other hand, compiles both with and without -XAllowAmbiguousTypes! This adds another wrinkle to the bug—should it require -XAllowAmbiguousTypes, or should it type defaulting make certain otherwise-ambiguous type signatures permissible?

I'm not sure what the answer is, but I do believe that we should at least have some way to write theFloatDigits without needing to disable -Wtype-defaults altogether. Would it make sense to disable type defaulting in ambiguity checks when -XAllowAmbiguousTypes is enabled? After all, the warning message already suggests turning it on.

comment:5 Changed 19 months ago by goldfire

But you should get no warning with -XAllowAmbiguousTypes, as that skips the ambiguity check that causes the warning. Or am I mistaken here?

comment:6 in reply to:  5 Changed 19 months ago by RyanGlScott

Replying to goldfire:

But you should get no warning with -XAllowAmbiguousTypes, as that skips the ambiguity check that causes the warning. Or am I mistaken here?

You do get a warning, it seems:

$ /opt/ghc/8.0.1/bin/runghc -Wall -XAllowAmbiguousTypes TheFloatDigits.hs 

TheFloatDigits.hs:6:19: warning: [-Wtype-defaults]
    • Defaulting the following constraint to type ‘Double’ RealFloat a0
    • In the ambiguity check for ‘theFloatDigits’
      In the type signature:
        theFloatDigits :: forall a. RealFloat a => Int
(53,24)

comment:7 Changed 19 months ago by goldfire

That is a solid bug. GHC should do no such thing.

comment:8 Changed 19 months ago by simonpj

I think that the solution here is fairly simple: simplifyAmbiguityCheck should not do defaulting. Should not be hard to try this out.

Simon

comment:9 Changed 19 months ago by Simon Peyton Jones <simonpj@…>

In edf54d72/ghc:

Do not use defaulting in ambiguity check

This fixes Trac #11947.  See TcSimplify
Note [No defaulting in the ambiguity check]

comment:10 Changed 19 months ago by simonpj

Resolution: fixed
Status: newclosed
Test Case: typecheck/should_compile/T11947

Yes, that worked.

The change is small and non-invasive, so could be merged; but it's also a corner case so I think I'll suggest leaving for 8.2 unless anyone yells.

Simon

comment:11 Changed 19 months ago by RyanGlScott

Milestone: 8.0.2
Status: closedmerge

It'd be nice to have this in 8.0.2, since this bug currently requires me to disable -Wall in one of my projects.

comment:12 Changed 15 months ago by bgamari

Status: mergeclosed
Note: See TracTickets for help on using tickets.