Merge pull request #99 from MichalStrehovsky/stringlayout

Special case string field layout

src/ILToNative.Compiler/src/Compiler/DependencyAnalysis/EETypeNode.cs

@@ -3,12 +3,11 @@
 
 using System;
 using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
 using ILToNative.DependencyAnalysisFramework;
 using Internal.TypeSystem;
 
+using Debug = System.Diagnostics.Debug;
+
 namespace ILToNative.DependencyAnalysis
 {
     class EETypeNode : ObjectNode, ISymbolNode
@@ -132,6 +131,16 @@ namespace ILToNative.DependencyAnalysis
 
             objectSize = AlignmentHelper.AlignUp(objectSize, pointerSize);
             objectSize = Math.Max(minimumObjectSize, objectSize);
+
+            if (_type.IsString)
+            {
+                // If this is a string, throw away objectSize we computed so far. Strings are special.
+                // SyncBlock + EETypePtr + length + firstChar
+                objectSize = 2 * pointerSize +
+                    _type.Context.GetWellKnownType(WellKnownType.Int32).GetElementSize() +
+                    _type.Context.GetWellKnownType(WellKnownType.Char).GetElementSize();
+            }
+
             objData.EmitInt(objectSize);
 
             if (Type.BaseType != null)

src/System.Private.CoreLib/src/System/String.cs

@@ -83,7 +83,11 @@ namespace System
     // eagerly constructed to avoid the cost of defered ctors. I can't imagine any app that doesn't use string
     //
     [ComVisible(true)]
+#if !CORERT
     [StructLayout(LayoutKind.Explicit)]
+#else
+    [StructLayout(LayoutKind.Sequential)]
+#endif
     [System.Runtime.CompilerServices.EagerStaticClassConstructionAttribute]
     public sealed class String : IComparable, IEnumerable, IEnumerable<char>, IComparable<String>, IEquatable<String>, IConvertible
     {
@@ -99,12 +103,18 @@ namespace System
         // CS0169: The private field '{blah}' is never used
         // CS0649: Field '{blah}' is never assigned to, and will always have its default value
 #pragma warning disable 169, 649
+
+        // CORERT porting note: offset is if'd out because it's bogus. These were needed for Bartok
+        // compatibility years ago and NUTC has a corresponding workaround for this in the field layout code.
+        // We should remove the if'd out code on the .NET Native side too and remove the workaround in NUTC.
 #if !CORERT
         [Bound]
-#endif
         [FieldOffset(STRING_LENGTH_OFFSET)]
+#endif
         private int _stringLength;
+#if !CORERT
         [FieldOffset(FIRST_CHAR_OFFSET)]
+#endif
         private char _firstChar;
 #pragma warning restore
 
@@ -113,10 +123,6 @@ namespace System
 // declared constructors. We use a RuntimeImport/RuntimeExport combination to workaround this difference.
 // TODO: Determine a more reasonable solution for this.
 
-// TODO: For (length == 0), the code goes through AllocateString and returns an empty string. If this
-// is more common, we can explicitly check for it. We should also decide on semantics when passed in array/char* is NULL. 
-// The code checks and returns String.Empty.
-
         [MethodImplAttribute(MethodImplOptions.InternalCall)]
 #if CORERT
         public extern String(char[] value);   // CtorCharArray