Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Fixing the Facade's double typeforward to CultureTypes #6899

Merged
merged 1 commit into from
Aug 24, 2016

Conversation

joperezr
Copy link
Member

@joperezr joperezr commented Aug 24, 2016

When merging #6694 CultureTypes was moved from internal to public because it was needed by one of the methods of CultureInfo. CultureTypes was already being type-forwarded in the facade project before GenFacades even ran, so after GenFacades this was causing the facade to have two different typeforwards to CultureTypes. Because of this, Microsoft.CCI was basically off by one in the type table ref, which was causing all nested types to have the wrong forwarder.

cc: @weshaggard @danmosemsft @stephentoub
FYI: @hann013 @sepidehms @tarekgh

@@ -5730,7 +5730,7 @@ public partial class CultureInfo : System.ICloneable, System.IFormatProvider
public static System.Globalization.CultureInfo GetCultureInfo(string name) { throw null; }
public static System.Globalization.CultureInfo GetCultureInfo(string name, string altName) { throw null; }
public static System.Globalization.CultureInfo GetCultureInfoByIetfLanguageTag(string name) { throw null; }
public static System.Globalization.CultureInfo[] GetCultures(System.Globalization.CultureTypes types) { throw null; }
Copy link
Member

Choose a reason for hiding this comment

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

could you add a small comment telling why we are commenting this API and the enum? and we need to open issue tack understanding what is going on.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I'll let the builds finish first to make sure my CI is green, and then I'll add the comment and merge

Copy link
Member Author

Choose a reason for hiding this comment

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

No need anymore, found the right fix.

@tarekgh
Copy link
Member

tarekgh commented Aug 24, 2016

LGTM.

@joperezr
Copy link
Member Author

Actually I just figured out what the root cause is and how to get an easy fix, I'll update this PR now. @tarekgh with this new fix, CultureTypes will be public again.

@tarekgh
Copy link
Member

tarekgh commented Aug 24, 2016

@weshaggard do we know why we have these type forward for the following?

[assembly: TypeForwardedTo(typeof(System.Globalization.CultureData))]
[assembly: TypeForwardedTo(typeof(System.Globalization.CalendarData))]

@joperezr
Copy link
Member Author

@mmitche looks like my Linux ARM Emulator failures are due to "No space left on device" issue: #6676

@jashook
Copy link

jashook commented Aug 24, 2016

@joperezr tracked with #6676

@weshaggard
Copy link
Member

LGTM.

Please do verify that we don't have any other similar issues. Can you verify that none of the other types in this file are duplicated?

@tarekgh
Copy link
Member

tarekgh commented Aug 24, 2016

@joperezr would be good if you can quickly edit the title and the description of this issue. just in case if we reference it in the future :-)

@joperezr joperezr changed the title Fixing the Facade's typeforward to ConfiguredTaskAwaiter Fixing the Facade's double typeforward to CultureTypes Aug 24, 2016
@jkotas jkotas merged commit 0da0052 into dotnet:master Aug 24, 2016
@joperezr joperezr deleted the FixFacadeTypeForward branch August 24, 2016 23:15
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
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.

6 participants