Biohazrd icon indicating copy to clipboard operation
Biohazrd copied to clipboard

Add support for mixin transformations

Open PathogenDavid opened this issue 3 years ago • 1 comments

One thing that became apparent during #40 is that there may be room for mixin-style transformations.

Right now constant array transformation happens as part of CSharpTypeReductionTransformation. This was done because it should happen during type reduction since the types of the constant array elements need to be reduced too (although #64 somewhat nerfed that.) However, it makes sense that someone might not want the built-in constant array transformation and would want to provide their own. (For instance: The built-in solution assumes unmanaged types will never go onto the manage heap, which some people might not like. Or some people might prefer C# fixed arrays in places where they can be used.)

In the context of persistent transformations, it makes sense that you might want transformations to happen between a few transformations.

We should provide a special Mixin-type transformation which essentially delegates the transformation to multiple child transformations with the transform methods implemented something like this:

protected virtual TypeTransformationResult TransformClangTypeReference(TypeTransformationContext context, ClangTypeReference type)
{
    foreach (TypeTransformationBase child in children)
    {
        TypeTransformationResult result = child.TransformClangTypeReference(context, type);

        if (result.IsChange(type))
        { return result; }
    }

    return type;
}

PathogenDavid avatar Sep 25 '20 17:09 PathogenDavid

Another place where this might be nice to have is the verification stage. CSharpTranslationVerifier is slowly becoming a mess of unrelated concerns, it might be nice to try and break it up into distinct steps.

However other than this and the situation mentioned above, I haven't found myself wanting/needing this much. I'm thinking it might be better to special-case these two situations rather than trying to come up with a generalized mixin transformation API.

Also dumping down the prioritization on this since realistically this has not been a large issue.

PathogenDavid avatar Feb 15 '22 00:02 PathogenDavid