UNPACK pragmas jumble fields and terms or: hsc_unit_envs has wrong type in variables pane and can't expand

[alt-romes]

When inspecting an HscEnv variable hsc_env in the variables pane I see hsc_unit_env :: FilePath -> IO Fingerprint; but that's very wrong!? It should be of type UnitEnv and we should be able to expand and inspect it.

[alt-romes]

Ah, the fields seem to be all confused. Note that hsc_llvm_config is displayed as the UnitEnv!?

Clearly something went wrong in the logic which associates Field Labels to the Term's actual sub-terms.
We're probably assuming an ordering which doesn't exist some times.

This should be in termVarFields, the calls to do with dataConFieldLabels.

[alt-romes]

After investigating it became clear that the issue is that HscEnv has UNPACKED fields.

An UNPACKED field will result in there being more subTerms than there are fieldLabels for a particular data constructor (we previously worked under the assumption that the dataconfields matched 1-1 with the Term subTerms; but on the following example, containerPacked field will actually contain two sub terms directly)

module Main where

data Packed = Packed
  { packedLeft :: Int
  , packedRight :: Int
  }
  deriving Show

data Container = Container
  { containerBefore :: Int
  , containerPacked :: {-# UNPACK #-} !Packed
  , containerAfter :: Int
  }
  deriving Show

mkContainer :: Container
mkContainer = Container 10 (Packed 20 30) 40

main :: IO ()
main = do
  let !container = mkContainer
  const (pure ()) container
  print container

[alt-romes]

I've added a regression test for this at #241. No fix yet (feel free to tackle this ...)

[alt-romes]

It'd be good to agree on a design. What should happen here anyway? We have more subTerms than recordFields in the data constructor, so what should be displayed?

An easy solution is to discard record labels when their number doesn't match the subTerms.

But we can probably do better than that.

Again, the code that needs to be changed here with the design that ends up agreed upon is termVarFields

[michaelpj]

Drive by comment as I haven't used this yet, but it seems to me like there's a question about whether to present the "logical" structure or the "physical" structure.

• If we present the logical structure, we could try and re-create the unpacked inner structure for the debug view
• If we present the physical structure, we could just show the unpacked fields where they are, but label them appropriately (so we'd have a containerPacked.packedLeft field and a containerPacked.packedRight field

The logical structure is probably least surprising (especially since GHC can choose to unpack things automatically!) but I think there's something to be said for the debugger giving you a more faithful view of what's actually going on? Perhaps this is a higher-level question that should have a global answer?

[alt-romes]

Thanks Michael, the physical option is a good suggestion!

In the debugger, many of our decisions have used the philosophy of "Do not try to hide the actual evaluation model and runtime details of the language". That was first true in 2007 when Simon M. et all claimed the debugger should display lazy evaluation as is; and is still true of decisions we do now. Another example is tail calls: if a function is tail called, there won't be a stack frame and we just won't display one in the debugger. That's fine!

In my opinion we should do something like the physical structure as you propose. If a Term really has more direct subterms than it does record fields, then there are unpacked fields and the debugger should display them with the appropriate labels.

FWIW It's not immediately clear how to do either the logical or physical option :). We have N record fields and M subterms, how do we match them up? Would types be sufficient to pair them up?

An example to consider:

data X = X !Int !Int
data Y = Y { a :: {-# UNPACK #-} !X, b :: !Int }

Will result in Y with fields {a, b} and subterms {_ :: Int, _ :: Int, _ :: Int}.

[michaelpj]

Maybe we don't have enough information to do this at present, then? To do what I'm suggesting, we'd need to know that

1. Field X is in fact a field of the unpacked datastructure originally stored as field Y
2. The field of Y which X corresponds to is Z

So we could label it as Y.Z. Possibly neither of those is easy to get.

[alt-romes]

I'll investigate the implementation details a bit further during the week. Thanks!