Fix hashing in Logging source gen

[tarekgh]

Fixes #110698

This change to avoid throwing from the hashing method which can break logging source generation.

I'll submit another PR to fix https://github.com/dotnet/extensions/blob/20c12ef61fc33865f36c1f4f6e8e2240e8c25f32/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs#L149 after I merge this one.

[tarekgh]

@stephentoub could you please help review this change. I recall you helped review the original code.

CC @geeknoid

[geeknoid]

Is this code supposed to actually be compiled with overflow checking on?

[tarekgh]

Is this code supposed to actually be compiled with overflow checking on?

My understanding is that overflowing check is turned on by default and we don't turn it off anywhere.

I misspoke here. Actually, the default behavior is unchecked according to the docs https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/checked-and-unchecked#default-overflow-checking-context. I'll remove the use of unchecked in the change then. We still need the remaining changes

[geeknoid]

Ah, it's Math.Abs that's throwing the exception. Two's complement strikes again!

[KalleOlaviNiemitalo]

If the hash code is negative but not Int32.MinValue, then the previous version of the source generator already mapped it to a positive EventId.Id, but the new version now maps it to a different EventId.Id. That is a breaking change if there is downstream event processing that recognises the EventId.Id. I'm not sure how likely that is.

Microsoft.Extensions.Logging.EventSource writes the EventId.Id and EventId.Name to the event log, and a user could have saved an Event Viewer filter that recognises the EventId.Id. However, because the EventId.Id goes only to the event data and not to EVENTLOGRECORD::EventID, such a filter cannot be constructed by clicking in the Event Viewer UI and can only be input as XPath. And I think the user would be likely to filter by the EventId.Name instead, to make the XPath filter easier to maintain.

Microsoft.Extensions.Logging.TraceSource writes EventId.Id to trace listeners but ignores EventId.Name. A trace listener could in principle have a TraceFilter that recognises the EventId.Id. But .NET does not define such a filter class so it would have to be installed and configured as part of the application. So this does not seem very likely either.

Anyway, these compatibility risks are why I had suggested only mapping Int32.MinValue to zero and keeping Math.Abs for other negative values.

[tarekgh]

@KalleOlaviNiemitalo good point. Compatibility is important and will be good to avoid breaking. I'll submit another PR to fix that. Thanks!