Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#10047 closed bug (fixed)

inconsistency in name binding between splice and quasiquotation

Reported by: rwbarton Owned by: spinda
Priority: normal Milestone: 8.0.1
Component: Template Haskell Version: 7.8.4
Keywords: Cc: slyfox
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case: th/T10047
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D1199
Wiki Page:

Description

Let me preface this by saying that this may not be a bug. If not then it would be nice if the documentation for Template Haskell could clarify what's going on here.

My understanding of quasiquotation is that a quasiquote [n|foo|] is equivalent to a splice $(quoteExp n "foo"). However, that is not the case in all contexts.

module Q where

import Language.Haskell.TH
import Language.Haskell.TH.Quote

n = QuasiQuoter { quoteExp = dyn }
rwbarton@morphism:/tmp$ ghci -XTemplateHaskell -XQuasiQuotes Q
GHCi, version 7.8.3: http://www.haskell.org/ghc/  :? for help
Loading package ghc-prim ... linking ... done.
Loading package integer-gmp ... linking ... done.
Loading package base ... linking ... done.
[1 of 1] Compiling Q                ( Q.hs, interpreted )

Q.hs:6:5: Warning:
    Fields of ‘QuasiQuoter’ not initialised: quotePat, quoteType,
                                             quoteDec
    In the expression: QuasiQuoter {quoteExp = dyn}
    In an equation for ‘n’: n = QuasiQuoter {quoteExp = dyn}
Ok, modules loaded: Q.
*Q> :t [| $(dyn "foo") |]
[| $(dyn "foo") |] :: ExpQ
*Q> :t [| [n|foo|] |]
Loading package pretty-1.1.1.1 ... linking ... done.
Loading package array-0.5.0.0 ... linking ... done.
Loading package deepseq-1.3.0.2 ... linking ... done.
Loading package containers-0.5.5.1 ... linking ... done.
Loading package template-haskell ... linking ... done.

<interactive>:1:7:
    Not in scope: ‘foo’
    In the Template Haskell quotation [| [n|foo|] |]

Why do these behave differently?

(Lastly, the link to the paper "Why It's Nice to be Quoted: Quasiquoting for Haskell" in the User's Guide at https://downloads.haskell.org/~ghc/latest/docs/html/users_guide/template-haskell.html#th-quasiquotation is broken. Does this paper have a permanent home? In any case, I only skimmed it but it didn't seem to answer my question.)

Change History (21)

comment:1 Changed 3 years ago by Simon Peyton Jones <simonpj@…>

In f46360ed7139ff25741b381647b0a0b6d1000d84/ghc:

Refactor the handling of quasi-quotes

As Trac #10047 points out, a quasi-quotation [n|...blah...|] is supposed
to behave exactly like $(n "...blah...").  But it doesn't!  This was outright
wrong: quasiquotes were being run even inside brackets.

Now that TH supports both typed and untyped splices, a quasi-quote is properly
regarded as a particular syntax for an untyped splice. But apart from that
they should be treated the same.  So this patch refactors the handling of
quasiquotes to do just that.

The changes touch quite a lot of files, but mostly in a routine way.
The biggest changes by far are in RnSplice, and more minor changes in
TcSplice.  These are the places where there was real work to be done.
Everything else is routine knock-on changes.

* No more QuasiQuote forms in declarations, expressions, types, etc.
  So we get rid of these data constructors
    * HsBinds.QuasiQuoteD
    * HsExpr.HsSpliceE
    * HsPat.QuasiQuotePat
    * HsType.HsQuasiQuoteTy

* We get rid of the HsQuasiQuote type altogether

* Instead, we augment the HsExpr.HsSplice type to have three
  consructors, for the three types of splice:
    * HsTypedSplice
    * HsUntypedSplice
    * HsQuasiQuote
  There are some related changes in the data types in HsExpr near HsSplice.
  Specifically: PendingRnSplice, PendingTcSplice, UntypedSpliceFlavour.

* In Hooks, we combine rnQuasiQuoteHook and rnRnSpliceHook into one.
  A smaller, clearer interface.

* We have to update the Haddock submodule, to accommodate the hsSyn changes

comment:2 Changed 3 years ago by simonpj

Milestone: 7.12.1
Resolution: fixed
Status: newclosed
Test Case: th/T10047

Very good bug report thank you. Now properly fixed.

It's a big-ish change, and includes a simplification in the hooks API, so probably not for 7.10.

Simon

comment:3 Changed 3 years ago by nomeata

Did you forget to add ./th/T10047.run.stderr? I observe

Actual stderr output differs from expected:
--- /dev/null	2014-07-11 16:48:13.679453102 +0200
+++ ./th/T10047.run.stderr	2015-02-10 19:42:50.001947139 +0100
@@ -0,0 +1,6 @@
+
+T10047.hs:6:5: Warning:
+    Fields of ‘QuasiQuoter’ not initialised: quotePat, quoteType,
+                                             quoteDec
+    In the expression: QuasiQuoter {quoteExp = dyn}
+    In an equation for ‘n’: n = QuasiQuoter {quoteExp = dyn}
*** unexpected failure for T10047(ghci)

on https://raw.githubusercontent.com/nomeata/ghc-speed-logs/master/f46360ed7139ff25741b381647b0a0b6d1000d84.log

comment:4 Changed 3 years ago by Simon Peyton Jones <simonpj@…>

In 1d982ba10f590828b78eba992e73315dee33f78a/ghc:

Do not complain about missing fields in Trac #10047

comment:5 Changed 2 years ago by spinda

This has broken the name resolution behavior for quasiquoters. Previously, this was valid (assuming "wow" is a quasiquoter producing a declaration):

{-# LANGUAGE QuasiQuotes #-}

thing = okay
[wow|stuff|]
okay = 3

but now it produces an error:

/tmp/runghcXXXX1804289383846930886.hs:3:9: error:
    Not in scope: ‘okay’

because after this change, quasiquoters share the declaration order restrictions of splices.

I have code that depends on the previous behavior, and I'm sure there's a lot more code out there that does as well, as the lack of this restriction was one of the main advantages of using quasiquoters over regular splices.

Last edited 2 years ago by spinda (previous) (diff)

comment:6 Changed 2 years ago by spinda

Resolution: fixed
Status: closednew

comment:7 Changed 2 years ago by rwbarton

spinda: indeed, your example hints at a simpler manifestation of the bug; when wow runs in your program, thing is not accessible to reify (not in scope), whereas if you'd written a splice with $(...), it would be in scope.

Sorry to hear that your programs were broken by this fix, but can't you just move the quasiquotations upward in your file so that they aren't in the middle of any recursive definition groups (e.g., to the top of the file)?

I'm pretty sure the main advantage of quasiquoters was supposed to be the succinct syntax, not an undocumented difference in scoping rules. If there's a real use case for a type of splice with different scoping, it doesn't particularly make language design sense to tie that difference to quasiquotes vs. traditional splices.

comment:8 Changed 2 years ago by simonpj

Reid is, as usual, spot on. The big deal about quasiquotes is that they save you writing $(wow "blah"). It's interesting that the (entirely accidental) change in scoping is "one of the main advantages of using quasi-quoters". Can you say why it's so important?

Quasi-quoters may use anti-quotation:

xs = blah
[wow| funny language `(reverse xs)` blah |]

Here wow might use back-quotes to trigger anti-quotation, and then use reify to look up reverse and xs. So we'd need them to be in scope.

It's not ridiculous to propose the scoping you want for declaration splices; it could be something like

  • bring all the binders into scope (thing and okay in your example)
  • run the quasi-quote
  • splice it in

But someone would need to work out the details. Eg if there were two quasiquotes, what would each see in its reification environment.

Simon

comment:9 Changed 2 years ago by spinda

Firstly, at least as far as I've seen, the scoping restriction is one of the most common complaints about Template Haskell splices, as using them at all suddenly causes order of declaration to matter where it didn't before. This contributes to the stigma and avoidance of Template Haskell usage. It would be a shame to see quasiquoters get shackled with this issue as well.

Then, I think it's important to consider the role of quasiquoters as enabling extensions to the language without requiring compiler plugins, external preprocessors, or what have you, while keeping them contained from the rest of the Haskell source around them. As a specific example, I'll pull from my current GSoC project, which is impacted by this change (and, to be honest, is the reason I ran into this).

I have an lq quasiquoter which allows for LiquidHaskell type signatures and specifications to be attached to variables and types. In the declaration context, it parses a subset of Haskell with LiquidHaskell extensions and emits declarations and annotations. Take some vanilla Haskell code:

module Test () where

type Nat = Int

add :: Nat -> Nat -> Nat
add x y = id' $ x + y

id' :: a -> a
id' x = x

A little contrived, but not too far from what you'd run into in sufficiently complex real-world projects. Under the current (or, previous) quasiquoter implementation, extending this existing code with custom annotations is a fairly straightforward translation:

{-# LANGUAGE QuasiQuotes #-}

module Test () where

import LiquidHaskell

[lq| type Nat = { v:Int | 0 <= v } |]

[lq| add :: Nat -> Nat -> Nat |]
add x y = id' $ x + y

[lq| id' :: x:a -> { v:a | v == a } |]
id' x = x

But after this change, introducing these annotations suddenly makes order of declaration matter. What were lightweight inline extensions to the language now require restructuring of code, either reordering the functions themselves or moving all signatures and specifications to the top of every file. Needless to say, this makes the whole thing much less attractive.

And, frankly, this is what quasiquoters are all about: lightweight, inline language extensions that don't interfere with the rest of the code. This intent is reflected in the original paper. With this restriction imposed, anything using quasiquoters suddenly brings in a lot more baggage than it used to, discouraging use. It's not just a matter of modifying some existing code to fit, it's that this hampers a whole set of use-cases for which quasiquoters (a) used to fit quite nicely and (b) are the only real solution at present.

Quietly breaking this behavior of 12 years now in a tangentially related bugfix strikes me as, well, wrong, especially when there isn't an alternative available. Excuse me if I seem rather passionate about this issue.

comment:10 Changed 2 years ago by spinda

Sorry, I should clarify that I'm not completely opposed to change here, as long as a viable alternative that preserves these use cases makes it in at the same time.

Last edited 2 years ago by spinda (previous) (diff)

comment:11 Changed 2 years ago by spinda

Quasi-quoters may use anti-quotation:

xs = blah
[wow| funny language `(reverse xs)` blah |]

Is this a new feature? It doesn't seem to be present in 7.10.1.

(Apologies for the multiple consecutive replies.)

Last edited 2 years ago by spinda (previous) (diff)

comment:12 Changed 2 years ago by spinda

re: what multiple declaration splices would see in their reification environments, the current behavior for quasiquoters seems to be to evaluate declaration quasiquoters in the order of appearance in the source. So decalarations produced by a declaration splice further up the file would make it into the reification environments of subsequent declaration splices. Declarations produced by splices lower down would be invisible to ones further up. This seems reasonable to me, as long as the scoping restriction is contained within the TH/quasiquoter processing and doesn't leak out over the rest of the source.

I'm likely overlooking a deeper issue here, so please let me know what else would need to be addressed.

re: anti-quotation in quasiquoters, I think I misunderstood simonpj's comment the first time around. If I understand correctly now, this isn't referring to some new native support for antiquotation, but rather an example of how a quasiquoter would come to use reify.

comment:13 Changed 2 years ago by slyfox

Cc: slyfox added

I skimmed through testsuite failures on full validate today and found qq007 and qq008 failures:

qq007 $ cat QQ.hs 
{-# LANGUAGE TemplateHaskell #-}
module QQ where

import Language.Haskell.TH.Quote
import Language.Haskell.TH

pq = QuasiQuoter { quoteDec = \_ -> [d| f x = x |], 
                   quoteType = \_ -> [t| Int -> Int |],
                   quoteExp = \_ -> [| $(varE (mkName "x")) + 1::Int |],
                   quotePat = \_ -> [p| Just x |] }
qq007 $ cat Test.hs 
{-# LANGUAGE QuasiQuotes #-}
module Test where

import QQ

f :: [pq| foo |]    -- Expands to Int -> Int
[pq| blah |]        -- Expands to f x = x

h [pq| foo |] = f [pq| blah |] * 8
        -- Expands to h (Just x) = f (x+1) * 8
qq007 $ "/home/slyfox/dev/git/ghc/inplace/bin/ghc-stage2" --make  Test -fforce-recomp -dcore-lint -dcmm-lint -dno-debug-output -no-user-package-db -rtsopts -fno-warn-tabs -fno-ghci-history  -v0

Test.hs:6:1: error:
    The type signature for f lacks an accompanying binding

Reid suggested this fix likely caused the change. Is it fine/expected?

Last edited 2 years ago by slyfox (previous) (diff)

comment:14 Changed 2 years ago by spinda

Regarding what I wrote earlier about the splicing restriction:

There's an essential conflict between having module-local declarations in scope for the reification environment and avoiding the splitting of declaration groups on splices. For some splices, such as makeLenses, reification takes precedence. But not all splices need to reify module-local declarations: for those, giving that up would be worth avoiding the splicing restriction.

In 7.10, the first use case for splices is covered by the $(...) syntax, and the second by quasiquoters. After this change, only the first is represented.

This can be rectified by providing some way to mark splices as falling into the second category, then processing them along with the other declarations in the group instead of splitting on them. To accomplish this, an additional $$(...) syntax (or similar) could be added. In the top-level declaration context, these splices would be marked as not to be split on. For consistency, the same syntax would yield the same result as $(...) in all other contexts (types, etc).

Quasiquoters would then gain a similar additional syntax, [:name| ... |] or such. Since this would actually emulate the current behavior of quasiquoters, perhaps the effect should be reversed here for compatibility, with the second syntax enabling splitting. I'm not sure.

Alternately, we could forgo the additional syntax and simply mark all splices arising from quasiquoters as not causing a split. This would accomplish the end goal of preserving compatibility.

comment:15 in reply to:  13 Changed 2 years ago by goldfire

Replying to slyfox:

I skimmed through testsuite failures on full validate today and found qq007 and qq008 failures:

These failures are indeed due to this ticket -- we need to fix them up. But first, we have to decide what to do with this ticket in general.

I have to say I find @spinda's arguments convincing. The idea that [q|blah|] is identical to $(quoteDec q "blah") is nice, but perhaps there is a reason for two separate mechanisms here. The splitting that @spinda is so worried about happens only for declaration splice/quasiquotes, so we could have the splices/quasiquote consistency for other contexts. With some careful documentation in the manual, I think it's not hard for users to understand this difference.

comment:16 Changed 2 years ago by Thomas Miedema <thomasmiedema@…>

In cbb4d788/ghc:

Testsuite: mark qq007 and qq008 expect_broken(#10047)

This fixes the wrong ticket number in
16a87397295fa92bcbe7a2c6277f938622b93969 (#10181).

comment:17 Changed 2 years ago by goldfire

See #5463 for another scenario where having flexibility around the issue discussed here would be helpful.

comment:18 Changed 2 years ago by spinda

Differential Rev(s): Phab:D1199
Owner: set to spinda

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

In c8f623e3/ghc:

Expand declaration QQs first (#10047)

Declaration QuasiQuoters do not cause a group split like $(...)
splices, and are run and expanded before other declarations in
the group.

Resolves the lingering issue with #10047, and fixes broken tests
qq007 and qq008.

Test Plan: validate

Reviewers: goldfire, austin, bgamari

Reviewed By: bgamari

Subscribers: goldfire, simonpj, thomie, spinda

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

GHC Trac Issues: #10047

comment:20 Changed 2 years ago by bgamari

Resolution: fixed
Status: newclosed

comment:21 Changed 2 years ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

Note: See TracTickets for help on using tickets.