abiggerhammer / hammer

Parser combinators for binary formats, in C. Yes, in C. What? Don't look at me like that.
GNU General Public License v2.0
285 stars 72 forks source link

confusing points about token types #19

Open pete- opened 10 years ago

pete- commented 10 years ago

A couplefew things:

I'm happy to write code for any/all of this, but I don't want to start digging in without some discussion/approval from the powers-that-be. =)

pete- commented 10 years ago

Also, the existence of all the macros that shield users from having to worry about the arena allocator makes it difficult to see how one would create user-defined tokens while still keeping p->arena duly hidden. Dems sum inconsistencies and I likes me sum consistency.

thequux commented 10 years ago
  1. The macros were never intended to hide functionality; they were always intended to handle common cases with fewer tokens. I.e., we found that code was easier to read with fewer references to p->arena.
  2. Token creation happens in a critical path (parse tree creation), so performance is somewhat more critical than it would otherwise be. Having to stash token IDs somewhere else doesn't seem that bad, particularly considering that canonical token names will probably be fairly long. In fact, even if we did have an H_MAKE_USER macro that takes a string, I would want there to be some facility for adding aliases, at which point we're just reimplementing the host language.
  3. Point noted regarding TT_. Do you want to send that PR or should I?
abiggerhammer commented 10 years ago

In general I'm in favour of a consistency-oriented rewrite. Pete, how about you do one like you're envisioning, and send along a PR?

pete- commented 10 years ago

@thequux

re: 2. Precisely. That's what I was trying to get at with my fourth bullet point, though it almost certainly could have been phrased better. I'll see if I can come up with something both intuitive and efficient.

re: 3. I'll take care of it.

@abiggerhammer

On it.