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

Add conditionals allowing some calenders to be disabled.#270

Merged
marek-safar merged 2 commits intomono:masterfrom
baulig:martin-linker-calendars
Apr 2, 2019
Merged

Add conditionals allowing some calenders to be disabled.#270
marek-safar merged 2 commits intomono:masterfrom
baulig:martin-linker-calendars

Conversation

@baulig
Copy link

@baulig baulig commented Mar 29, 2019

Add GlobalizationMode.Invariant conditionals to some calendar code.

This allows the Japanese, Taiwan and Hebrew calendars to be disabled.

@baulig baulig force-pushed the martin-linker-calendars branch 2 times, most recently from d2a0cc6 to 018e36e Compare March 29, 2019 20:03
@baulig baulig marked this pull request as ready for review March 29, 2019 20:06
This allows the Japanese, Taiwan and Hebrew calendars to be disabled.
@baulig baulig force-pushed the martin-linker-calendars branch from 018e36e to 715b2ab Compare March 29, 2019 20:17
@baulig baulig requested a review from marek-safar April 1, 2019 17:35
{
if (!GlobalizationMode.Invariant)
{
m_hebrewNumberParser = new MatchNumberDelegate(DateTimeParse.MatchHebrewDigits);
Copy link
Member

Choose a reason for hiding this comment

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

This should be LazyInitialized instead of explicit static cctor which we won't be able easily remove

Copy link
Author

Choose a reason for hiding this comment

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

I am fine with using some Lazy<T> initialization and / or putting that inside DoStrictParse().

However, the field initializer that this code was previously using has to go away.

The general rule here is that the pattern new SomeDelegate (ConditionalMethod) must not be used outside a linker conditional (like for instance GlobalizationMode.Invariant), so you cannot put it into a field initializer. The linker is unable to "go inside" that delegate initialization and we also don't have full support for dead field elimination yet.

Copy link
Member

Choose a reason for hiding this comment

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

That's what I meant. You keep only internal static MatchNumberDelegate m_hebrewNumberParser; with LazyInitializer.EnsureInitialized (ref m_hebrewNumberParser, ...) inside DoStrictParse

Copy link
Author

Choose a reason for hiding this comment

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

Like this:

            if (!GlobalizationMode.Invariant && (CalendarId)parseInfo.calendar.ID == CalendarId.HEBREW)
            {
                LazyInitializer.EnsureInitialized (ref m_hebrewNumberParser, () => new MatchNumberDelegate(DateTimeParse.MatchHebrewDigits));
                parseInfo.parseNumberDelegate = m_hebrewNumberParser;
                parseInfo.fCustomNumberParser = true;
            }

or do we need locking?

Copy link
Member

Choose a reason for hiding this comment

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

yep, EnsureInitialized is thread-safe

Copy link
Author

Choose a reason for hiding this comment

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

Okay, done.

@baulig baulig requested a review from marek-safar April 1, 2019 22:16
@marek-safar marek-safar merged commit f759dcc into mono:master Apr 2, 2019
@baulig baulig deleted the martin-linker-calendars branch April 2, 2019 18:33
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.

2 participants