ikvm icon indicating copy to clipboard operation
ikvm copied to clipboard

Covarient return types mangled by the static compiler cannot be implemented in C#

Open martin-honnen opened this issue 1 year ago • 42 comments

Using IKVM 8.2.1 with .NET 6, I am trying to use C# to implement an abstract class in a Java library that also implements two interfaces in that library. For some reason I don't understand at all currently, VS gives an error like "class Foo does not implement the interface member InterfaceName.methodName()". Of course it offers me to generate the missing code but literally generates some syntactically incorrect construct

           ReturnType InterfaceName.<bridge>methodName()
            {
                throw new NotImplementedException();
            }

The Java interface doesn't have any methods using generics (or any mention of any bridge type).

I am kind of stuck where this lookup for such a method could come from and how I could resolve that or work around the problem.

I understand that probably nobody can help with that short description without seeing a full sample, unless the problem is somehow know.

So my question is, has anyone run into that problem about some spurious interface member with <bridge> being asked for to be implemented? Any ideas how to resolve it?

martin-honnen avatar Sep 20 '22 09:09 martin-honnen

To try to give some sample where the problem occurs, in a project having

 <ItemGroup>
    <PackageReference Include="IKVM.Maven.Sdk" Version="1.0.1" />
    <MavenReference Include="net.sf.saxon:Saxon-HE" Version="11.4" />
  </ItemGroup>

add a class e.g.

using net.sf.saxon.om;
using net.sf.saxon.tree.iter;
using System.Xml;
using JArrayList = java.util.ArrayList;

namespace InterfaceProblemIKVMTest1
{
    sealed class AttributeEnumeration : AxisIterator, LookaheadIterator
    {

        private readonly JArrayList attList = new JArrayList(10); //ArrayList<XmlNode> attList = new ArrayList<>(10);
        private int ix = 0;
        private readonly DotNetNodeWrapper start;
        private DotNetNodeWrapper current;
        private DotNetDocumentWrapper docWrapper;
        private bool disposedValue;

        public AttributeEnumeration(DotNetDocumentWrapper docWrapper, DotNetNodeWrapper start)
        {
            this.docWrapper = docWrapper;
            this.start = start;
            XmlNamedNodeMap atts = start.node.Attributes;
            if (atts != null)
            {
                for (int i = 0; i < atts.Count; i++)
                {
                    string name = atts.Item(i).Name;
                    if (!(name.StartsWith("xmlns") &&
                            (name.Length == 5 || name[5] == ':')))
                    {
                        attList.add(atts.Item(i));
                    }
                }
            }
            ix = 0;
        }

        //@Override
        public bool supportsHasNext()
        {
            return true;
        }

        //@Override
        public bool hasNext()
        {
            return ix < attList.size();
        }

        //@Override
        public NodeInfo next()
        {
            if (ix >= attList.size())
            {
                return null;
            }
            current = makeWrapper((XmlNode)attList.get(ix), docWrapper, start, ix);
            ix++;
            return current;
        }

        Item SequenceIterator.next()
        {
            return ((AxisIterator)this).next();
        }

        public void close()
        {
            SequenceIterator.__DefaultMethods.close(this);
        }

        public void discharge()
        {
            SequenceIterator.__DefaultMethods.discharge(this);
        }

        private void Dispose(bool disposing)
        {
            if (!disposedValue)
            {
                if (disposing)
                {
                    // TODO: Verwalteten Zustand (verwaltete Objekte) bereinigen
                }

                // TODO: Nicht verwaltete Ressourcen (nicht verwaltete Objekte) freigeben und Finalizer überschreiben
                // TODO: Große Felder auf NULL setzen
                disposedValue = true;
            }
        }

        // // TODO: Finalizer nur überschreiben, wenn "Dispose(bool disposing)" Code für die Freigabe nicht verwalteter Ressourcen enthält
        // ~AttributeEnumeration()
        // {
        //     // Ändern Sie diesen Code nicht. Fügen Sie Bereinigungscode in der Methode "Dispose(bool disposing)" ein.
        //     Dispose(disposing: false);
        // }

        public void Dispose()
        {
            // Ändern Sie diesen Code nicht. Fügen Sie Bereinigungscode in der Methode "Dispose(bool disposing)" ein.
            Dispose(disposing: true);
            GC.SuppressFinalize(this);
        }

        //Item AxisIterator.<bridge>next()
        //{
        //    throw new NotImplementedException();
        //}
    }
}

It will not compile (partially because it is not complete, the DotNetNodeWrapper is not provided), but it will show (at least for me here with VS 2022) the mentioned error in the form of "AttributeEnumeration doesn't implement the interface member AxisIterator.<bridge>next()".

martin-honnen avatar Sep 20 '22 10:09 martin-honnen

To answer your first question, this is the first time this issue has been reported AFAIK.

Out of curiosity, will it compile if you create a declaration as follows?


Item AxisIterator.next()
{
    throw new NotImplementedException();
}

NightOwl888 avatar Sep 20 '22 10:09 NightOwl888

No, I get a Compiler Error CS0539: 'AttributeEnumeration.next()' in explicit interface declaration is not a member of interface in that case and some suggestion to "fix it" by inserting that <bridge> stuff again, although already in the suggestion popup that part has a red underline to indicate it doesn't compile.

martin-honnen avatar Sep 20 '22 11:09 martin-honnen

Well, the reason the name is mangled is because it's marked as ACC_BRIDGE in the .class file. Not sure I understand why this code would compile that way in Java, though. Bridge methods are generally used for type erasure.

It's also a default method implementation that functions as a pass through to NodeInfo next(). It's also marked ACC_SYNTHETIC which indicates it's not to be exposed.

Possible that it shouldn't have been exposed on the interface by the static compiler.

wasabii avatar Sep 20 '22 13:09 wasabii

It's most definitely not a pattern I'm familiar with from Java. It looks like the goal is to override and change the return type of an interface method.

from SequenceIterator:

/*@Nullable*/
Item next();

And now AxisIterator:

public interface AxisIterator extends SequenceIterator {
 
@Override
NodeInfo next();
 
}

@Override used on next(), with the same parameter signature, but different return type. The javac compiler appears to generate a default method on AxisIterator, marked with ACC_BRIDGE and ACC_SYNTHETIC, which implements SequenceIterator.next() and invokes this.next(), casting the result.

wasabii avatar Sep 20 '22 13:09 wasabii

@wasabii , thanks for looking into it.

The various classes implementing one abstract class and additionally implementing one or two interfaces where sometimes the method names are the same and the return type is a subtype of the type used in another method has already given me a lot of headache, I couldn't find a way to rely on Java default implementation methods in the interfaces, it appears .NET doesn't recognize them but wants you to implement all interface methods; however, often I could fix that by doing some "reimplementation" like

        public void close()
        {
            SequenceIterator.__DefaultMethods.close(this);
        }

On the issue of that method, do you have any idea on how to get .NET to implement it so that the compilation is possible? I am still kind of lost, all three remaining compilation problems in my attempt to implement various classes show that odd problem.

martin-honnen avatar Sep 20 '22 13:09 martin-honnen

So the magic here is that this is called a "covariant return type".

AxisIterator.next() overrides SequenceIterator.next(), but changes it's return type. This is allowed. And behind the scenes, a bridge method is generated that implements SequenceIterator.next() and does so by invoking AxisIterator.next().

This method is marked with ACC_BRIDGE and ACC_SYNTHETIC.

So how does IKVM handle this?

Well, in the CLR, at least until recently (lots of changes in Core need to be examined), you couldn't simply "delete" an interface method. You have to implement it. So, the old Item next() and NodeItem next() methods must be implemented by all classes extending from AxisIterator. IKVM solves this by actually implementing them automatically in concrete classes it is transpiling. So AxisIterator gets two interface methods. And then IKVM implements them BOTH in the classes that implement them.

But you're trying to implement the interface yourself, so IKVMC isn't around to do it for you. So you have to implement them both. But because they're meant to be hidden, the names are mangled, and you can't in C#.

Not sure how to address yet.

Well, I do have one solution: implement AxisIterator in java. That way ikvmc can generate the implementation for you. And then inherit from that in C#.

wasabii avatar Sep 20 '22 13:09 wasabii

This does open a bunch of interesting questions around the new CLR features supported by Core, and how or whether to capitalize on them in IKVM. Interface methods are now available. And the Proposal explicitly cites Java compatibility as a possible use case for them.

So yeah, we could redo the whole translation thing to use them. No more __DefaultMethods class to hold the implementation. Etc. And it does look like you can put a default implementation on an interface for a method declared by an interface implemented by that interface. So, this whole thing could be smoothed out.

Except we want to continue supporting Framework for the foreseeable future.

Multitargeting could let us use different versions of the implementation for different target frameworks.

But the public API surface would be exposed pretty glaring. __DefaultMethods would need to be used in #ifdef blocks for Framework, by all consumers. But then not in #ifdef COREAPP blocks. That's a pretty nasty infection.

wasabii avatar Sep 20 '22 14:09 wasabii

@martin-honnen

Would you be interested in trying out IKVM.NET.Sdk to do my recommendation? The idea is you can have actual .java source code in a .NET project.

So you could have a project named Saxon.Impl or some such, which contained actual .java files, one of which implemented AxisIterator as an abstract class, providing implementations for next(), which could be abstract, and inherited by your corresponding .NET code.

wasabii avatar Sep 20 '22 14:09 wasabii

@wasabii , I suppose that refers to https://www.nuget.org/packages/IKVM.NET.Sdk/8.2.2-prerelease0241#readme-body-tab? Yes, I will give it a try during this week. Can the Java code refer to .NET code, like ikvmstub compiled code could?

martin-honnen avatar Sep 20 '22 14:09 martin-honnen

Yup. It behaves like a real Net project. With references, nuget, Maven, etc. Prefix native .Net types with cli.

wasabii avatar Sep 20 '22 14:09 wasabii

Another possible solution: why don't we just not mangle the name? Mangling the bridge method name makes sense. But why the interface method?

wasabii avatar Sep 20 '22 15:09 wasabii

Another possible solution: why don't we just not mangle the name? Mangling the bridge method name makes sense. But why the interface method?

Exactly what I was thinking. Covariance on return types of interfaces has been supported in .NET Framework at least since 4.0 (probably long before that). I built an entire application in 2012 that depends on this behavior as a core feature of the design.

It is classes that have recently had support for covariant return types in .NET 5.0. And in that case, it requires the .NET 5.0 runtime in order to work (not just a bump to the C# 9.0 language version), so there would still need to be a workaround for .NET Core prior to that and .NET Framework if it is indeed an issue.

Related: https://github.com/apache/lucenenet/issues/430#issuecomment-783674205

NightOwl888 avatar Sep 20 '22 16:09 NightOwl888

What do you mean exactly? As far as I can tell, covariant return types on interfaces isn't even available in C# 10. Though default methods (including implementing methods) are, which make IKVMs __DefaultMethods class unnecessary.

And none of this works in Framework.

wasabii avatar Sep 20 '22 16:09 wasabii

Consider the following code on .NET 6 C# 10:

    interface I1 { object M(); }
    interface I2 : I1 { int M(); }

This warns with CS0108 that I2.M() hides I1.M().

Additionally, attempting to implement I2 requires that both I1.M and I2.M be implemented by the consuming class.

This is different than in Java, where I2.M() will in fact override I1.M() with a new return type, and thus not require implementing classes to provide an implementation for I1.M().

wasabii avatar Sep 20 '22 17:09 wasabii

This is ultimately what IKVM generates:

    interface I1
    {

        object M();
    }

    interface I2 : I1
    {

        new int M();

        public static class __DefaultMethods
        {
            public static object M(I1 i) => i.M();

        }

    }

    class C : I2
    {

        public int M() => 0;

        object I1.M() => I2.__DefaultMethods.M(this);

    }

The problem here is C. C requires both versions of M() to be provided. But the second can simply dispatch to the __DefaultMethods.M() implementation.

This is fine if IKVM is generating C. It can do all that. But when it's a user (such as the OP) attempting to implement I2 themselves.... it's quite obnoxious. This can be solved by DIM:



    interface I1
    {

        object M();
    }

    interface I2 : I1
    {

        new int M();

        object I1.M() => M();

    }

    class C : I2
    {

        public int M() => 0;

    }

Now C is relatively clean. Requiring only an implementation of int M(). This is exactly what javac does. I2.I1.M() is implemented by this so-called bridge method.

wasabii avatar Sep 20 '22 17:09 wasabii

This is what I mean:

    interface I1 { object M(); }
    interface I2 : I1 { int M(); }


    internal class C : I1, I2
    {
        object I1.M() => new object();
        int I2.M() => 0;
    }

Explicit interface declaration can be used to get different return types depending on the interface that the class instance is cast to.

            I1 x = new C();
            I2 y = x as I2;

            object o1 = x.M();
            int o2 = y.M();

Yes, the caller is expected to implement all interface members, unless using at least C# 8.0 where default interface implementations are now allowed.

However, for a situation like an enumerator, I don't see how that is an issue. All .NET collections implement multiple interfaces to override GetEnumerator() to return multiple enumerator types. So, it isn't unusual at all for this requirement to exist. The only thing that is unusual is the way IKVM seems to name the methods when there are multiple methods that have the same signature with a different return type. We can have multiple methods like this in the same class in C#, but all of them (or all of them but one) need to be explicitly declared to an interface when defining a class.

NightOwl888 avatar Sep 20 '22 17:09 NightOwl888

What you just described isn't "covariant return types". You had to implement the method with the same return type. Now of course you were free to invoke some other version of that method as the implementation: but you did have to implement it. That's not the case in Java. Nor is it the case with "covariant return types" for classes in C# 9.

wasabii avatar Sep 20 '22 17:09 wasabii

Well, the docs say that default method implementations can be overridden, so that is a plus.

However, according to this answer, default interface members are not supported in .NET Framework. Sounds like more #ifdef statements to support such a thing on both Framework and Core.

NightOwl888 avatar Sep 20 '22 17:09 NightOwl888

Right. So now you're caught up.

wasabii avatar Sep 20 '22 17:09 wasabii

@wasabii

Would you be interested in trying out IKVM.NET.Sdk to do my recommendation? The idea is you can have actual .java source code in a .NET project.

I have been trying to create a basic project but I fail to get anything buildable, VS 2022 for all dependencies just show problems through an exclamation mark and any attempt to build fails with an error MSB4057 The target "ResolveIkvmExporter" does not exist in the project. SaxonHE10Net6Test2IKVMSDK C:\Users\marti\.nuget\packages\ikvm.net.sdk\8.2.2-prerelease0396\targets\IKVM.Java.Core.NoTasks.targets 11.

The current setup is a project file

<Project Sdk="IKVM.NET.Sdk/8.2.2-prerelease0396">
  <PropertyGroup>
    <TargetFrameworks>netcoreapp3.1</TargetFrameworks>
  </PropertyGroup>
  <ItemGroup>
    <Compile Remove="JavaClass1.java" />
  </ItemGroup>

  <ItemGroup>
    <PackageReference Include="IKVM.NET.Sdk" Version="8.2.2-prerelease0396" />
  </ItemGroup>

  <!--<ItemGroup>
	<PackageReference Include="IKVM.Maven.Sdk" Version="1.0.1" />
   </ItemGroup>

  <ItemGroup>
	<MavenReference Include="net.sf.saxon:Saxon-HE" Version="10.8" />
  </ItemGroup>-->

  <ItemGroup>
    <None Include="JavaClass1.java" />
  </ItemGroup>

</Project>	

Is there some sample project up somewhere on GitHub or elsewhere online?

martin-honnen avatar Sep 20 '22 21:09 martin-honnen

@martin-honnen First bug that fast, eh.

Add a property <UseIkvmTasks>true</UseIkvmTasks>. Not supposed to be required.

It will take a couple minutes for all of the references to resolve. It has to ikvmstub all of the framework DLLs.

wasabii avatar Sep 20 '22 22:09 wasabii

Also you dont' need to include IKVM.NET.Sdk as a package reference.

wasabii avatar Sep 20 '22 22:09 wasabii

Okay. I misled you again. Do this in your project instead:

<Project>
    <PropertyGroup>
        <UseIkvmTasks>true</UseIkvmTasks>
    </PropertyGroup>
    <Import Sdk="IKVM.NET.Sdk" Version="8.2.2-prerelease0396" Project="Sdk.props"/>
    <PropertyGroup>
        <TargetFrameworks>netcoreapp3.1</TargetFrameworks>
    </PropertyGroup>
    <ItemGroup>
        <None Include="JavaClass1.java" />
    </ItemGroup>
    <Import Sdk="IKVM.NET.Sdk" Version="8.2.2-prerelease0396" Project="Sdk.targets"/>
</Project>

wasabii avatar Sep 20 '22 22:09 wasabii

So I just did all of it locally and it worked. Made a IKVM.NET.Sdk project, added a AxisIteratorBase class, in .java:

import net.sf.saxon.om.NodeInfo;
import net.sf.saxon.tree.iter.AxisIterator;

public abstract class AxisIteratorBase implements AxisIterator
{

    @Override
    public abstract NodeInfo next();

}

And then made a C# project, added a project reference to the Java project, and created AxisIteratorImpl : AxisIteratorBase, and it only requires me to implement the single abstract method. All of the rest is set up properly.

wasabii avatar Sep 20 '22 23:09 wasabii

@wasabii , what is needed in these kind of IKVM.NET.Sdk projects to reference Saxon from Maven? Both

	<ItemGroup>
		<PackageReference Include="IKVM.Maven.Sdk" Version="1.0.1" />
		<MavenReference Include="net.sf.saxon:Saxon-HE" Version="11.4" />
	</ItemGroup>

or just

	<ItemGroup>
		<MavenReference Include="net.sf.saxon:Saxon-HE" Version="11.4" />
	</ItemGroup>

martin-honnen avatar Sep 20 '22 23:09 martin-honnen

The former. IKVM.Maven.Sdk isn't automatically part of IKVM.NET.Sdk.

wasabii avatar Sep 20 '22 23:09 wasabii

IKVM.NET.Sdk/8.2.2-prerelease0553 should have all those fixes applied, so no need to have UseIkvmTasks, or inline the SDK references.

wasabii avatar Sep 21 '22 03:09 wasabii

@wasabii , I am now trying to use 8.2.2-prerelease0553 with Java files of some larger project, any idea why I would get the following errors related to Java 8 JRE/JDK classes?

1>MSBUILD : error compiler.err.cant.access: cannot access java.util.Map.Entry
1>MSBUILD : error compiler.err.cant.access:   class file for java.util.Map$Entry not found
1>MSBUILD : error compiler.err.cant.access: cannot access java.util.stream.Collector.Characteristics
1>MSBUILD : error compiler.err.cant.access:   class file for java.util.stream.Collector$Characteristics not found
1>MSBUILD : error compiler.err.cant.resolve.location: cannot find symbol
1>MSBUILD : error compiler.err.cant.resolve.location:   symbol:   class Entry
1>MSBUILD : error compiler.err.cant.resolve.location:   location: interface java.util.Map
1>MSBUILD : error compiler.err.cant.access: cannot access javax.xml.datatype.DatatypeConstants.Field
1>MSBUILD : error compiler.err.cant.access:   class file for javax.xml.datatype.DatatypeConstants$Field not found
1>MSBUILD : error compiler.err.cant.resolve.location: cannot find symbol
1>MSBUILD : error compiler.err.cant.resolve.location:   symbol:   class Field
1>MSBUILD : error compiler.err.cant.resolve.location:   location: class javax.xml.datatype.DatatypeConstants
1>MSBUILD : error compiler.err.cant.access: cannot access java.util.Builder
1>MSBUILD : error compiler.err.cant.access:   bad class file: C:\.virtual-ikvm-home\assembly\IKVM.Java__8.2.0.0__13235d27fcbfff58\classes\java\util\Builder.class
1>MSBUILD : error compiler.err.cant.access:     unable to access file: File not found.
1>MSBUILD : error compiler.err.cant.access:     Please remove or make sure it appears in the correct subdirectory of the classpath.

msbuildproj has e.g.

<Project Sdk="IKVM.NET.Sdk/8.2.2-prerelease0553">
    <PropertyGroup>
        <TargetFrameworks>netcoreapp3.1</TargetFrameworks>
    </PropertyGroup>

martin-honnen avatar Sep 24 '22 14:09 martin-honnen

Hmm. I do not. Just to verify I wasn't talking out my ass, I whipped up another quick IKVM.NET.Sdk project, and made sure it can still build stuff.... and it did.

The error you're seeing is the compiler trying to load the class stubs for various class files out of the VFS...... which shouldn't be environment specific in anyways. This is on Windows, right?

Any chance you can post your project file somewhere so I can look at it directly?

wasabii avatar Sep 24 '22 14:09 wasabii