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

NamedType: add defaults and make sure state stays valid #193

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

tomjnixon
Copy link
Member

I think the first part of this is fairly obvious and worth merging soon, but i'm not sure about the move stuff.

getNamedTypeDefault is different from getDefault for two reasons:

  • it performs a different function; getNamedTypeDefault is just to make default-constructed values valid, whereas getDefault is used for ADM defaults. Sometimes these are the same, but not always.
  • the default implementation of these needs to be different: the one used by NamedType needs to default-construct the underlying value, while the one for auto base needs to default-construct the final value (which may or may not be a NamedType). If these both had the same behaviour they would either result in infinite recursion, or would only work on NamedTypes. It would be possible to implement this with partial specialisation, but then can't be done with function templates.

I considered having getNamedTypeDefault return the underlying value, bu this is a bit messier to write (more types, and you need to specify the tag).

I'm not sure if the defaults should be inlined or not. They seem like a good thing to inline, it's really part of the interface, and most types will use the default implementation, which will be inlined anyway.

The main reason i'm not sure about the move stuff is that the new T get() && will sometimes get called when you call .get() on a temporary, which is not ideal because it results in a call to getNamedTypeDefault and the validator. This means that it is absolutely necessary to implement getNamedTypeDefault for types with validators which don't accept default-constructed values, as otherwise .get() can throw a cryptic error.

these allow updating the value without running the validator
the mess here is to make sure that the new value is valid without
changing the old value, to make sure that the value stays valid even when
an exception is thrown

fixes #192
In this case the value in the NamedType needs to be left valid, and this
adds some overhead: constructing the new value is offset by the overhead
saved by the move, but running the validator can be expensive.
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.

1 participant