Page MenuHomePhabricator

csharp: Specifying StringComparison.
ClosedPublic

Authored by brunobelo on Nov 12 2019, 11:56 AM.

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
brunobelo created this revision.Nov 12 2019, 11:56 AM
brunobelo requested review of this revision.Nov 12 2019, 11:56 AM
lauromoura accepted this revision.Tue, Nov 19, 7:00 PM
This revision is now accepted and ready to land.Tue, Nov 19, 7:00 PM
lauromoura requested changes to this revision.Tue, Nov 19, 7:10 PM

Ooops. Looks like we might have an issue with this one on Mono (6.4.0.198 here):

System.NotImplementedException: The method or operation is not implemented.
  at System.Globalization.CompareInfo.IndexOf (System.String source, System.String value, System.Int32 startIndex, System.Int32 count, System.Globalization.CompareOptions options, System.Int32* matchLengthPtr) <0x7f5d776bb410 + 0x000f0> in <285579f54af44a2ca048dad6be20e190>:0 
  at System.String.ReplaceCore (System.String oldValue, System.String newValue, System.Globalization.CultureInfo culture, System.Globalization.CompareOptions options) <0x7f5d773f5ad0 + 0x000bf> in <285579f54af44a2ca048dad6be20e190>:0 
  at System.String.Replace (System.String oldValue, System.String newValue, System.StringComparison comparisonType) <0x7f5d773f5980 + 0x00104> in <285579f54af44a2ca048dad6be20e190>:0 
  at Efl.Eo.ClassRegister.GetManagedType (System.IntPtr klass) [0x00047] in <f7906c363c504b09af140bf70790fc33>:0 
  at Efl.Eo.Globals.CreateWrapperFor (System.IntPtr handle, System.Boolean shouldIncRef) [0x0007c] in <f7906c363c504b09af140bf70790fc33>:0 
  at Efl.Eo.MarshalEoNoMove.MarshalNativeToManaged (System.IntPtr pNativeData) [0x00000] in <f7906c363c504b09af140bf70790fc33>:0 
  at (wrapper managed-to-native) System.Object.wrapper_native_0x7f5d763329ac()
  at Efl.App.GetAppMain () [0x0000f] in <f7906c363c504b09af140bf70790fc33>:0 
  at Efl.App.get_AppMain () [0x00000] in <f7906c363c504b09af140bf70790fc33>:0 
  at Test.CollectAndIterate (System.Int32 iterations, System.Int32 global_iterations) [0x00023] in <6140687a2d5a424ea90b9da160417f64>:0 
  at TestMain.Main (System.String[] args) [0x0011b] in <6140687a2d5a424ea90b9da160417f64>:0 
[ERROR] FATAL UNHANDLED EXCEPTION: System.NotImplementedException: The method or operation is not implemented.
  at System.Globalization.CompareInfo.IndexOf (System.String source, System.String value, System.Int32 startIndex, System.Int32 count, System.Globalization.CompareOptions options, System.Int32* matchLengthPtr) <0x7f5d776bb410 + 0x000f0> in <285579f54af44a2ca048dad6be20e190>:0 
  at System.String.ReplaceCore (System.String oldValue, System.String newValue, System.Globalization.CultureInfo culture, System.Globalization.CompareOptions options) <0x7f5d773f5ad0 + 0x000bf> in <285579f54af44a2ca048dad6be20e190>:0 
  at System.String.Replace (System.String oldValue, System.String newValue, System.StringComparison comparisonType) <0x7f5d773f5980 + 0x00104> in <285579f54af44a2ca048dad6be20e190>:0 
  at Efl.Eo.ClassRegister.GetManagedType (System.IntPtr klass) [0x00047] in <f7906c363c504b09af140bf70790fc33>:0 
  at Efl.Eo.Globals.CreateWrapperFor (System.IntPtr handle, System.Boolean shouldIncRef) [0x0007c] in <f7906c363c504b09af140bf70790fc33>:0 
  at Efl.Eo.MarshalEoNoMove.MarshalNativeToManaged (System.IntPtr pNativeData) [0x00000] in <f7906c363c504b09af140bf70790fc33>:0 
  at (wrapper managed-to-native) System.Object.wrapper_native_0x7f5d763329ac()
  at Efl.App.GetAppMain () [0x0000f] in <f7906c363c504b09af140bf70790fc33>:0 
  at Efl.App.get_AppMain () [0x00000] in <f7906c363c504b09af140bf70790fc33>:0 
  at Test.CollectAndIterate (System.Int32 iterations, System.Int32 global_iterations) [0x00023] in <6140687a2d5a424ea90b9da160417f64>:0 
  at TestMain.Main (System.String[] args) [0x0011b] in <6140687a2d5a424ea90b9da160417f64>:0

(Had accepted after testing just on dotnet, sorry).

This revision now requires changes to proceed.Tue, Nov 19, 7:10 PM
lauromoura added inline comments.Tue, Nov 19, 7:23 PM
src/bindings/mono/eo_mono/iwrapper.cs
872–873

Removing this change makes the test work. It is reasonable to expect that classes names won't be localized, isn't it?

I think we could pragma disable/restore it.

segfaultxavi requested changes to this revision.Wed, Nov 20, 1:07 AM
segfaultxavi added a subscriber: segfaultxavi.
segfaultxavi added inline comments.
src/bindings/mono/eo_mono/iwrapper.cs
872–873

Wait, what? Are we using culture-dependent string operations on CLASS NAMES?

That does not sound correct at all, and is bound to cause problems in the future... Class names are always written in English!

brunobelo updated this revision to Diff 27003.Wed, Nov 20, 6:21 AM

update and changing CurrentCulture to Ordinal.

segfaultxavi resigned from this revision.Wed, Nov 20, 7:07 AM

You scared me there for a second :)

lauromoura accepted this revision.Wed, Nov 20, 12:13 PM
lauromoura added inline comments.
src/bin/eolian_mono/eolian/mono/struct_definition.hh
488

This is not needed with D10696 (forgot to add you as a reviewer).

I'll update it removing and replacing with the actual comparison in the generator.

This revision is now accepted and ready to land.Wed, Nov 20, 12:13 PM
This revision was automatically updated to reflect the committed changes.