Skip to content
  • Iustin Pop's avatar
    Cleanup THH function use from built module namespace · 32a569fe
    Iustin Pop authored
    
    
    Currently, THH.hs "injects" into the built code names of library
    functions like Text.JSON.makeObj, Ganeti.JSON.fromObj, etc. built
    directly from strings, via (e.g.)
    
      varE (mkName "makeObj")
    
    This means that the "makeObj" name must exist in the target module,
    i.o.w. must be imported there. This leads to the strange case of
    having to have imports that do not appear at all in the used
    (template) code, but are needed to satisfy this "hidden" dependency;
    look at Ganeti/Jobs.hs before this patch, for example.
    
    This is also not very obvious, because we usually import Text.JSON
    anyway; I only stumbled upon it while doing some cleanup work.
    
    So to clean this up, the current patch changes the THH.hs to use not
    string-derived, but identifier-derived names («'identifier» versus
    «mkName "identifier"»); this is better, as the names must be
    resolvable when compiling THH itself (once), and not when compiling
    the multiple derived modules. As you can see, this allows removal of
    extraneous imports from various modules.
    
    Background information: an `mkName "foo"` results in a name of flavour
    NameS (“An unqualified name; dynamically bound”) or alternatively to a
    qualified name, but still dynamically bound. Whereas what we want is a
    statically bound name: `'foo` results in a NameG flavour, “Global name
    bound outside of the TH AST: An original name”.
    
    One more explanation: the change is similar to going from 'x = eval
    "map"' to 'x = map'; the name is no longer dynamically evaluated, but
    statically when the module is compiled. In our case, previously names
    were bound at target module compile time, now they are bound at THH.hs
    compile time.
    
    Signed-off-by: default avatarIustin Pop <iustin@google.com>
    Reviewed-by: default avatarGuido Trotter <ultrotter@google.com>
    32a569fe