search

zserge / jsmn

Jsmn is a world fastest JSON parser/tokenizer. This is the official repo replacing the old one at Bitbucket
MIT License
3.72k stars 783 forks source link

Make jsmntype_t values bit flags #108

Closed olmokramer closed 3 years ago

olmokramer commented 7 years ago

This way one can easily check whether a token is either of multiple types, e.g.:

if (token.type & (JSMN_ARRAY | JSMN_OBJECT))
    // do stuff with array or object
olmokramer commented 7 years ago

Oops, my bad! Guess that's what you get for doing a pull request through github's online interface... Thanks for spotting, fixed it in 2nd commit.

pt300 commented 5 years ago

I still think it's an interesting concept that's worth considering. It can't be pulled at this point but I'll keep this open just so I won't easily forget about it.

olmokramer commented 5 years ago

Don't know why the PR couldn't be merged anymore... Fixed the "conflicts" so it can be pulled again.

pt300 commented 5 years ago

This goes into my bucket of things to actually add to jsmn. Thanks for the fix

olmokramer commented 5 years ago

Awesome, thanks! Didn't actually change anything in the merge commit I think...

PS Oh wait, looks like indentation whitespace has changed.

alexhenrie commented 3 years ago

I want this feature too, but please leave JSMN_UNDEFINED as 0, otherwise if (token->type) will no longer be the same as if (token->type != JSMN_UNDEFINED). The definition of jsmntype_t should be:

typedef enum {
  JSMN_UNDEFINED = 0,
  JSMN_OBJECT = 1 << 0,
  JSMN_ARRAY = 1 << 1,
  JSMN_STRING = 1 << 2,
  JSMN_PRIMITIVE = 1 << 3
} jsmntype_t;
alexhenrie commented 3 years ago

I want this feature because on some architectures (such as ARM), less instructions are required for if (token->type & (JSMN_ARRAY | JSMN_PRIMITIVE)) than for if (token->type == JSMN_ARRAY || token->type == JSMN_PRIMITIVE). Redefining jsmntype_t to resemble a set of bit flags will allow code that uses it to take advantage of that shortcut.

olmokramer commented 3 years ago

@alexhenrie that's a good point about leaving JSMN_UNDEFINED = 0. I've reverted the change to JSMN_UNDEFINED.

pt300 commented 3 years ago

Something similar is already available in the experimental branch, @alexhenrie. Due to extremely slow development there I will just merge this pull request with the main branch.

I do not see any possible incompatibilities at this point. This definition of jsmntype is compatible with any usage I'm aware of and also brings in possibility to test more efficiently for multiple types.

pt300 commented 3 years ago

There are no issues introduced based on the tests either.

alexhenrie commented 3 years ago

Thank you very much!

aloisklink commented 1 year ago

I do not see any possible incompatibilities at this point. This definition of jsmntype is compatible with any usage I'm aware of and also brings in possibility to test more efficiently for multiple types.

The only possible incompatibility is that this is an ABI breaking change. But since jsmn is a header-only library, I'm not really sure if ABI breaking changes matter? :shrug:

Still, it might be worth making the next release of jsmn a v2.0.0 release, just to be on the safe side. It looks like there are some OpenSUSE packages that are bundling jsmn is a pre-compiled binary, so otherwise they might be affected, see https://pkgs.org/search/?q=jsmn.