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

gbc shouldn't allow struct fields to have default of nothing #164

Closed
chadwalters opened this issue May 10, 2016 · 6 comments
Closed

gbc shouldn't allow struct fields to have default of nothing #164

chadwalters opened this issue May 10, 2016 · 6 comments
Labels

Comments

@chadwalters
Copy link
Contributor

chadwalters commented May 10, 2016

gbc shouldn't allow struct fields to have default of nothing. The following schema definition should fail:

struct A
{
    0: int32 x;
}

struct B
{
    0: A a = nothing;
}
@chwarr
Copy link
Member

chwarr commented May 10, 2016

Related: #72, #73, #128.

@chwarr chwarr added the bug label May 19, 2016
@lalo
Copy link
Member

lalo commented Jun 15, 2016

So basically struct fields can never have a default value?

This is the only valid syntax?

struct A
{
    0: int32 x;
}

struct B
{
    0: A a;
}

@chadwalters
Copy link
Contributor Author

Correct; struct fields can't have default values.

lalo added a commit that referenced this issue Jun 17, 2016
- Fail when struct field has default value of 'nothing'
- Fail when enum field doesn't have default value
- Validate aliases' default value
- Validate default value type mistmatches
- Validate default value out-of-range values

Fixes #177, fixes #164, fixes #128, fixes #73, fixes #72
lalo added a commit that referenced this issue Jun 22, 2016
- Validate default value type mistmatches (fixes #72, fixes #128)
- Validate default value out-of-range values (fixes #73)
- Fail when struct field has default value of 'nothing' (fixes #164)
- Fail when enum field doesn't have default value (fixes #177)
- Validate default value of type aliases
@lalo lalo closed this as completed in 90b8bad Jun 22, 2016
@chwarr
Copy link
Member

chwarr commented Aug 23, 2016

Bond version 4.3.0 is now live on NuGet.org. These changes are included in that version.

@clarkrd
Copy link

clarkrd commented Aug 31, 2016

A default value of nothing allowed an optional struct field omitted by the sender to deserialize to nothing. Doesn't this mean that an optional struct field omitted by the sender will now automatically deserialize into an instance with all its default field value set? That seems like a major and breaking change. Is there a way to specify that an optional struct field should simply deserialize to nothing if omitted? This was the previous compiler behavior and it had a very clear and useful semantic.

Does this mean that we need to make all such struct fields nullable in the schema in order to avoid serializing / deserializing default values for the struct? This requires major changes to existing schemas to get back the previous behavior support by allowing nothing as a default value for a struct.

@chwarr
Copy link
Member

chwarr commented Sep 1, 2016

Thanks for bringing this up, @clarkrd, and sorry about the churn.

Doesn't this mean that an optional struct field omitted by the sender will now automatically deserialize into an instance with all its default field value set?

Yes, that is what will happen (and what was supposed to happen before: read on).

That seems like a major and breaking change. ... This was the previous compiler behavior and it had a very clear and useful semantic.

The compiler was never supposed to allow struct fields to be = nothing. That it allowed it was a bug.

Why was it a bug? Because of the history of Bond before it went open source... There is a Microsoft-internal, older version of Bond. The internal version of Bond has been around for many, many years. The older compiler is a completely different code base from the current one. It never allowed struct fields to be = nothing. So, from this perspective, the = nothing behavior of gbc is a regression.

At the moment, the various teams at Microsoft are migrating from the old, internal version to gbc. During this transition, we need to make it possible to use the same .bond file with both compilers.

However, there are interesting possibilities for struct fields with a default value of nothing. I've opened #226, "Consider adding support for struct fields to have a default value of nothing" to explore these possibilities.

Is there a way to specify that an optional struct field should simply deserialize to nothing if omitted?

If the semantic of the field is that sometimes the producer wants to include it, but not always, then nullable<> is how you should model that. It indicates the explicit ability to "not have a value". Let's say that = nothing support were added/restored. You still wouldn't be able to assign null and be guaranteed to serialize the value. Serialization to Simple Binary, for example, would fail, as Simple Binary cannot omit any fields and the value of null for an = nothing field doesn't have a serializable representation, where as null does have a serializable representation for a nullable<> field.

Does this mean that we need to make all such struct fields nullable in the schema in order to avoid serializing / deserializing default values for the struct?

That is a way to do it. Another way is to look at a field in the inner struct and check whether it has it default value--assuming the default value is not meaningfully used, which is sometimes the case.

I understand that this change might need some schema adjustments. If anyone needs help, feel free to open a new issue with details about the schemas in question, and we'll see what we can come up with that will be minimally disruptive. And, if we can't, we'll need to figure out a different way forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants