Opened 3 years ago

Last modified 3 months ago

#10946 new bug

Typed hole inside typed Template Haskell bracket causes panic

Reported by: jstolarek Owned by:
Priority: high Milestone: 8.8.1
Component: Template Haskell Version: 7.10.1
Keywords: Cc: goldfire, Tritlo
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Compile-time crash Test Case: th/T10946
Blocked By: Blocking:
Related Tickets: #14969 Differential Rev(s):
Wiki Page:

Description

When I say:

m :: a -> a
m x = $$([||_||])

I get:

T10946.hs:47:13:ghc: panic! (the 'impossible' happened)
  (GHC version 7.10.1 for x86_64-unknown-linux):
        No skolem info: a_apJ[sk]

This happens both with GHC 7.10.1 and latest HEAD (e2b579e8d77357e8b36f57d15daead101586ac8e).

Change History (19)

comment:1 Changed 3 years ago by jstolarek

This error is raised in TcErrors.getSkolemInfo called from TcErrors.mkHoleError. Quick look at the code tells me that "implication constraints" are expected to be non-empty but they are. Are implication constraints things that come in the contexts, ie. everything before a =>? Why are they expected to be non-empty?

comment:2 Changed 3 years ago by Jan Stolarek <jan.stolarek@…>

In f64f7c3/ghc:

Tests for #10945 and #10946

comment:3 Changed 3 years ago by jstolarek

Test Case: th/T10946

comment:4 Changed 3 years ago by Jan Stolarek <jan.stolarek@…>

In 75492e7/ghc:

Add typed holes support in Template Haskell.

Fixes #10267. Typed holes in typed Template Haskell currently don't work.
See #10945 and #10946.

comment:5 Changed 3 years ago by bgamari

Milestone: 8.0.18.2.1

This still fails and won't be fixed for 8.0.1.

comment:6 Changed 2 years ago by Ben Gamari <ben@…>

In c6ac1e5/ghc:

users_guide: TH now partially supports typed holes

As requested in #10267. However we still lack support in typed splices.
See #10945 and #10946.

comment:7 Changed 21 months ago by bgamari

Milestone: 8.2.18.4.1

It looks like this won't happen for 8.2 either.

comment:8 Changed 19 months ago by elliottt

The problem seems to be that the quoted expression [|| _x ||] will introduce a hole constraint of the form _x :: a when spliced, but that the error will be reported outside of the implication introduced by the signature. When checking the following program:

m :: a
m  = _x

simplifyTop will report that it's found an unsolved wanted constraint of the form forall (a :: *). () => _x :: a . However, when checking the original program, simplifyTop is called from a different path (tcTopSpliceExpr), and reports the wanted constraint as being of the form _x :: a . As it lacks the context of the binding for a, it falls through to the base case of getSkolemInfo, which causes the panic.

At this point, I'm not sure what the right path forward is. Should tcTopSpliceExpr be using simplifyTop, or is there just some additional context needed to be setup before that call?

comment:9 Changed 7 months ago by RyanGlScott

So in f64f7c36ef9395da1cc7b686aaf1b019204cd0fc, we added a test case for this that was marked as expect_broken. However, that program now loops infinitely on GHC HEAD!

$ inplace/bin/ghc-stage2 --interactive
GHCi, version 8.5.20180305: http://www.haskell.org/ghc/  :? for help
Loaded GHCi configuration from /home/rgscott/.ghci
λ> :set -XTemplateHaskell
λ> m :: a -> a; m x = $$([||_||]) -- Loops infinitely
^CInterrupted.

Should we disable the test case entirely in the meantime?

comment:10 Changed 7 months ago by RyanGlScott

A correction to comment:9—it doesn't loop infinitely, but it does take a full five minutes to complete that test case on my machine. Still, that's quite severe—we should find out what caused this to regress.

comment:11 Changed 7 months ago by RyanGlScott

Cc: Tritlo added

It looks like much of the slowdown was introduced in commit cbdea95938bf09e8e3e7be31918549224d171873 (Sort valid substitutions for typed holes by "relevance").

Tritlo, do you have any idea what might be happening here?

comment:12 Changed 7 months ago by Tritlo

RyanGlScott: It seems like what's happening is that since the variable has no constraints on it at all, then it's matching everything in scope (including everything in scope in TemplateHaskell), and then doing the subsumption graph sorting on that, which takes a very long time. I'll give it some thought, but it seems like the right way to go would be to not try to find valid substitutions for this too general case.

comment:13 Changed 7 months ago by simonpj

Or just to limit the maximum number of substitutions to ten, or something arbitrary like that.

comment:14 Changed 7 months ago by Tritlo

On the other hand, this is the right behavior, since a type variable with no constraints on it could be substituted with any identifier of any type. So the time it is taking is really due to the bug giving the hole a too general type.

simonpj: Yes, that's a good point. When we don't try to sort the substitutions, that is indeed what happens. We should set some limit for how many substitutions we attempt to sort by the subsumption graph method, and default to some other method otherwise (like e.g. sorting by module).

Last edited 7 months ago by Tritlo (previous) (diff)

comment:15 Changed 5 months ago by RyanGlScott

#14969 is the new home for the performance issue brought up in comment:9.

comment:16 Changed 4 months ago by Ben Gamari <ben@…>

In e0b44e2/ghc:

Improved Valid Hole Fits

I've changed the name from `Valid substitutions` to `Valid hole fits`,
since "substitution" already has a well defined meaning within the
theory. As part of this change, the flags and output is reanamed, with
substitution turning into hole-fit in most cases. "hole fit" was already
used internally in the code, it's clear and shouldn't cause any
confusion.

In this update, I've also reworked how we manage side-effects in the
hole we are considering.

This allows us to consider local bindings such as where clauses and
arguments to functions, suggesting e.g. `a` for `head (x:xs) where head
:: [a] -> a`.

It also allows us to find suggestions such as `maximum` for holes of
type `Ord a => a -> [a]`, and `max` when looking for a match for the
hole in `g = foldl1 _`, where `g :: Ord a => [a] -> a`.

We also show much improved output for refinement hole fits, and
fixes #14990. We now show the correct type of the function, but we also
now show what the arguments to the function should be e.g. `foldl1 (_ ::
Integer -> Integer -> Integer)` when looking for `[Integer] -> Integer`.

I've moved the bulk of the code from `TcErrors.hs` to a new file,
`TcHoleErrors.hs`, since it was getting too big to not live on it's own.

This addresses the considerations raised in #14969, and takes proper
care to set the `tcLevel` of the variables to the right level before
passing it to the simplifier.

We now also zonk the suggestions properly, which improves the output of
the refinement hole fits considerably.

This also filters out suggestions from the `GHC.Err` module, since even
though `error` and `undefined` are indeed valid hole fits, they are
"trivial", and almost never useful to the user.

We now find the hole fits using the proper manner, namely by solving
nested implications. This entails that the givens are passed along using
the implications the hole was nested in, which in turn should mean that
there will be fewer weird bugs in the typed holes.

I've also added a new sorting method (as suggested by SPJ) and sort by
the size of the types needed to turn the hole fits into the type of the
hole. This gives a reasonable approximation to relevance, and is much
faster than the subsumption check. I've also added a flag to toggle
whether to use this new sorting algorithm (as is done by default) or the
subsumption algorithm. This fixes #14969

I've also added documentation for these new flags and update the
documentation according to the new output.

Reviewers: bgamari, goldfire

Reviewed By: bgamari

Subscribers: simonpj, rwbarton, thomie, carter

GHC Trac Issues: #14969, #14990, #10946

Differential Revision: https://phabricator.haskell.org/D4444

comment:17 Changed 4 months ago by bgamari

Milestone: 8.4.18.6.1

Tritlo, is this now fixed?

comment:18 Changed 4 months ago by Tritlo

No, the initial error still occurs, but the hang introduced by the valid hole fits should no longer be a problem.

comment:19 Changed 3 months ago by bgamari

Milestone: 8.6.18.8.1

This won't be fixed in 8.6. Bumping to 8.8.

Note: See TracTickets for help on using tickets.