ValidateInterfaceImplTable enforces bogus ordering rules

[MichalStrehovsky]

Description

System.Reflection.Metadata metadata writer enforces weird rules around metadata validity for the InterfaceImpl table.

Reproduction Steps

It's not possible to generate a module with the metadata that corresponds to the following C# program:

interface I1
{
    void Method();
}

interface I2
{
    void Method();
}

class C1 : I1, I2
{
    public void Method() { }
}

class C2 : I2, I1
{
    public void Method() { }
}

Expected behavior

It's possible to generate interface list for I1, I2 and I2, I1.

Actual behavior

InvalidOperationException is thrown out of this code because it's impossible to have the interface list sorted both ways:

runtime/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Tables.cs

Lines 1552 to 1576 in 57bfe47

	private void ValidateInterfaceImplTable()
	{
	if (_interfaceImplTable.Count == 0)
	{
	return;
	}
	InterfaceImplRow current, previous = _interfaceImplTable[0];
	for (int i = 1; i < _interfaceImplTable.Count; i++, previous = current)
	{
	current = _interfaceImplTable[i];
	if (current.Class > previous.Class)
	{
	continue;
	}
	if (previous.Class == current.Class && current.Interface > previous.Interface)
	{
	continue;
	}
	Throw.InvalidOperation_TableNotSorted(TableIndex.InterfaceImpl);
	}
	}

Interface index should not be looked at by this code.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

[ghost]

Tagging subscribers to this area: @buyaa-n
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

System.Reflection.Metadata metadata writer enforces made up rules around metadata validity for the InterfaceImpl table.

Reproduction Steps

It's not possible to generate a module with the metadata that corresponds to the following C# program:

interface I1
{
    void Method();
}

interface I2
{
    void Method();
}

class C1 : I1, I2
{
    public void Method() { }
}

class C2 : I2, I1
{
    public void Method() { }
}

Expected behavior

It's possible to generate interface list for I1, I2 and I2, I1.

Actual behavior

InvalidOperationException is thrown out of this code because it's impossible to have the interface list sorted both ways:

runtime/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataBuilder.Tables.cs

Lines 1552 to 1576 in 57bfe47

	private void ValidateInterfaceImplTable()
	{
	if (_interfaceImplTable.Count == 0)
	{
	return;
	}
	InterfaceImplRow current, previous = _interfaceImplTable[0];
	for (int i = 1; i < _interfaceImplTable.Count; i++, previous = current)
	{
	current = _interfaceImplTable[i];
	if (current.Class > previous.Class)
	{
	continue;
	}
	if (previous.Class == current.Class && current.Interface > previous.Interface)
	{
	continue;
	}
	Throw.InvalidOperation_TableNotSorted(TableIndex.InterfaceImpl);
	}
	}

Interface index should not be looked at by this code.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

Author:	MichalStrehovsky
Assignees:	-
Labels:	area-System.Reflection.Metadata
Milestone:	-

[agocke]

This is the same as dotnet/roslyn#3905, but it was incorrectly thought to be a Roslyn bug, when it is in fact a SRM and spec bug -- I think ECMA's restriction here is too strict.

[MichalStrehovsky]

Ah, didn't see ECMA calls for it. Such rule doesn't make sense - the interface ordering affects resolution rules (see II.12.2: - "Where there are multiple implementations for a given interface method due to differences in type
parameters, the declaration order of the interfaces on the class determines which method is invoked, as
well as the order in which the methods are declared"). The file format restriction would make it impossible to order things in a way the code intends.

Removing that line from the spec should go to the ECMA-335 addendums doc in this repo.