From edbec0425b9f09932b5c7ec3b1b53166e081e212 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Thu, 6 Feb 2025 20:32:52 +0100 Subject: [PATCH 1/6] Add supportfor Unicode strings --- .../LlvmIrGenerator/LlvmIrComposer.cs | 2 +- .../LlvmIrGenerator/LlvmIrGenerator.cs | 97 ++++++++++---- .../LlvmIrGenerator/LlvmIrInstructions.cs | 4 +- .../Utilities/LlvmIrGenerator/LlvmIrModule.cs | 31 ++--- .../LlvmIrGenerator/LlvmIrStringEncoding.cs | 7 + .../LlvmIrGenerator/LlvmIrStringManager.cs | 21 +-- .../LlvmIrGenerator/LlvmIrVariable.cs | 25 +++- .../LlvmIrGenerator/MemberInfoUtilities.cs | 22 ++++ .../NativeAssemblerAttribute.cs | 6 + .../Utilities/LlvmIrGenerator/StringHolder.cs | 124 ++++++++++++++++++ 10 files changed, 286 insertions(+), 53 deletions(-) create mode 100644 src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringEncoding.cs create mode 100644 src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/StringHolder.cs diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrComposer.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrComposer.cs index b7bcaaf9db5..589ab5e7c48 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrComposer.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrComposer.cs @@ -23,7 +23,7 @@ protected LlvmIrComposer (TaskLoggingHelper log) public LlvmIrModule Construct () { - var module = new LlvmIrModule (cache); + var module = new LlvmIrModule (cache, Log); Construct (module); module.AfterConstruction (); constructed = true; diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs index 7e29ecad9f2..a81715fe00d 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs @@ -199,12 +199,19 @@ void WriteStrings (GeneratorWriteContext context) } foreach (LlvmIrStringVariable info in group.Strings) { - string s = QuoteString ((string)info.Value, out ulong size); + string s = QuoteString (info, out ulong size); + + if (!info.IsConstantStringLiteral) { + WriteCommentLine (context, $" '{info.Value}'"); + } WriteGlobalVariableStart (context, info); context.Output.Write ('['); context.Output.Write (size.ToString (CultureInfo.InvariantCulture)); - context.Output.Write (" x i8] c"); + context.Output.Write ($" x {info.IrType}] "); + if (info.IsConstantStringLiteral) { + context.Output.Write ('c'); + } context.Output.Write (s); context.Output.Write (", align "); context.Output.WriteLine (target.GetAggregateAlignment (1, size).ToString (CultureInfo.InvariantCulture)); @@ -560,7 +567,7 @@ void WriteInlineArray (GeneratorWriteContext context, byte[] bytes, bool encodeA { if (encodeAsASCII) { context.Output.Write ('c'); - context.Output.Write (QuoteString (bytes, bytes.Length, out _, nullTerminated: false)); + context.Output.Write (QuoteUtf8String (bytes, bytes.Length, out _, nullTerminated: false)); return; } @@ -616,7 +623,7 @@ void WriteValue (GeneratorWriteContext context, StructureInstance structInstance return; } - WriteValue (context, smi.MemberType, value); + WriteValue (context, smi.MemberType, value, smi.Info.GetStringEncoding (context.TypeCache)); } bool WriteNativePointerValue (GeneratorWriteContext context, StructureInstance si, StructureMemberInfo smi, object? value) @@ -670,7 +677,7 @@ string ToHex (BasicType basicTypeDesc, Type type, object? value) return $"{(basicTypeDesc.IsUnsigned ? prefixUnsigned : prefixSigned)}0x{hex}"; } - void WriteValue (GeneratorWriteContext context, Type type, object? value) + void WriteValue (GeneratorWriteContext context, Type type, object? value, LlvmIrStringEncoding stringEncoding = LlvmIrStringEncoding.UTF8) { if (value is LlvmIrVariable variableRef) { context.Output.Write (variableRef.Reference); @@ -716,7 +723,7 @@ void WriteValue (GeneratorWriteContext context, Type type, object? value) return; } - LlvmIrStringVariable sv = context.Module.LookupRequiredVariableForString ((string)value); + LlvmIrStringVariable sv = context.Module.LookupRequiredVariableForString (StringHolder.AsHolder (value, stringEncoding)); context.Output.Write (sv.Reference); return; } @@ -775,7 +782,7 @@ void WriteStructureValue (GeneratorWriteContext context, StructureInstance? inst string? comment = info.GetCommentFromProvider (smi, instance); if (String.IsNullOrEmpty (comment)) { var sb = new StringBuilder (" "); - sb.Append (MapManagedTypeToNative (smi)); + sb.Append (MapManagedTypeToNative (context, smi)); sb.Append (' '); sb.Append (smi.Info.Name); comment = sb.ToString (); @@ -1460,8 +1467,12 @@ public static string MapManagedTypeToNative (Type type) return type.GetShortName (); } - static string MapManagedTypeToNative (StructureMemberInfo smi) + static string MapManagedTypeToNative (GeneratorWriteContext context, StructureMemberInfo smi) { + if (smi.Info.IsUnicodeString (context.TypeCache)) { + return "char16_t*"; + } + string nativeType = MapManagedTypeToNative (smi.MemberType); // Silly, but effective if (nativeType[nativeType.Length - 1] == '*') { @@ -1487,8 +1498,9 @@ static string MapManagedTypeToNative (StructureMemberInfo smi) throw new InvalidOperationException ($"Field '{smi.Info.Name}' of structure '{info.Name}' should have a value of '{expectedType}' type, instead it had a '{value.GetType ()}'"); } - if (valueType == typeof(string)) { - return context.Module.LookupRequiredVariableForString ((string)value); + if (valueType == typeof(string) || valueType == typeof(StringHolder)) { + var encoding = smi.Info.GetStringEncoding (context.TypeCache); + return context.Module.LookupRequiredVariableForString (StringHolder.AsHolder (value, encoding)); } return value; @@ -1555,30 +1567,63 @@ public static string QuoteStringNoEscape (string s) return $"\"{s}\""; } - public static string QuoteString (string value, bool nullTerminated = true) + public static string QuoteString (LlvmIrStringVariable variable, out ulong stringSize, bool nullTerminated = true) { - return QuoteString (value, out _, nullTerminated); - } + if (variable.Encoding == LlvmIrStringEncoding.UTF8) { + var value = (StringHolder)variable.Value; + if (value.Data == null) { + throw new InvalidOperationException ("Internal error: null strings not supported here, they should be handled elsewhere."); + } - public static string QuoteString (byte[] bytes) - { - return QuoteString (bytes, bytes.Length, out _, nullTerminated: false); + int byteCount = Encoding.UTF8.GetByteCount (value.Data); + var bytes = ArrayPool.Shared.Rent (byteCount); + + try { + Encoding.UTF8.GetBytes (value.Data, 0, value.Data.Length, bytes, 0); + return QuoteUtf8String (bytes, byteCount, out stringSize, nullTerminated); + } finally { + ArrayPool.Shared.Return (bytes); + } + } + + if (variable.Encoding == LlvmIrStringEncoding.Unicode) { + return QuoteUnicodeString (variable, out stringSize, nullTerminated); + } + + throw new InvalidOperationException ($"Internal error: unsupported string encoding {variable.Encoding}"); } - public static string QuoteString (string value, out ulong stringSize, bool nullTerminated = true) + static string QuoteUnicodeString (LlvmIrStringVariable variable, out ulong stringSize, bool nullTerminated = true) { - var encoding = Encoding.UTF8; - int byteCount = encoding.GetByteCount (value); - var bytes = ArrayPool.Shared.Rent (byteCount); - try { - encoding.GetBytes (value, 0, value.Length, bytes, 0); - return QuoteString (bytes, byteCount, out stringSize, nullTerminated); - } finally { - ArrayPool.Shared.Return (bytes); + var value = (StringHolder)variable.Value; + if (value.Data == null) { + throw new InvalidOperationException ("Internal error: null strings not supported here, they should be handled elsewhere."); + } + + // Each character/lexeme is encoded as iXY u0xVXYZ + comma and a space, and on top of that we have two square brackets and a trailing nul + var sb = new StringBuilder ((value.Data.Length * 13) + 3); // rough estimate of capacity + sb.Append ('['); + for (int i = 0; i < value.Data.Length; i++) { + var ch = (short)value.Data[i]; + if (i > 0) { + sb.Append (", "); + } + sb.Append ($"{variable.IrType} u0x{ch:X2}"); } + + if (nullTerminated) { + if (value.Data.Length > 0) { + sb.Append (", "); + } + sb.Append ($"{variable.IrType} 0"); + } + sb.Append (']'); + + stringSize = (ulong)value.Data.Length + (nullTerminated ? 1u : 0u); + return sb.ToString (); } - public static string QuoteString (byte[] bytes, int byteCount, out ulong stringSize, bool nullTerminated = true) + static string QuoteUtf8String (byte[] bytes, int byteCount, out ulong stringSize, bool nullTerminated = true) { var sb = new StringBuilder (byteCount * 2); // rough estimate of capacity diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrInstructions.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrInstructions.cs index 1d93cbec9e5..f2c23b80e9b 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrInstructions.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrInstructions.cs @@ -366,8 +366,8 @@ void WriteArgument (GeneratorWriteContext context, LlvmIrFunctionParameter? para throw new InvalidOperationException ($"Internal error: value type '{value.GetType ()}' for argument {index} to function '{function.Signature.Name}' is invalid. Expected '{parameter.Type}' or compatible"); } - if (value is string str) { - context.Output.Write (context.Module.LookupRequiredVariableForString (str).Reference); + if (value is string || value is StringHolder) { + context.Output.Write (context.Module.LookupRequiredVariableForString (StringHolder.AsHolder (value)).Reference); return; } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs index 7d9acb7c3e4..a9db93cf406 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs @@ -2,6 +2,8 @@ using System.Collections.Generic; using System.Linq; +using Microsoft.Build.Utilities; + using Xamarin.Android.Tools; namespace Xamarin.Android.Tasks.LLVMIR @@ -43,10 +45,13 @@ partial class LlvmIrModule LlvmIrFunction? puts; LlvmIrFunction? abort; + TaskLoggingHelper log; + public readonly LlvmIrTypeCache TypeCache; - public LlvmIrModule (LlvmIrTypeCache cache) + public LlvmIrModule (LlvmIrTypeCache cache, TaskLoggingHelper log) { + this.log = log; TypeCache = cache; metadataManager = new LlvmIrMetadataManager (cache); @@ -275,7 +280,7 @@ public void Add (LlvmIrGlobalVariable variable, string stringGroupName, string? EnsureValidGlobalVariableType (variable); if (IsStringVariable (variable)) { - AddStringGlobalVariable (variable, stringGroupName, stringGroupComment, symbolSuffix); + AddStringGlobalVariable ((LlvmIrStringVariable)variable, stringGroupName, stringGroupComment, symbolSuffix); return; } @@ -299,7 +304,7 @@ public void Add (LlvmIrGlobalVariable variable) EnsureValidGlobalVariableType (variable); if (IsStringVariable (variable)) { - AddStringGlobalVariable (variable); + AddStringGlobalVariable ((LlvmIrStringVariable)variable); return; } @@ -359,7 +364,7 @@ void PrepareStructure (StructureInstance structure) string? value = smi.GetValue (structure.Obj) as string; if (value != null) { - RegisterString (value, stringGroupName: structure.Info.Name, symbolSuffix: smi.Info.Name); + RegisterString (value, stringGroupName: structure.Info.Name, symbolSuffix: smi.Info.Name, encoding: smi.Info.GetStringEncoding (TypeCache)); } } } @@ -387,24 +392,20 @@ void AddStandardGlobalVariable (LlvmIrGlobalVariable variable) globalVariables.Add (variable); } - void AddStringGlobalVariable (LlvmIrGlobalVariable variable, string? stringGroupName = null, string? stringGroupComment = null, string? symbolSuffix = null) + void AddStringGlobalVariable (LlvmIrStringVariable variable, string? stringGroupName = null, string? stringGroupComment = null, string? symbolSuffix = null) { - RegisterString (variable, stringGroupName, stringGroupComment, symbolSuffix); + RegisterString ((string)variable.Value, stringGroupName, stringGroupComment, symbolSuffix, variable.Encoding); AddStandardGlobalVariable (variable); } - void RegisterString (LlvmIrGlobalVariable variable, string? stringGroupName = null, string? stringGroupComment = null, string? symbolSuffix = null) - { - RegisterString ((string)variable.Value, stringGroupName, stringGroupComment, symbolSuffix); - } - - public void RegisterString (string value, string? stringGroupName = null, string? stringGroupComment = null, string? symbolSuffix = null) + public void RegisterString (string value, string? stringGroupName = null, string? stringGroupComment = null, string? symbolSuffix = null, + LlvmIrStringEncoding encoding = LlvmIrStringEncoding.UTF8, StringComparison comparison = StringComparison.Ordinal) { if (stringManager == null) { - stringManager = new LlvmIrStringManager (); + stringManager = new LlvmIrStringManager (log); } - stringManager.Add (value, stringGroupName, stringGroupComment, symbolSuffix); + stringManager.Add (value, stringGroupName, stringGroupComment, symbolSuffix, encoding, comparison); } void AddStructureArrayGlobalVariable (LlvmIrGlobalVariable variable) @@ -569,7 +570,7 @@ void EnsureValidGlobalVariableType (LlvmIrGlobalVariable variable) /// are part of structure instances. Such strings **MUST** be registered by and, thus, failure to do /// so is an internal error. /// - public LlvmIrStringVariable LookupRequiredVariableForString (string value) + public LlvmIrStringVariable LookupRequiredVariableForString (StringHolder value) { LlvmIrStringVariable? sv = stringManager?.Lookup (value); if (sv == null) { diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringEncoding.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringEncoding.cs new file mode 100644 index 00000000000..abbad82df4f --- /dev/null +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringEncoding.cs @@ -0,0 +1,7 @@ +namespace Xamarin.Android.Tasks.LLVMIR; + +enum LlvmIrStringEncoding +{ + UTF8, + Unicode, +} diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringManager.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringManager.cs index 9a49e544428..4653553bd7c 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringManager.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringManager.cs @@ -1,35 +1,40 @@ using System; using System.Collections.Generic; +using Microsoft.Build.Utilities; + namespace Xamarin.Android.Tasks.LLVMIR; partial class LlvmIrModule { protected class LlvmIrStringManager { - Dictionary stringSymbolCache = new Dictionary (StringComparer.Ordinal); + Dictionary stringSymbolCache = new Dictionary (); Dictionary stringGroupCache = new Dictionary (StringComparer.Ordinal); List stringGroups = new List (); LlvmIrStringGroup defaultGroup; + TaskLoggingHelper log; public List StringGroups => stringGroups; - public LlvmIrStringManager () + public LlvmIrStringManager (TaskLoggingHelper log) { + this.log = log; defaultGroup = new LlvmIrStringGroup (); stringGroupCache.Add (String.Empty, defaultGroup); stringGroups.Add (defaultGroup); } - public LlvmIrStringVariable Add (string value, string? groupName = null, string? groupComment = null, string? symbolSuffix = null) + public LlvmIrStringVariable Add (string value, string? groupName = null, string? groupComment = null, string? symbolSuffix = null, + LlvmIrStringEncoding encoding = LlvmIrStringEncoding.UTF8, StringComparison comparison = StringComparison.Ordinal) { if (value == null) { throw new ArgumentNullException (nameof (value)); } - LlvmIrStringVariable? stringVar; - if (stringSymbolCache.TryGetValue (value, out stringVar) && stringVar != null) { + var holder = new StringHolder (value, encoding, comparison); + if (stringSymbolCache.TryGetValue (holder, out LlvmIrStringVariable? stringVar) && stringVar != null) { return stringVar; } @@ -52,14 +57,14 @@ public LlvmIrStringVariable Add (string value, string? groupName = null, string? symbolName = $"{symbolName}_{symbolSuffix}"; } - stringVar = new LlvmIrStringVariable (symbolName, value); + stringVar = new LlvmIrStringVariable (symbolName, holder); group.Strings.Add (stringVar); - stringSymbolCache.Add (value, stringVar); + stringSymbolCache.Add (holder, stringVar); return stringVar; } - public LlvmIrStringVariable? Lookup (string value) + public LlvmIrStringVariable? Lookup (StringHolder value) { if (stringSymbolCache.TryGetValue (value, out LlvmIrStringVariable? sv)) { return sv; diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrVariable.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrVariable.cs index d66bda3d7c9..d6571f3760e 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrVariable.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrVariable.cs @@ -297,11 +297,34 @@ public void OverrideName (string newName) class LlvmIrStringVariable : LlvmIrGlobalVariable { - public LlvmIrStringVariable (string name, string value) + public LlvmIrStringEncoding Encoding { get; } + public string IrType { get; } + public bool IsConstantStringLiteral { get; } + + public LlvmIrStringVariable (string name, StringHolder value) : base (typeof(string), name, LlvmIrVariableOptions.LocalString) { Value = value; + Encoding = value.Encoding; + + switch (value.Encoding) { + case LlvmIrStringEncoding.UTF8: + IrType = "i8"; + IsConstantStringLiteral = true; + break; + + case LlvmIrStringEncoding.Unicode: + IrType = "i16"; + break; + + default: + throw new InvalidOperationException ($"Internal error: unsupported string encoding {value.Encoding}"); + } } + + public LlvmIrStringVariable (string name, string value, LlvmIrStringEncoding encoding, StringComparison comparison = StringComparison.Ordinal) + : this (name, new StringHolder (value, encoding, comparison)) + {} } /// diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/MemberInfoUtilities.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/MemberInfoUtilities.cs index c467e03faee..500e5f8f398 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/MemberInfoUtilities.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/MemberInfoUtilities.cs @@ -34,6 +34,28 @@ public static bool PointsToSymbol (this MemberInfo mi, LlvmIrTypeCache cache, ou return true; } + public static bool IsUnicodeString (this MemberInfo mi, LlvmIrTypeCache cache) + { + var attr = cache.GetNativeAssemblerAttribute (mi); + if (attr == null) { + return false; + } + + return attr.IsUTF16; + } + + public static LlvmIrStringEncoding GetStringEncoding (this MemberInfo mi, LlvmIrTypeCache cache) + { + const LlvmIrStringEncoding DefaultStringEncoding = LlvmIrStringEncoding.UTF8; + + var attr = cache.GetNativeAssemblerAttribute (mi); + if (attr == null) { + return DefaultStringEncoding; + } + + return attr.IsUTF16 ? LlvmIrStringEncoding.Unicode : DefaultStringEncoding; + } + public static bool ShouldBeIgnored (this MemberInfo mi, LlvmIrTypeCache cache) { var attr = cache.GetNativeAssemblerAttribute (mi); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/NativeAssemblerAttribute.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/NativeAssemblerAttribute.cs index c239255e1ac..58b86b29dc4 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/NativeAssemblerAttribute.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/NativeAssemblerAttribute.cs @@ -35,6 +35,12 @@ class NativeAssemblerAttribute : Attribute public bool NeedsPadding { get; set; } public LLVMIR.LlvmIrVariableNumberFormat NumberFormat { get; set; } = LLVMIR.LlvmIrVariableNumberFormat.Default; + + /// + /// Taken into account only for fields of string types. If set to true, the string is output as an UTF16 in + /// the native assembly file. + /// + public bool IsUTF16 { get; set; } } [AttributeUsage (AttributeTargets.Class, Inherited = true)] diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/StringHolder.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/StringHolder.cs new file mode 100644 index 00000000000..cbcfd02c9d8 --- /dev/null +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/StringHolder.cs @@ -0,0 +1,124 @@ +using System; + +namespace Xamarin.Android.Tasks.LLVMIR; + +class StringHolder : IComparable, IComparable, IEquatable +{ + public LlvmIrStringEncoding Encoding { get; } + public string? Data { get; } + + StringComparison comparison; + + public StringHolder (string? data, LlvmIrStringEncoding encoding = LlvmIrStringEncoding.UTF8, StringComparison comparison = StringComparison.Ordinal) + { + Data = data; + Encoding = encoding; + this.comparison = comparison; + } + + public static StringHolder AsHolder (object? value, LlvmIrStringEncoding encoding = LlvmIrStringEncoding.UTF8, StringComparison comparison = StringComparison.Ordinal) + { + if (value == null) { + return new StringHolder ((string)value); + } + + StringHolder holder; + if (value is string) { + holder = new StringHolder ((string)value, encoding, comparison); + } else if (value is StringHolder) { + holder = (StringHolder)value; + } else { + throw new InvalidOperationException ($"Internal error: expected 'string' type, got '{value.GetType ()}' instead."); + } + + return holder; + } + + public int CompareTo (object obj) + { + var holder = obj as StringHolder; + if (holder == null) { + return 1; + } + + return CompareTo (holder); + } + + public int CompareTo (StringHolder? other) + { + if (other == null) { + return 1; + } + + int encodingCompare = Encoding.CompareTo (other.Encoding); + if (Data == null) { + if (other.Data != null) { + // We are "smaller", because the other holder actually has a valid string + return -1; + } + + // Both strings are null, so we care only about the encoding + return encodingCompare; + } + + int dataCompare = other.Data == null ? Data.CompareTo (other.Data) : String.Compare (Data, other.Data, comparison); + // If encodings are identical, we compare strings, allowing any result of the comparisons + if (encodingCompare == 0) { + return dataCompare; + } + + // However if encodings aren't the same, we mustn't allow Data comparison to return 0 + // as the strings are **not** equal, even though their Data values are. In this case, + // we fall back to encoding for comparison result. + return dataCompare != 0 ? dataCompare : encodingCompare; + } + + public override int GetHashCode () + { + int hc = 0; + if (Data != null) { + hc ^= Data.GetHashCode (); + } + + return hc ^ Encoding.GetHashCode (); + } + + public override bool Equals (object obj) + { + var holder = obj as StringHolder; + if (holder == null) { + return false; + } + + return Equals (holder); + } + + public bool Equals (StringHolder? other) + { + if (other == null || Encoding != other.Encoding) { + return false; + } + + return String.Compare (Data, other.Data, comparison) == 0; + } + + public static bool operator > (StringHolder a, StringHolder b) + { + return a.CompareTo (b) > 0; + } + + public static bool operator < (StringHolder a, StringHolder b) + { + return a.CompareTo (b) < 0; + } + + public static bool operator >= (StringHolder a, StringHolder b) + { + return a.CompareTo (b) >= 0; + } + + public static bool operator <= (StringHolder a, StringHolder b) + { + return a.CompareTo (b) <= 0; + } +} From a1ef311904f2c028ebda151cebae00c39ed7900a Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Mon, 10 Feb 2025 11:27:52 +0100 Subject: [PATCH 2/6] Create an LlvmIrStringVariable instance when adding a string variable --- .../Utilities/LlvmIrGenerator/LlvmIrModule.cs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs index a9db93cf406..3baa55fd5aa 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs @@ -267,10 +267,19 @@ public LlvmIrGlobalVariable AddGlobalVariable (string name, object value, LlvmIr /// public LlvmIrGlobalVariable AddGlobalVariable (Type type, string name, object? value, LlvmIrVariableOptions? options = null, string? comment = null) { - var ret = new LlvmIrGlobalVariable (type, name, options) { - Value = value, - Comment = comment, - }; + LlvmIrGlobalVariable ret; + if (type == typeof(string)) { + // The cast to `string?` is intentionally meant to throw if `value` type isn't a string. + ret = new LlvmIrStringVariable (name, new StringHolder ((string?)value)) { + Comment = comment, + }; + } else { + ret = new LlvmIrGlobalVariable (type, name, options) { + Value = value, + Comment = comment, + }; + } + Add (ret); return ret; } From a781d87281a1b7240b5f26ccbe821592cf348165 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Mon, 10 Feb 2025 20:01:39 +0100 Subject: [PATCH 3/6] Let's try this --- .../LlvmIrGenerator/LlvmIrGenerator.cs | 13 +++++++++-- .../Utilities/LlvmIrGenerator/LlvmIrModule.cs | 23 ++++++++++++++----- .../LlvmIrGenerator/LlvmIrStringManager.cs | 12 +++++++++- 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs index a81715fe00d..d096ca72146 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs @@ -326,13 +326,22 @@ void WriteTypeAndValue (GeneratorWriteContext context, LlvmIrVariable variable, throw new InvalidOperationException ($"Internal error: variable '{variable.Name}'' of type {variable.Type} must not have a null value"); } - if (valueType != variable.Type && !LlvmIrModule.NameValueArrayType.IsAssignableFrom (variable.Type)) { + if (!IsValueAssignableFrom (valueType, variable) && !IsValueAssignableFrom (LlvmIrModule.NameValueArrayType, variable)) { throw new InvalidOperationException ($"Internal error: variable type '{variable.Type}' is different to its value type, '{valueType}'"); } WriteValue (context, valueType, variable); } + bool IsValueAssignableFrom (Type valueType, LlvmIrVariable variable) + { + if (valueType != typeof(string) && valueType != typeof(StringHolder)) { + return valueType.IsAssignableFrom (variable.Type); + } + + return variable.Type == typeof(string) || variable.Type == typeof(StringHolder); + } + ulong GetAggregateValueElementCount (GeneratorWriteContext context, LlvmIrVariable variable) => GetAggregateValueElementCount (context, variable.Type, variable.Value, variable as LlvmIrGlobalVariable); ulong GetAggregateValueElementCount (GeneratorWriteContext context, Type type, object? value, LlvmIrGlobalVariable? globalVariable = null) @@ -717,7 +726,7 @@ void WriteValue (GeneratorWriteContext context, Type type, object? value, LlvmIr return; } - if (type == typeof(string)) { + if (type == typeof(string) || type == typeof(StringHolder)) { if (value == null) { context.Output.Write ("null"); return; diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs index 3baa55fd5aa..6cd03079f4c 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs @@ -273,14 +273,15 @@ public LlvmIrGlobalVariable AddGlobalVariable (Type type, string name, object? v ret = new LlvmIrStringVariable (name, new StringHolder ((string?)value)) { Comment = comment, }; + AddStringGlobalVariable ((LlvmIrStringVariable)ret); } else { ret = new LlvmIrGlobalVariable (type, name, options) { Value = value, Comment = comment, }; + Add (ret); } - Add (ret); return ret; } @@ -401,19 +402,29 @@ void AddStandardGlobalVariable (LlvmIrGlobalVariable variable) globalVariables.Add (variable); } + void EnsureStringManager () + { + if (stringManager == null) { + stringManager = new LlvmIrStringManager (log); + } + } + void AddStringGlobalVariable (LlvmIrStringVariable variable, string? stringGroupName = null, string? stringGroupComment = null, string? symbolSuffix = null) { - RegisterString ((string)variable.Value, stringGroupName, stringGroupComment, symbolSuffix, variable.Encoding); + RegisterString (variable, stringGroupName, stringGroupComment, symbolSuffix); AddStandardGlobalVariable (variable); } + public void RegisterString (LlvmIrStringVariable variable, string? stringGroupName = null, string? stringGroupComment = null, string? symbolSuffix = null) + { + EnsureStringManager (); + stringManager.Add (variable, stringGroupName, stringGroupComment, symbolSuffix); + } + public void RegisterString (string value, string? stringGroupName = null, string? stringGroupComment = null, string? symbolSuffix = null, LlvmIrStringEncoding encoding = LlvmIrStringEncoding.UTF8, StringComparison comparison = StringComparison.Ordinal) { - if (stringManager == null) { - stringManager = new LlvmIrStringManager (log); - } - + EnsureStringManager (); stringManager.Add (value, stringGroupName, stringGroupComment, symbolSuffix, encoding, comparison); } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringManager.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringManager.cs index 4653553bd7c..4acbe5ae316 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringManager.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringManager.cs @@ -26,6 +26,12 @@ public LlvmIrStringManager (TaskLoggingHelper log) stringGroups.Add (defaultGroup); } + public LlvmIrStringVariable Add (LlvmIrStringVariable variable, string? groupName = null, string? groupComment = null, string? symbolSuffix = null) + { + // Let it throw if Value isn't a StringHolder, it must be. + return Add((StringHolder)variable.Value, groupName, groupComment, symbolSuffix); + } + public LlvmIrStringVariable Add (string value, string? groupName = null, string? groupComment = null, string? symbolSuffix = null, LlvmIrStringEncoding encoding = LlvmIrStringEncoding.UTF8, StringComparison comparison = StringComparison.Ordinal) { @@ -33,7 +39,11 @@ public LlvmIrStringVariable Add (string value, string? groupName = null, string? throw new ArgumentNullException (nameof (value)); } - var holder = new StringHolder (value, encoding, comparison); + return Add (new StringHolder (value, encoding, comparison), groupName, groupComment, symbolSuffix); + } + + LlvmIrStringVariable Add (StringHolder holder, string? groupName = null, string? groupComment = null, string? symbolSuffix = null) + { if (stringSymbolCache.TryGetValue (holder, out LlvmIrStringVariable? stringVar) && stringVar != null) { return stringVar; } From 36f67acd173bc9731d5ff81f435d43788c9de950 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 11 Feb 2025 14:06:14 +0100 Subject: [PATCH 4/6] "Namespace" all the local strings Since all the files are generated from separate tasks, we don't have a global LLVM IR state which can keep track of strings and ensure that there are no duplicate symbol names. To prevent potential clashes, each generator now sets the default string group name which is unique for each module. In the future, we should try to manage strings globally (which would also result in more de-duplication) --- .../ApplicationConfigNativeAssemblyGenerator.cs | 4 +++- .../CompressedAssembliesNativeAssemblyGenerator.cs | 2 ++ .../Utilities/JniRemappingAssemblyGenerator.cs | 2 ++ .../Utilities/LlvmIrGenerator/LlvmIrModule.cs | 4 +++- .../Utilities/LlvmIrGenerator/LlvmIrStringManager.cs | 10 ++++++++-- .../Utilities/LlvmIrGenerator/LlvmIrVariable.cs | 8 ++++---- .../Utilities/MarshalMethodsNativeAssemblyGenerator.cs | 2 ++ .../TypeMappingDebugNativeAssemblyGenerator.cs | 2 ++ .../TypeMappingReleaseNativeAssemblyGenerator.cs | 2 ++ 9 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs index 9a0bc213d6c..50d079388dc 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs @@ -203,6 +203,8 @@ public ApplicationConfigNativeAssemblyGenerator (IDictionary env protected override void Construct (LlvmIrModule module) { + module.DefaultStringGroup = "env"; + MapStructures (module); module.AddGlobalVariable ("format_tag", FORMAT_TAG, comment: $" 0x{FORMAT_TAG:x}"); @@ -211,7 +213,7 @@ protected override void Construct (LlvmIrModule module) var envVars = new LlvmIrGlobalVariable (environmentVariables, "app_environment_variables") { Comment = " Application environment variables array, name:value", }; - module.Add (envVars, stringGroupName: "env", stringGroupComment: " Application environment variables name:value pairs"); + module.Add (envVars, stringGroupName: "env.var", stringGroupComment: " Application environment variables name:value pairs"); var sysProps = new LlvmIrGlobalVariable (systemProperties, "app_system_properties") { Comment = " System properties defined by the application", diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/CompressedAssembliesNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/CompressedAssembliesNativeAssemblyGenerator.cs index 29d7fc1665b..9143baafe83 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/CompressedAssembliesNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/CompressedAssembliesNativeAssemblyGenerator.cs @@ -149,6 +149,8 @@ void InitCompressedAssemblies (out List? compressedAssembl protected override void Construct (LlvmIrModule module) { + module.DefaultStringGroup = "cas"; + MapStructures (module); InitCompressedAssemblies ( diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/JniRemappingAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/JniRemappingAssemblyGenerator.cs index dd6d0924fff..03bf8e70a4d 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/JniRemappingAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/JniRemappingAssemblyGenerator.cs @@ -284,6 +284,8 @@ uint GetLength (string str) protected override void Construct (LlvmIrModule module) { + module.DefaultStringGroup = "jremap"; + MapStructures (module); List>? typeReplacements; List>? methodIndexTypes; diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs index 6cd03079f4c..ab38d7b2ecd 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs @@ -49,6 +49,8 @@ partial class LlvmIrModule public readonly LlvmIrTypeCache TypeCache; + public string? DefaultStringGroup { get; set; } + public LlvmIrModule (LlvmIrTypeCache cache, TaskLoggingHelper log) { this.log = log; @@ -405,7 +407,7 @@ void AddStandardGlobalVariable (LlvmIrGlobalVariable variable) void EnsureStringManager () { if (stringManager == null) { - stringManager = new LlvmIrStringManager (log); + stringManager = new LlvmIrStringManager (log, DefaultStringGroup); } } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringManager.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringManager.cs index 4acbe5ae316..7290a87a5f5 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringManager.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringManager.cs @@ -9,6 +9,8 @@ partial class LlvmIrModule { protected class LlvmIrStringManager { + readonly string defaultGroupName = "str"; + Dictionary stringSymbolCache = new Dictionary (); Dictionary stringGroupCache = new Dictionary (StringComparer.Ordinal); List stringGroups = new List (); @@ -18,9 +20,13 @@ protected class LlvmIrStringManager public List StringGroups => stringGroups; - public LlvmIrStringManager (TaskLoggingHelper log) + public LlvmIrStringManager (TaskLoggingHelper log, string? defaultStringGroup = null) { this.log = log; + if (!String.IsNullOrEmpty (defaultStringGroup)) { + defaultGroupName = defaultStringGroup; + } + defaultGroup = new LlvmIrStringGroup (); stringGroupCache.Add (String.Empty, defaultGroup); stringGroups.Add (defaultGroup); @@ -52,7 +58,7 @@ LlvmIrStringVariable Add (StringHolder holder, string? groupName = null, string? string groupPrefix; if (String.IsNullOrEmpty (groupName) || String.Compare ("str", groupName, StringComparison.Ordinal) == 0) { group = defaultGroup; - groupPrefix = ".str"; + groupPrefix = $".{defaultGroupName}"; } else if (!stringGroupCache.TryGetValue (groupName, out group) || group == null) { group = new LlvmIrStringGroup (groupComment ?? groupName); stringGroups.Add (group); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrVariable.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrVariable.cs index d6571f3760e..18c67db93a6 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrVariable.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrVariable.cs @@ -301,8 +301,8 @@ class LlvmIrStringVariable : LlvmIrGlobalVariable public string IrType { get; } public bool IsConstantStringLiteral { get; } - public LlvmIrStringVariable (string name, StringHolder value) - : base (typeof(string), name, LlvmIrVariableOptions.LocalString) + public LlvmIrStringVariable (string name, StringHolder value, LlvmIrVariableOptions? options = null) + : base (typeof(string), name, options ?? LlvmIrVariableOptions.GlobalConstexprString) { Value = value; Encoding = value.Encoding; @@ -322,8 +322,8 @@ public LlvmIrStringVariable (string name, StringHolder value) } } - public LlvmIrStringVariable (string name, string value, LlvmIrStringEncoding encoding, StringComparison comparison = StringComparison.Ordinal) - : this (name, new StringHolder (value, encoding, comparison)) + public LlvmIrStringVariable (string name, string value, LlvmIrStringEncoding encoding, StringComparison comparison = StringComparison.Ordinal, LlvmIrVariableOptions? options = null) + : this (name, new StringHolder (value, encoding, comparison), options) {} } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs index c40a0c19628..cf2e1bf15e3 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs @@ -592,6 +592,8 @@ void AddParameter (Type type) protected override void Construct (LlvmIrModule module) { + module.DefaultStringGroup = "mm"; + MapStructures (module); Init (); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs index 3d047b206dc..39c040c0e0e 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs @@ -129,6 +129,8 @@ public TypeMappingDebugNativeAssemblyGenerator (TaskLoggingHelper log, TypeMapGe protected override void Construct (LlvmIrModule module) { + module.DefaultStringGroup = "tmd"; + MapStructures (module); if (data.ManagedToJavaMap != null && data.ManagedToJavaMap.Count > 0) { diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs index eeb9977cd86..a5d75fcb40b 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs @@ -189,6 +189,8 @@ public TypeMappingReleaseNativeAssemblyGenerator (TaskLoggingHelper log, NativeT protected override void Construct (LlvmIrModule module) { + module.DefaultStringGroup = "tmr"; + MapStructures (module); var cs = new ConstructionState (); From af564ccdedcf5c390be7ca60d438a827a29e49ef Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 11 Feb 2025 19:17:10 +0100 Subject: [PATCH 5/6] Fix string constant options --- .../LlvmIrGenerator/LlvmIrGenerator.cs | 23 ++++++++++++++++--- .../Utilities/LlvmIrGenerator/LlvmIrModule.cs | 2 +- .../LlvmIrGenerator/LlvmIrVariable.cs | 2 +- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs index d096ca72146..7b9bc47f96d 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs @@ -205,7 +205,10 @@ void WriteStrings (GeneratorWriteContext context) WriteCommentLine (context, $" '{info.Value}'"); } - WriteGlobalVariableStart (context, info); + WriteGlobalVariableName (context, info); + + // Strings must always be local symbols, global variables will point to them + WriteVariableOptions (context, LlvmIrVariableOptions.LocalString); context.Output.Write ('['); context.Output.Write (size.ToString (CultureInfo.InvariantCulture)); context.Output.Write ($" x {info.IrType}] "); @@ -253,7 +256,7 @@ void WriteGlobalVariables (GeneratorWriteContext context) } } - void WriteGlobalVariableStart (GeneratorWriteContext context, LlvmIrGlobalVariable variable) + void WriteGlobalVariableName (GeneratorWriteContext context, LlvmIrGlobalVariable variable) { if (!String.IsNullOrEmpty (variable.Comment)) { WriteCommentLine (context, variable.Comment); @@ -261,8 +264,10 @@ void WriteGlobalVariableStart (GeneratorWriteContext context, LlvmIrGlobalVariab context.Output.Write ('@'); context.Output.Write (variable.Name); context.Output.Write (" = "); + } - LlvmIrVariableOptions options = variable.Options ?? LlvmIrGlobalVariable.DefaultOptions; + void WriteVariableOptions (GeneratorWriteContext context, LlvmIrVariableOptions options) + { WriteLinkage (context, options.Linkage); WritePreemptionSpecifier (context, options.RuntimePreemption); WriteVisibility (context, options.Visibility); @@ -270,6 +275,18 @@ void WriteGlobalVariableStart (GeneratorWriteContext context, LlvmIrGlobalVariab WriteWritability (context, options.Writability); } + void WriteVariableOptions (GeneratorWriteContext context, LlvmIrGlobalVariable variable, LlvmIrVariableOptions? defaultOptions = null) + { + LlvmIrVariableOptions options = variable.Options ?? defaultOptions ?? LlvmIrGlobalVariable.DefaultOptions; + WriteVariableOptions (context, options); + } + + void WriteGlobalVariableStart (GeneratorWriteContext context, LlvmIrGlobalVariable variable) + { + WriteGlobalVariableName (context, variable); + WriteVariableOptions (context, variable, LlvmIrGlobalVariable.DefaultOptions); + } + void WriteGlobalVariable (GeneratorWriteContext context, LlvmIrGlobalVariable variable) { if (!context.InVariableGroup) { diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs index ab38d7b2ecd..163e60ed51c 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrModule.cs @@ -272,7 +272,7 @@ public LlvmIrGlobalVariable AddGlobalVariable (Type type, string name, object? v LlvmIrGlobalVariable ret; if (type == typeof(string)) { // The cast to `string?` is intentionally meant to throw if `value` type isn't a string. - ret = new LlvmIrStringVariable (name, new StringHolder ((string?)value)) { + ret = new LlvmIrStringVariable (name, new StringHolder ((string?)value), options) { Comment = comment, }; AddStringGlobalVariable ((LlvmIrStringVariable)ret); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrVariable.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrVariable.cs index 18c67db93a6..07badc4fd2c 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrVariable.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrVariable.cs @@ -302,7 +302,7 @@ class LlvmIrStringVariable : LlvmIrGlobalVariable public bool IsConstantStringLiteral { get; } public LlvmIrStringVariable (string name, StringHolder value, LlvmIrVariableOptions? options = null) - : base (typeof(string), name, options ?? LlvmIrVariableOptions.GlobalConstexprString) + : base (typeof(string), name, options ?? LlvmIrVariableOptions.GlobalConstantStringPointer) { Value = value; Encoding = value.Encoding; From 5f9205fb9382d87c7785d601584e8fd60e2cea28 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Wed, 12 Feb 2025 15:56:58 +0100 Subject: [PATCH 6/6] Address feedback --- .../Utilities/LlvmIrGenerator/StringHolder.cs | 56 +++++++------------ 1 file changed, 20 insertions(+), 36 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/StringHolder.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/StringHolder.cs index cbcfd02c9d8..1043f1dc145 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/StringHolder.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/StringHolder.cs @@ -34,15 +34,7 @@ public static StringHolder AsHolder (object? value, LlvmIrStringEncoding encodin return holder; } - public int CompareTo (object obj) - { - var holder = obj as StringHolder; - if (holder == null) { - return 1; - } - - return CompareTo (holder); - } + public int CompareTo (object obj) => CompareTo (obj as StringHolder); public int CompareTo (StringHolder? other) { @@ -83,15 +75,7 @@ public override int GetHashCode () return hc ^ Encoding.GetHashCode (); } - public override bool Equals (object obj) - { - var holder = obj as StringHolder; - if (holder == null) { - return false; - } - - return Equals (holder); - } + public override bool Equals (object obj) => Equals (obj as StringHolder); public bool Equals (StringHolder? other) { @@ -103,22 +87,22 @@ public bool Equals (StringHolder? other) } public static bool operator > (StringHolder a, StringHolder b) - { - return a.CompareTo (b) > 0; - } - - public static bool operator < (StringHolder a, StringHolder b) - { - return a.CompareTo (b) < 0; - } - - public static bool operator >= (StringHolder a, StringHolder b) - { - return a.CompareTo (b) >= 0; - } - - public static bool operator <= (StringHolder a, StringHolder b) - { - return a.CompareTo (b) <= 0; - } + { + return a.CompareTo (b) > 0; + } + + public static bool operator < (StringHolder a, StringHolder b) + { + return a.CompareTo (b) < 0; + } + + public static bool operator >= (StringHolder a, StringHolder b) + { + return a.CompareTo (b) >= 0; + } + + public static bool operator <= (StringHolder a, StringHolder b) + { + return a.CompareTo (b) <= 0; + } }