On Thu, Sep 29, 2011 at 02:52:26PM +0200, Agata Murawska wrote:
> On Mon, Sep 26, 2011 at 11:57 AM, Iustin Pop <[email protected]> wrote:
> > This replaces the hand-coded opID with one automatically generated
> > from the constructor names, similar to the way Python does it, except
> > it's done at compilation time as opposed to runtime.
> >
> > Again, the code line delta does not favour this patch, but this
> > eliminates error-prone, manual code with auto-generated one; in case
> > we add more opcode support, this will help a lot.
> > ---
> > htools/Ganeti/OpCodes.hs | 7 +------
> > htools/Ganeti/THH.hs | 46
> > +++++++++++++++++++++++++++++++++++++++++++---
> > 2 files changed, 44 insertions(+), 9 deletions(-)
> > -- | Generates a fromString function.
> > @@ -135,7 +144,8 @@ declareSADT :: String -> [(String, Name)] -> Q [Dec]
> > declareSADT sname cons = do
> > let name = mkName sname
> > let ddecl = strADTDecl sname (map fst cons)
> > - tostr <- genToString (toStrName sname) name cons
> > + tostr <- genToString (toStrName sname) name $
> > + map (\(a, b) -> (a, Right b)) cons
> I think I'dput the map into separate variable for readability reasons
Ack. This would be solved nicer with map (second Right) cons, but we
don't use Arrows anywhere, so I'm staying away from it for now.
> > +-- This builds a custom list of name/string pairs and then uses
> > +-- 'genToString' to actually generate the function
> > +genOpID :: Name -> String -> Q [Dec]
> > +genOpID name fname = do
> > + info <- reify name
> > + let TyConI (DataD _ _ _ cons _) = info
> Why not TyConl (DataD _ _ _ cons _) <- reinfy name? You do not seem to
> use info anywhere else..
Good catch. Interdiff:
diff --git a/htools/Ganeti/THH.hs b/htools/Ganeti/THH.hs
index 1650ddc..808836f 100644
--- a/htools/Ganeti/THH.hs
+++ b/htools/Ganeti/THH.hs
@@ -144,8 +144,9 @@ declareSADT :: String -> [(String, Name)] -> Q [Dec]
declareSADT sname cons = do
let name = mkName sname
ddecl = strADTDecl name (map fst cons)
- tostr <- genToString (toStrName sname) name $
- map (\(a, b) -> (a, Right b)) cons
+ -- process cons in the format expected by genToString
+ cons' = map (\(a, b) -> (a, Right b)) cons
+ tostr <- genToString (toStrName sname) name cons'
fromstr <- genFromString (fromStrName sname) name cons
return $ ddecl:tostr ++ fromstr
@@ -219,8 +220,7 @@ constructorName x = fail $ "Unhandled
constructor " ++ show x
-- 'genToString' to actually generate the function
genOpID :: Name -> String -> Q [Dec]
genOpID name fname = do
- info <- reify name
- let TyConI (DataD _ _ _ cons _) = info
+ TyConI (DataD _ _ _ cons _) <- reify name
cnames <- mapM (liftM nameBase . constructorName) cons
let svalues = map (Left . deCamelCase) cnames
genToString (mkName fname) name $ zip cnames svalues
--
iustin
