1. 17 Oct, 2019 1 commit
    • Eric Myhre's avatar
      gen: rearrange files. · 68b3383d
      Eric Myhre authored
      The previous names started to seem more questionable as we start
      generating additional code which is natively typed rather than
      interface constrained.  Now the latter stuff is in a file that
      includes 'Node' as part of the filename.  This seems better.
      68b3383d
  2. 01 Sep, 2019 2 commits
    • Eric Myhre's avatar
      Add exported accessors for builders + reprbuilders · dc19057d
      Eric Myhre authored
      I'm not sure if I like these symbol names.  Or rather, I don't,
      but haven't decided on what would be preferable yet.
      
      There's a couple of options that have come to mind:
      
      - option 1
        - `func {{ .Type.Name }}__NodeBuilder() ipld.NodeBuilder`
        - `func {{ .Type.Name }}__ReprBuilder() ipld.NodeBuilder`
      
      - option 2
        - `func NewBuilderFor{{ .Type.Name }}() ipld.NodeBuilder`
        - `func NewReprBuilderFor{{ .Type.Name }}() ipld.NodeBuilder`
      
      - option 3
        - `func (Builders) {{ .Type.Name }}() ipld.NodeBuilder`
        - `func (ReprBuilders) {{ .Type.Name }}() ipld.NodeBuilder`
      
      Option 3 would make 'Builders' and 'ReprBuilders' effectively reserved
      as type names if you're using codegen.  Schemas using them could use
      adjunct config specific to golang to rename things out of conflict in
      the generated code, but it's still a potential friction.
      
      Option 2 would also have some naming collision hijinx to worry about,
      on further though.  Only Option 1 is immune, by virtue of using "__"
      in combination with the schema rule that type names can't contain "__".
      
      This diff is implementing Option 1.  I think I'm partial to Option 3,
      but not quite confident enough in it to lunge for it yet.
      
      Putting more methods on the *concrete* types would also be another
      interesting fourth option!  These methods would ignore the actual
      value, and typically be used on the zero value: e.g., usage would
      resemble `Foo{}.ReprBuilder()`.
      The upside of this would be we'd then have no package scoped exported
      symbols except exactly the set matching type names in the schema.
      However, the opportunities for confusion with this would be numerous:
      we couldn't use the 'NodeBuilder' method name (because that's the
      potentially-stateful/COW one), but would still be returning a
      NodeBuilder type?  Etc.  Might not be good.
      
      More to think about here in the future.
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      dc19057d
    • Eric Myhre's avatar
      DRYing some type ident munges. · a84c7ec1
      Eric Myhre authored
      I'm still aiming to keep this as simple and un-clever as possible,
      because putting lipstick on a pig -- this is all about to become
      strings which get shoveled back to a compiler parser anyway -- is
      not useful, and becomes antiuseful if it obstructs readability...
      
      But I'm starting to find these elements are repeated enough that
      it will help rather than hurt readability to extract some things.
      
      Also, since the munges have recently started to appear in both go code
      source as well as in the templates, that starts to put more weight in
      favor of extracting a function for it, which keeps the two syntactic
      universes from drifting on this subject.
      
      At the same time, moved all NodeBuilders to being unexported (by using
      idents prefixed with a "_").  I looked at the godoc for the generated
      code and felt this is looking like a wiser choice than exporting.
      
      We'll need to export more methods for getting initial instances of the
      now-unexported stuff... but we should be adding those anyway, so this
      is not an argument against unexporting.
      
      Some additional type idents around iterators and map builders have not
      yet been hoisted to DRYed munge methods.  I'm debating if that's useful
      (as you can see in the comments in that file), but leaning towards
      it being more parsimoneous to just go for it.  So that'll probably be
      the next commit.
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      a84c7ec1
  3. 26 Aug, 2019 1 commit
    • Eric Myhre's avatar
      Fix generality of codegen helper mixins. · 5042c7e4
      Eric Myhre authored
      So far everything we've used these mixins for was generating based on
      a *schema.Type*.  This isn't true anymore: representation nodes are
      still nodes, so most of the same helpers are useful for the same
      reasons and we want to reuse them... but then now they're not always
      directly related to a particular reified schema.Type anymore.
      
      Correspondingly, these helper templates were using the .Type.Kind
      property as a shortcut to avoid more args, but that is wrong now:
      for example, if we're going to generate the representation node for
      a struct with a stringjoin representation, the closest thing we would
      have to a schema.Type is the struct; but that kind clearly isn't right.
      So, now that is another property passed internally to the embeddable
      helpers (the helper-helpers, I guess?), and affixed by the helper
      rather than provided by a schema.Type param (which, again, we no
      longer have).
      
      Note for more future work: this is the first time that a "TypeIdent"
      property has shown up explicitly, but it's also *all over* in the
      templates in a defacto way.  So far, my policy has been that extracting
      consts from the templates isn't a priority until it proves it has a
      reason to also be handled *outside* the immediate locality of the
      templates (because if we pulled *everything* out the templates will
      become a thin unreadable soup; and doing bulk sed-like edits to the
      templates is fairly easy as long as they're consistent).  Now, that
      criteria is probably satisfied, so more refactors on this are probably
      due very soon.
      
      And one more further sub-note on that note: I'm not actually sure if
      some of these types should have a "_" prefix on them or not.
      "$T_NodeBuilder" currently doesn't -- meaning it's exported -- and that
      might not be desirable.  I started them that way, so for now I'm just
      sticking with it, but it's a thing that deserves consideration.
      (It might be nice for godoc readability if there's a clear hint that
      the builders exist; but also, at present, there's no guarantee that
      their zero values are at all usable, and that should be cause for
      concern.  Other considerations may also exist; I haven't attepmted to
      weigh them all yet.)
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      5042c7e4
  4. 13 Aug, 2019 1 commit
    • Eric Myhre's avatar
      codegen: nodebuilderGenerator now in flight. · 8940f0be
      Eric Myhre authored
      String and Struct generation now uses it.
      
      Implemented the whole kinded rejection helper embeddables pattern,
      as already proven out to work reasonable well to DRY the generation
      code we needed for the nodes.  Working well here too.
      
      Struct NodeBuilder has still got several todo's in it -- this diff is
      already big enough before finishing that implementation -- but it
      at least fully fleshes out the interfaces now, which is progress.
      
      Some comments indicating how many more of these we're going to need.
      (Several.)
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      8940f0be