Opened 2 years ago

Last modified 12 months ago

#13905 new bug

ApplicativeDo is too strict with newtype patterns

Reported by: dfeuer Owned by:
Priority: normal Milestone: 8.10.1
Component: Compiler Version: 8.3
Keywords: ApplicativeDo Cc: simonmar
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: GHC rejects valid program Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

The fix to #13875 went a little too far, and interprets newtype constructor patterns as strict. It's not clear to me how to fix this, as the Name of the constructor doesn't seem to give any clue as to whether it is a newtype constructor or a data constructor. The comment at the top of basicTypes/Name.hs indicates

-- *  'Name.Name' is the type of names that have had their scoping and binding resolved. They
--   have an 'OccName.OccName' but also a 'Unique.Unique' that disambiguates Names that have
--   the same 'OccName.OccName' and indeed is used for all 'Name.Name' comparison. Names
--   also contain information about where they originated from, see "Name#name_sorts"

which suggests that this information should be available, but I don't know what's involved in propagating it to the right place.

Change History (8)

comment:1 Changed 2 years ago by simonmar

I don't think this is fixable, I documented it in Phab:D3691

comment:2 Changed 2 years ago by dfeuer

I'm not trying to be annoying, but may I ask why you don't think it's fixable? Don't we already know, at this point, where the constructor name comes from? I see the documentation, and the documentation is fine, but I suspect idiomatic code will run into this, and people will likely be surprised to see explicit ~ laziness around newtype constructors.

comment:3 Changed 2 years ago by simonmar

We do ApplicativeDo rearrangement in the renamer, and we don't have the mapping from Names to TyThings until the typechecker, so at least superficially it seems hard.

comment:4 Changed 2 years ago by simonpj

That is extremely annoying, and so clearly an artefact of GHC's architecture, rather than any solid reason, that we should question it.

For example we could

  1. Keep track in the renamer of what local data constructors are newtypes. (We already know this for imported ones.)
  1. Rename and typecheck all data/class decls before we even start on the value declarations. A bit like we do for top-level Template Haskell splices.

The second of these looks more solid to me. I recall that there's a similar issue with record fields, when the renamer wants to know what fields a particular data constructor has. Currently we use a variant of (1) I think; but (2) would solve it and thereby simplify that code.

So I quite like (2).

comment:5 Changed 2 years ago by simonpj

Keywords: ApplicativeDo added

comment:6 Changed 23 months ago by bgamari

Milestone: 8.4.18.6.1

This ticket won't be resolved in 8.4; remilestoning for 8.6. Do holler if you are affected by this or would otherwise like to work on it.

comment:7 Changed 18 months ago by bgamari

Milestone: 8.6.18.8.1

These won't be addressed by GHC 8.6.

comment:8 Changed 12 months ago by osa1

Milestone: 8.8.18.10.1

Bumping milestones of low-priority tickets.

Note: See TracTickets for help on using tickets.