On Thu, Sep 29, 2011 at 6:10 PM, Iustin Pop <[email protected]> wrote:
> 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 > LGTM
