Skip to content
This repository has been archived by the owner on Aug 8, 2024. It is now read-only.

[reflection] Changes in Type.cs, TypeTests.cs and TypeTests.netcoreapp.cs #185

Merged
merged 2 commits into from
Nov 15, 2018

Conversation

MaximLipnin
Copy link

@MaximLipnin MaximLipnin commented Nov 15, 2018

Relates to mono/mono#11665.

The changes:

… from referencesource; disable some test cases in TypeTests.cs and TypeTests.netcoreapp.cs
@@ -23,7 +23,11 @@ public abstract partial class Type : MemberInfo, IReflect
public abstract string FullName { get; }

public abstract Assembly Assembly { get; }
#if MONO
public override abstract Module Module { get; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep the comment from the refsrc code:

Suggested change
public override abstract Module Module { get; }
// Workaround for JIT bug with bad VTable lookup
//
// MemberInfo mi = typeof (System.DBNull);
// System.Console.WriteLine (mi.Module);
//
public override abstract Module Module { get; }

I assume you checked if this bug still exists?

Copy link
Author

@MaximLipnin MaximLipnin Nov 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any failures related to removing this workaround. I left it only due to potential API change. If that API change is OK then I can revert it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be fine to remove the workaround.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@marek-safar marek-safar merged commit 806496e into mono:master Nov 15, 2018
@MaximLipnin MaximLipnin deleted the fix_typetests branch November 15, 2018 12:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants