microsoft / bond

Bond is a cross-platform framework for working with schematized data. It supports cross-language de/serialization and powerful generic mechanisms for efficiently manipulating data. Bond is broadly used at Microsoft in high scale services.
MIT License
2.61k stars 322 forks source link

[gbc] Generic structs erroneously permit creation of bonded fields with non-struct types #430

Open chwarr opened 7 years ago

chwarr commented 7 years ago

The following .bond file does not fail during code generation. It ends up creating, in Invalid_UsesString, a field of type bonded<string>, which technically is not permitted. The generated C++ code will fail to compile, and the generated C# code will throw an exception during creation of the Serializer.

namespace b;

struct HasBonded<T>
{
    0: bonded<T> bf;
}

struct Empty { }

struct Valid_UsesEmpty : HasBonded<Empty> { }

struct Invalid_UsesString : HasBonded<string> { }

The current expectation is that for bonded<T>, the T is a struct itself.

In this particular case, the compiler can see all of the definitions and should be able to fail this. More complicated cases involving imports and forward declarations also need to be considered.

Bonus points for generating code in each of the languages that would prevent in-language instantiation of a generic struct with a non-struct type.

aschmahmann commented 7 years ago

I imagine this issue was created in response to #428. Is the position being taken is that bonded<T> really shouldn't work on "primitive" Bond types like int, String, vector<Y> (for all Y)? Is this out of design principle, or because it appears to be too hard/time consuming to fix?

Since it seems pretty reasonable that people might want to have bonded<vector<T>> or generally just have a generic bonded<T> (e.g. just passing through generic data, or for use in an inheritance like scheme) perhaps one of the following solutions would seem acceptable:

  1. Allow bonded<T> to work on "primitive" Bond types (seems like probably the most straightforward)
  2. Make analogous Bond types for each primitive type so that this problem (and #151) go away
  3. Since people will likely want to use bonded<T> Bond can just have a generic wrapper class that developers are supposed to know to use on string, int, vector<T>, etc.

Answer 3 is basically what we have now, but if this is the "supported" solution then maybe there should be a standard wrapper. I believe answer 1 is the most work out of the three but still significantly less work than what would be required to fully implement the behavior you have described above.

In short, why not just make bonded<T> work on primitive types?

chwarr commented 7 years ago

Yes, this was opened during my writing of a reply to #428. :-) The "meat" of the issue is over there: this is a bug fix to make today's Bond easier to use. If anyone is interested in discussion about what it would mean to serialize a non-struct type, head on over to #428.