Add external attribute

[jamescourtney]

@joncham -- this is what I was thinking for fs_extern. LMK if you think this has a chance to solve your problems.

[jamescourtney]

I'm not 100% convinced that we should open this up to everything right now, but it may make sense for Value structs. Tables and Ref Structs are a bit picky with the constructors they want.

[codecov[bot]]

Codecov Report

Merging #322 (2e37991) into main (7993a58) will increase coverage by 0.03%. The diff coverage is 98.03%.

@@            Coverage Diff             @@
##             main     #322      +/-   ##
==========================================
+ Coverage   95.59%   95.62%   +0.03%     
==========================================
  Files         103      103              
  Lines        7941     8027      +86     
  Branches      735      748      +13     
==========================================
+ Hits         7591     7676      +85     
  Misses        243      243              
- Partials      107      108       +1     

Impacted Files	Coverage Δ
...atSharp.Compiler/SchemaModel/PropertyFieldModel.cs	95.92% <66.66%> (ø)
...tSharp.Compiler/SchemaModel/FlatSharpAttributes.cs	98.86% <90.00%> (-1.14%)	:arrow_down:
src/FlatSharp.Compiler/FlatSharpCompiler.cs	92.52% <100.00%> (+0.69%)	:arrow_up:
src/FlatSharp.Compiler/Helpers.cs	55.55% <100.00%> (+5.55%)	:arrow_up:
src/FlatSharp.Compiler/MetadataKeys.cs	100.00% <100.00%> (ø)
.../FlatSharp.Compiler/SchemaModel/BaseSchemaModel.cs	96.29% <100.00%> (+0.46%)	:arrow_up:
.../FlatSharp.Compiler/SchemaModel/EnumSchemaModel.cs	97.50% <100.00%> (+0.06%)	:arrow_up:
...ompiler/SchemaModel/FlatSharpAttributeValidator.cs	98.75% <100.00%> (+0.08%)	:arrow_up:
...Compiler/SchemaModel/MutableFlatSharpAttributes.cs	87.50% <100.00%> (+0.54%)	:arrow_up:
...Compiler/SchemaModel/ReferenceStructSchemaModel.cs	92.10% <100.00%> (ø)
... and 5 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7993a58...2e37991. Read the comment docs.

[jamescourtney]

I'm not 100% convinced that we should open this up to everything right now, but it may make sense for Value structs. Tables and Ref Structs are a bit picky with the constructors they want.

I have verified that this works:

attribute "fs_serializer";
attribute "fs_nonVirtual";
attribute "fs_valueStruct";
attribute "fs_unsafeStructVector";
attribute "fs_nonVirtual";
attribute "fs_sortedVector";
attribute "fs_external";
attribute "fs_preserveFieldCasing";

namespace System.Numerics;

// could also define it as data : [ float : 3 ] since X, Y, and Z are not used. Note that
// using [ ubyte : 12 ] wouldn't work because the alignment would be wrong for the floats, which need
// to be 4-byte aligned.
struct Vector3 (fs_external, fs_preserveFieldCasing, fs_valueStruct)
{
    X : float32;
    Y : float32;
    Z : float32;
}

namespace BenchmarkCore;

table Table (fs_serializer) {
    V3 : System.Numerics.Vector3;
}

[joncham]

I have tried this branch locally and it seems to provide the functionality we need. I can define the following (adjusted from your example above):

attribute "fs_serializer";
attribute "fs_nonVirtual";
attribute "fs_valueStruct";
attribute "fs_unsafeStructVector";
attribute "fs_nonVirtual";
attribute "fs_sortedVector";
attribute "fs_external";
attribute "fs_preserveFieldCasing";

namespace UnityEngine;

// could also define it as data : [ float : 3 ] since X, Y, and Z are not used. Note that
// using [ ubyte : 12 ] wouldn't work because the alignment would be wrong for the floats, which need
// to be 4-byte aligned.
struct Vector3 (fs_external, fs_preserveFieldCasing, fs_valueStruct)
{
    x : float32;
    y : float32;
    z : float32;
}

namespace BenchmarkCore;

table Table (fs_serializer) {
    V3 : UnityEngine.Vector3;
}

And the generated C# will compile within Unity.

I think limiting to value structs is acceptable. @vvuk ?

[vvuk]

I think limiting to value structs is acceptable.

Absolutely.

[jamescourtney]

That's good news. I was doing extern and realized "Hey, I think I did the feature".

There are a couple of things to keep in mind about my implementation here:

• Little endian only (Assertion will fire for BE)
• Size at runtime must match size at compile time (Also an assertion)

At least in .NET 6, the value of these assertions are known at JIT time, so they are removed entirely if true.

Additionally, one thing I did notice is that for whatever reason when running under .NET 6, using the built-in Vector3 instead of a custom struct is actually much slower, possibly due to AVX registers or the CPU downclocking. I hope that isn't the case for your toolchain, but it might merit a quick check.

[jamescourtney]

@joncham and @vvuk -- the only concern I have about this PR beyond a little additional testing I want to do is to make sure that you're good not having the ability to use generics. Something like System.Numerics.Vector<byte> won't be possible here because that's not a valid name in FlatBuffers. We could solve it by making fs_extern take a string that specifies the full name. Thoughts?

However, Vector<byte> is also probably a bad example since its size varies by machine.

[joncham]

• Little endian only (Assertion will fire for BE)

I think little endian only is acceptable.

• Size at runtime must match size at compile time (Also an assertion)

This also makes sense.

[joncham]

@joncham and @vvuk -- the only concern I have about this PR beyond a little additional testing I want to do is to make sure that you're good not having the ability to use generics. Something like System.Numerics.Vector<byte> won't be possible here because that's not a valid name in FlatBuffers. We could solve it by making fs_extern take a string that specifies the full name. Thoughts?

@jamescourtney I think the flexibility would be useful, but I'm not sure I have a use case for it at the moment until I do some further integration and use of FlatSharp at Unity.

[jamescourtney]

@joncham and @vvuk -- the only concern I have about this PR beyond a little additional testing I want to do is to make sure that you're good not having the ability to use generics. Something like System.Numerics.Vector<byte> won't be possible here because that's not a valid name in FlatBuffers. We could solve it by making fs_extern take a string that specifies the full name. Thoughts?

@jamescourtney I think the flexibility would be useful, but I'm not sure I have a use case for it at the moment until I do some further integration and use of FlatSharp at Unity.

This turns out to be a bit more difficult than I anticipated. Basically, the problem with generics is that there are really two definitions: how you define it, and how you use it. This is not a legal definition:

public struct Vector<byte> { .. }

So, we'd have to ask for two pieces of data: both the generic definition and the usage definition, which means our attribute would need to look something like fs_extern:"genericDef=Vector<T>;usage=Vector<byte>", which is so ugly I'm not convinced that that's actually the right thing to do. I also definitely don't want to be in the business of parsing out type names. Let me know if you have any suggestions on how to resolve this one.