Opened 14 months ago

Closed 6 months ago

Last modified 6 months ago

#7667 closed bug (fixed)

Template Haskell fails to recognize type operator/function +

Reported by: andygill Owned by:
Priority: normal Milestone: 7.8.1
Component: Template Haskell Version: 7.6.2
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: GHC rejects valid program Difficulty: Unknown
Test Case: th/T7667 th/T7667a Blocked By:
Blocking: Related Tickets:

Description

The following message is issued for a valid TH program.

Main.hs:7:1:
    Illegal type constructor or class name: `+'
    When splicing a TH declaration:
      type instance GHC.TypeLits.+ 1 2 = 3
Failed, modules loaded: Test1.

Code attached.

The program is attempting to capture the name +,
as used by Nat at the type level.

The problem appears to be in Convert.hs

 -- Convert.hs

     okOcc :: OccName.NameSpace -> String -> Bool
     okOcc _ [] = False 
     okOcc ns str@(c:_) | OccName.isVarNameSpace ns = startsVarId c || startsVarSym c
                        | otherwise = startsConId c || startsConSym c || str == "[]"

+ is rejected, by okOcc, even though it is acceptable, the symbol neither starts with upper-case, or ':'.

I have tried using reify to extract the *actual* name from other sources (rather than use mkNameG_tc), and it fails in the same way.

Attachments (4)

Test1.hs (360 bytes) - added by andygill 14 months ago.
Main.hs (97 bytes) - added by andygill 14 months ago.
0001-Added-test-for-7667.patch (1.2 KB) - added by goldfire 6 months ago.
0001-Fix-Trac-7667.patch (2.7 KB) - added by goldfire 6 months ago.

Download all attachments as: .zip

Change History (19)

Changed 14 months ago by andygill

Changed 14 months ago by andygill

comment:1 Changed 14 months ago by simonpj

  • Difficulty set to Unknown

OK so we could test flags etc make okOcc accept the type operator. Or alternatively we could simply omit the test, which would allow TH to generate names that the programmer could not write. In fact it currently only checks the name space; that is, checks that if the name claims to be a data constructor then it starts with an uppper case letter or colon. But it does not check the name is a legal one; it could be the data constructor C$$ for example, which the programmer can't write.

I'm inclined to go the whole way, and simply treat the name space in the TH name as authoritative, regardless of the string that used for the name.

I'll that unless someone yells.

Simon

Last edited 6 months ago by simonpj (previous) (diff)

comment:2 Changed 12 months ago by igloo

  • Milestone set to 7.8.1

See also #7484

comment:3 Changed 6 months ago by goldfire

I've fixed this bug, and will attach a patch shortly, along with a testsuite patch. I still can't validate (see #8438), but I would be grateful if someone could include this with a validate run and push. Thanks!

comment:4 Changed 6 months ago by goldfire

  • Status changed from new to patch

Changed 6 months ago by goldfire

Changed 6 months ago by goldfire

comment:5 Changed 6 months ago by Krzysztof Gogolewski <krz.gogolewski@…>

In 798d0a62899ed05054ef348f3851f129de8c9a9e/ghc:

Fix Trac #7667.

We no longer check capitalization (or colons) in names that come
from TH, according to the commentary in #7667.

comment:6 Changed 6 months ago by Krzysztof Gogolewski <krz.gogolewski@…>

comment:7 Changed 6 months ago by monoidal

  • Resolution set to fixed
  • Status changed from patch to closed
  • Test Case set to th/T667

Applied, thanks!

comment:8 Changed 6 months ago by goldfire

  • Test Case changed from th/T667 to th/T7667

Thanks for validating!

comment:9 Changed 6 months ago by goldfire

  • Resolution fixed deleted
  • Status changed from closed to new

I would like to reinstate this check, having just spent several hours trying to track down a place where I had used VarE in some code where I should have used ConE. If the namespace check had been on, the error would have been very easy to spot. Instead, I just got "out of scope" errors for things that clearly were in scope. (Turned out, I had GHC looking in the wrong scope.)

A smaller change than the one Simon proposed (that is, "remove the check") would be just to remove the requirement for colons at the beginning of a type symbol. Is there anything else that should be changed here?

comment:10 Changed 6 months ago by simonpj

OK with me

comment:11 Changed 6 months ago by Richard Eisenberg <eir@…>

In ba6308ece51fbd86f7a0281c223dc49c2f73531a/ghc:

Refine fix for #7667.

Now, we allow types that do not begin with ':', but we retain other
checks on variable names.

comment:12 Changed 6 months ago by Richard Eisenberg <eir@…>

In 1270e252f6bc9eff140a78ec6b27472d2c373fef/testsuite:

Add new test for the second round of #7667

comment:13 Changed 6 months ago by goldfire

  • Resolution set to fixed
  • Status changed from new to closed
  • Test Case changed from th/T7667 to th/T7667 th/T7667a

comment:14 Changed 6 months ago by Richard Eisenberg <eir@…>

In bb9d53e32da52221173431733928c497187686ff/ghc:

Fix Trac #7667.

We no longer check capitalization (or colons) in names that come
from TH, according to the commentary in #7667.

comment:15 Changed 6 months ago by Richard Eisenberg <eir@…>

In ddf61ce990f551ae451432d6dae03abd1efb972b/ghc:

Revert "Fix Trac #7667.", restoring the refined fix.

Apologies -- there was some git confusion that cause the Wrong Thing
to happen.

This reverts commit bb9d53e32da52221173431733928c497187686ff.

Conflicts:

	compiler/hsSyn/Convert.lhs
Note: See TracTickets for help on using tickets.