1. 16 Aug, 2021 1 commit
  2. 29 Jul, 2021 1 commit
  3. 16 Dec, 2020 1 commit
    • Daniel Martí's avatar
      all: rewrite interfaces and APIs to support int64 · f6e9a891
      Daniel Martí authored
      We only supported representing Int nodes as Go's "int" builtin type.
      This is fine on 64-bit, but on 32-bit, it limited those node values to
      just 32 bits. This is a problem in practice, because it's reasonable to
      want more than 32 bits for integers.
      
      Moreover, this meant that IPLD would change behavior if built for a
      32-bit platform; it would not be able to decode large integers, for
      example, when in fact that was just a software limitation that 64-bit
      builds did not have.
      
      To fix this problem, consistently use int64 for AsInt and AssignInt.
      
      A lot more functions are part of this rewrite as well; mainly, those
      revolving around collections and iterating. Some might never need more
      than 32 bits in practice, but consistency and portability is preferred.
      Moreover, many are interfaces, and we want IPLD interfaces to be
      flexible, which will be important for ADLs.
      
      Below are some GNU sed lines which can be used to quickly update
      function signatures to use int64:
      
      	sed -ri 's/(func.* AsInt.*)\<int\>/\1int64/g' **/*.go
      	sed -ri 's/(func.* AssignInt.*)\<int\>/\1int64/g' **/*.go
      	sed -ri 's/(func.* Length.*)\<int\>/\1int64/g' **/*.go
      	sed -ri 's/(func.* LookupByIndex.*)\<int\>/\1int64/g' **/*.go
      	sed -ri 's/(func.* Next.*)\<int\>/\1int64/g' **/*.go
      	sed -ri 's/(func.* ValuePrototype.*)\<int\>/\1int64/g' **/*.go
      
      Note that the function bodies, as well as the code that calls said
      functions, may need to be manually updated with the integer type change.
      That cannot be automated, because it's possible that an automated fix
      would silently introduce potential overflows not being handled.
      
      Some TODOs and FIXMEs for overflow checks are removed, since we remove
      some now unnecessary int64->int conversions. On the other hand, the
      older codecs based on refmt need to gain some overflow check TODOs,
      since refmt uses ints. That is okay for now, since we'll phase out refmt
      pretty soon.
      
      While at it, update codectools to use int64 for token Length fields, so
      that it properly supports full IPLD integers without machine-dependent
      behavior and overflow checks. The budget integer is also updated to be
      int64, since the lengths it uses are now int64.
      
      Note that this refactor needed changes to the Go code generator as well
      as some of the tests, for the purpose of updating all the code.
      
      Finally, note that the code-generated iterator structs do not use int64
      fields internally, even though they must return int64 numbers to
      implement the interface. This is because they use the numeric fields to
      count up to a small finite amount (such as the number of fields in a Go
      struct), or up to the length of a map/slice. Neither of them can ever
      outgrow "int".
      
      Fixes #124.
      f6e9a891
  4. 24 Feb, 2020 2 commits
    • Eric Myhre's avatar
      Addntl comments on PathSegment equality. · a48c9db9
      Eric Myhre authored
      a48c9db9
    • Eric Myhre's avatar
      Fix for empty string PathSegment. · 98fdaf17
      Eric Myhre authored
      Yep, it makes a lot of creation by struct uglier, as you can see in the
      diff to the test file.  Fortunately?  Nobody outside the package is
      allowed to do that anyway, so, we can pretty much ignore the ergonomic.
      (It'll still look uglier if someone's looking at stuff with spew or
      go-cmp or some other library like that which peeks naughtily into
      unexported fields, but that's hard to do much about.)
      
      We could also address this by adding a discriminant field to the
      PathSegment struct.  (If we wanted to use uint internally, we'd
      probably have to do that, in fact.  Either that or start using
      some other alarmingly magical number like UINT_MAX.  Ygh.)  I didn't.
      I don't think we handle lists larger than 2 billion elements
      particularly well anyway... so, adding more words of memory to
      PathSegment to support that case seems like an entirely losing trade.
      98fdaf17
  5. 23 Feb, 2020 3 commits
    • Eric Myhre's avatar
      Document an already true fact about PathSegment's zero value. · 2d590abe
      Eric Myhre authored
      I... honestly hadn't even realized this before; I just noticed it
      when making a refactor that would produce this end state, and thought
      "oh, that's silly".  Evidently... it's fine, since this has already
      been the case, and not been problematic!  Alright then.
      Apparently it's not too silly after all.
      2d590abe
    • Eric Myhre's avatar
      Spelling fix. · 0f7de4a9
      Eric Myhre authored
      0f7de4a9
    • Eric Myhre's avatar
      Several significant clarifications to Path docs. · 2e1845b3
      Eric Myhre authored
      Primarily, being commital and direct about how special values, erm...
      *aren't* -- namely, "/" is a valid segment, and so is empty string --
      and confessing how much tricky work that makes.
      
      Several methods are now more upfront about how much they *do not do
      good things* on such values.
      
      Added some new constructors for Path which *do* work for all possible
      values.  (You could've handled such tricky values with a series of
      Join calls, previously, but that would be ergonomically grueling.)
      
      These facts are all defacto 'true' to the best of my knowledge now.
      However, the IPLD specs repo is a bit on the quiet side about them
      at present (precisely because it's such an irritating little nest of
      edge cases and fun stuff)... and so the comments are also littered
      with "this may change" warnings.  Still, it's better to be accurate
      about what the code does and does not do in its current state.
      
      I'd *like* to formally specify an escaping system and canonical
      string encoding for paths.  That should start in the IPLD Specs
      repo, though, and involve fixtures, so I won't start it here now.
      
      Thanks to @ribasushi for the kick in the shins that these docs needed
      work and clarifications.
      2e1845b3
  6. 30 Aug, 2019 1 commit
    • Eric Myhre's avatar
      Move selector.PathSegment up to ipld.PathSegment. · fc1f83d7
      Eric Myhre authored
      ipld.Path is now a slice of ipld.PathSegment instead of strings.
      
      This should be an improvement in sanity: there are now several fewer
      places importing "strconv", and that's just always a good thing.
      
      We will also be free in the future to add PathSegment-based accessor
      methods to ipld.Node, as has already been commented as a desire;
      and, to use PathSegment in building better typed errors
      (which is the specific thing that provokes this diff today and now).
      
      The implementation of PathSegment is now also based on a struct-style
      union rather than an interface style union.  There are comments about
      this in the diff.  I do not wish to comment on how much time I've spent
      looking at golang assembler and runtime internals while trying to find
      a path to a more perfect compromise between ergonomics and performance.
      tl;dr Selectors will probably get faster and trigger fewer allocations;
      ipld.Path will probably take slightly more memory (another word per
      path segment), but not enough to care about for any practical purposes.
      
      I did not attempt to hoist the SegmentIterator features from the
      selector package to anywhere more central.
      It may be a fine idea to do so someday; I just don't presently have
      a formed opinion and am not budgeting time to consider it today.
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      fc1f83d7