Opened 8 months ago

Last modified 8 months ago

#14892 new bug

Field imposters with DuplicateRecordFields and NamedFieldPuns.

Reported by: philderbeast Owned by:
Priority: normal Milestone:
Component: Compiler Version: 8.4.1-alpha3
Keywords: ORF Cc: adamgundry
Operating System: MacOS X Architecture: x86_64 (amd64)
Type of failure: GHC accepts invalid program Test Case:
Blocked By: Blocking:
Related Tickets: #13644 Differential Rev(s):
Wiki Page:

Description

There's a test case for this ghc panic. If I use NamedFieldPuns I wish this could more delicately pick the right field. If my record use is qualified then please restrict the set of candidate field names to use punned.

{-# LANGUAGE DuplicateRecordFields #-}
module Geodesy (X(..), Y(..)) where

data X a = X {x :: a}
data Y a = Y {x :: a}
{-# LANGUAGE NamedFieldPuns #-}
module GhcPanic12158 where

import qualified Geodesy as G (X(..))
import Geodesy (Y(..))

update :: G.X a -> G.X a
update G.X{x} = G.X{x = x}
> stack build
ghc-panic-translateConPatVec-lookup-0.1.0: build (lib)
Preprocessing library for ghc-panic-translateConPatVec-lookup-0.1.0..
Building library for ghc-panic-translateConPatVec-lookup-0.1.0..
[1 of 3] Compiling Geodesy
[2 of 3] Compiling GhcPanic12158
ghc: panic! (the 'impossible' happened)
  (GHC version 8.2.2 for x86_64-apple-darwin):
	translateConPatVec: lookup

Please report this as a GHC bug:  http://www.haskell.org/ghc/reportabug

There are some fixes;

  1. Add DuplicateRecordFields.
{-# LANGUAGE NamedFieldPuns #-}
{-# LANGUAGE DuplicateRecordFields #-}
module GhcPanic12158 where

import qualified Geodesy as G (X(..))
import Geodesy (Y(..))

update :: G.X a -> G.X a
update G.X{x} = G.X{x = x}
  1. Use qualified field names.
{-# LANGUAGE NamedFieldPuns #-}
module GhcPanic12158 where

import qualified Geodesy as G (X(..))
import Geodesy (Y(..))

update :: G.X a -> G.X a
update G.X{G.x} = G.X{G.x = x}

Interestingly, if I don't import the record with the clashing field name then GHC complains.

{-# LANGUAGE NamedFieldPuns #-}
module GhcPanic12158 where

import qualified Geodesy as G (X(..))

update :: G.X a -> G.X a
update G.X{x} = G.X{x = x}
> stack build
ghc-panic-translateConPatVec-lookup-0.1.0: unregistering
ghc-panic-translateConPatVec-lookup-0.1.0: build (lib)
Preprocessing library for ghc-panic-translateConPatVec-lookup-0.1.0..
Building library for ghc-panic-translateConPatVec-lookup-0.1.0..
[3 of 3] Compiling GhcPanic12158

/ghc-panic-12158/earth/library/GhcPanic12158.hs:8:12: error: Not in scope: ‘x’
  |
8 | update G.X{x} = G.X{x = x}
  |            ^

/ghc-panic-12158/earth/library/GhcPanic12158.hs:8:21: error: Not in scope: ‘x’
  |
8 | update G.X{x} = G.X{x = x}

Change History (5)

comment:1 Changed 8 months ago by philderbeast

Summary: Field imposters with DuplicateRecordFieldsField imposters with DuplicateRecordFields and NamedFieldPuns.

comment:2 Changed 8 months ago by RyanGlScott

Cc: adamgundry added
Keywords: ORF added
Version: 8.2.28.4.1-alpha3

FWIW, this does not panic on GHC 8.4.1 (after #13644 was fixed), but instead gives an incorrect error message:

$ /opt/ghc/8.4.1/bin/ghci GhcPanic12158.hs 
GHCi, version 8.4.0.20180224: http://www.haskell.org/ghc/  :? for help
Loaded GHCi configuration from /home/rgscott/.ghci
[1 of 2] Compiling Geodesy          ( Geodesy.hs, interpreted )
[2 of 2] Compiling GhcPanic12158    ( GhcPanic12158.hs, interpreted )

GhcPanic12158.hs:8:12: error:
    • Constructor ‘G.X’ does not have field ‘x’
    • In the pattern: G.X {x}
      In an equation for ‘update’: update G.X {x} = G.X {x = x}
  |
8 | update G.X{x} = G.X{x = x}
  |            ^

I believe this lines up with Adam's work on overloaded record fields, so I'll label this ticket as such.

comment:3 Changed 8 months ago by bgamari

Thanks for reducing this, philderbeast!

comment:4 Changed 8 months ago by philderbeast

Ryan, you beat me to the test with a later GHC version, something Ben asked me to do on #12158. I too am seeing the problem of GHC's panic fixed and have updated the test repository to compile against a later version of GHC that doesn't panic. I do wish however that the compiler here would pick the one x pun that fits.

# stack.yaml
resolver: ghc-8.4.0.20180118
compiler: ghc-8.4.0.20180118
compiler-check: match-exact
> stack build
ghc-panic-translateConPatVec-lookup-0.1.0: build (lib)
Preprocessing library for ghc-panic-translateConPatVec-lookup-0.1.0..
Building library for ghc-panic-translateConPatVec-lookup-0.1.0..
[2 of 3] Compiling GhcPanic12158

/earth/library/GhcPanic12158.hs:9:12: error:
    • Constructor ‘G.X’ does not have field ‘x’
    • In the pattern: G.X {x}
      In an equation for ‘update’: update G.X {x} = G.X {x = x}
  |
9 | update G.X{x} = G.X{x = x}
Last edited 8 months ago by philderbeast (previous) (diff)

comment:5 Changed 8 months ago by adamgundry

I think you want DisambiguateRecordFields. Without it, the 8.4 behaviour is correct, isn't it? Indeed G.X does not have a field x in scope (although it does have G.x). The error message could be better, perhaps suggesting DisambiguateRecordFields.

Note that DuplicateRecordFields implies DisambiguateRecordFields, which is why enabling the former fixes the problem.

Note: See TracTickets for help on using tickets.