Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use arshal flags instead of coder flags #100

Merged
merged 1 commit into from
Dec 30, 2024
Merged

Conversation

dsnet
Copy link
Collaborator

@dsnet dsnet commented Dec 28, 2024

Instead of consulting the flags on the coder via:

export.Encoder(enc).Flags
export.Decoder(dec).Flags

consult flags on the call-provided arshal options via:

mo.Flags
uo.Flags

This avoids unnecessarily peaking at the internals flags
of Encoder or Decoder and also allows us to simplify
some flag checking logic.

For example, the following are all equivalent:

export.Encoder(enc).Flags.Get(Foo) || mo.Flags.Get(Bar)
mo.Flags.Get(Foo) || mo.Flags.Get(Bar)
mo.Flags.Get(Foo | Bar)

A similar transform can also be done by applying
De Morgan's law to the following:

!export.Encoder(enc).Flags.Get(Foo) && !mo.Flags.Get(Bar)
!mo.Flags.Get(Foo) && !mo.Flags.Get(Bar)
!!(!mo.Flags.Get(Foo) && !mo.Flags.Get(Bar))
!(mo.Flags.Get(Foo) || mo.Flags.Get(Bar))
!(mo.Flags.Get(Foo | Bar))
!mo.Flags.Get(Foo | Bar)

There should be no behavior changes as a result of this
since the arshal option flags should be an exact
superset of the coder option flags.

There are 6 entry points to the "json" package:

  • Marshal
  • MarshalWrite
  • MarshaEncode
  • Unmarshal
  • UnmarshalRead
  • UnmarshalDecode

4 of them (Marshal, MarshalWrite, Unmarshal, UnmarshalRead)
obtain a coder from an internal pool,
where the arshal options plumbed down the call stack
is a pointer to the one inside the coder.
Therefore, there is no behavior difference for these 4 calls.

2 of them (MarshalEncode, UnmarshalDecode) take in a
user-provided coder, where there could theoretically
be a difference between the options struct and the coder options.
However, in both functions, we obtain a jsonopts.Struct locally
from an internal pool and then call jsonopts.Struct.CopyCoderOptions
where the coder options are copied from the user-provided coder
into the local jsonopts.Struct. Thus, the options struct that
we plumb down the stack is gauranteed to be a superset of
the options in the user-provided coder.

@dsnet dsnet force-pushed the cleanup-arshal-flags branch 3 times, most recently from ef2766b to 3259446 Compare December 28, 2024 19:19
Instead of consulting the flags on the coder via:

	export.Encoder(enc).Flags
	export.Decoder(dec).Flags

consult flags on the call-provided arshal options via:

	mo.Flags
	uo.Flags

This avoids unnecessarily peaking at the internals flags
of Encoder or Decoder and also allows us to simplify
some flag checking logic.

For example, the following are all equivalent:

	export.Encoder(enc).Flags.Get(Foo) || mo.Flags.Get(Bar)
	mo.Flags.Get(Foo) || mo.Flags.Get(Bar)
	mo.Flags.Get(Foo | Bar)

A similar transform can also be done by applying
De Morgan's law to the following:

	!export.Encoder(enc).Flags.Get(Foo) && !mo.Flags.Get(Bar)
	!mo.Flags.Get(Foo) && !mo.Flags.Get(Bar)
	!!(!mo.Flags.Get(Foo) && !mo.Flags.Get(Bar))
	!(mo.Flags.Get(Foo) || mo.Flags.Get(Bar))
	!(mo.Flags.Get(Foo | Bar))
	!mo.Flags.Get(Foo | Bar)

There should be no behavior changes as a result of this
since the arshal option flags should be an exact
superset of the coder option flags.

There are 6 entry points to the "json" package:
*	Marshal
*	MarshalWrite
*	MarshaEncode
*	Unmarshal
*	UnmarshalRead
*	UnmarshalDecode

4 of them (Marshal, MarshalWrite, Unmarshal, UnmarshalRead)
obtain a coder from an internal pool,
where the arshal options plumbed down the call stack
is a pointer to the one inside the coder.
Therefore, there is no behavior difference for these 4 calls.

2 of them (MarshalEncode, UnmarshalDecode) take in a
user-provided coder, where there could theoretically
be a difference between the options struct and the coder options.
However, in both functions, we obtain a jsonopts.Struct locally
from an internal pool and then call jsonopts.Struct.CopyCoderOptions
where the coder options are copied from the user-provided coder
into the local jsonopts.Struct. Thus, the options struct that
we plumb down the stack is gauranteed to be a superset of
the options in the user-provided coder.
@dsnet dsnet force-pushed the cleanup-arshal-flags branch from 3259446 to 23b16f8 Compare December 28, 2024 19:25
@dsnet dsnet merged commit 460a147 into master Dec 30, 2024
8 checks passed
@dsnet dsnet deleted the cleanup-arshal-flags branch December 30, 2024 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants