Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map struct to record classes (and structs), use default Equals/GetHas… #2235

Merged
merged 2 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 2 additions & 113 deletions cpp/src/slice2cs/Gen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2111,18 +2111,12 @@ Slice::Gen::TypesVisitor::visitStructStart(const StructPtr& p)

emitAttributes(p);
emitPartialTypeAttributes();
_out << nl << "public " << (classMapping ? "sealed partial class" : "partial record struct") << ' ' << name;

StringList baseNames;

if (classMapping)
{
baseNames.push_back("System.IEquatable<" + name + ">");
}
_out << nl << "public " << (classMapping ? "sealed partial record class" : "partial record struct") << ' ' << name;
Copy link
Member Author

Choose a reason for hiding this comment

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

It is common for record classes and structs to be read-only. However, here they are read-write for backwards source compatibility.


//
// Check for cs:implements metadata.
//
StringList baseNames;
const StringList metaData = p->getMetaData();
static const string prefix = "cs:implements:";
for (StringList::const_iterator q = metaData.begin(); q != metaData.end(); ++q)
Expand Down Expand Up @@ -2235,40 +2229,6 @@ Slice::Gen::TypesVisitor::visitStructEnd(const StructPtr& p)
_out << nl << "ice_initialize();";
_out << eb;

if (classMapping)
Copy link
Member Author

Choose a reason for hiding this comment

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

We get all this (and more) for free from the record class.

{
_out << sp;
emitGeneratedCodeAttribute();
_out << nl << "public " << name << " Clone() => (" << name << ")MemberwiseClone();";

_out << sp;
emitGeneratedCodeAttribute();
_out << nl << "public override int GetHashCode()";
_out << sb;
writeMemberHashCode(dataMembers);
_out << eb;

_out << sp;
emitGeneratedCodeAttribute();
_out << nl << "public override bool Equals(object? other) => Equals(other as " << name << ");";

_out << sp;
emitGeneratedCodeAttribute();
_out << nl << "public bool Equals(" << name << "? other)";
_out << sb;
writeMemberEquals(dataMembers);
_out << eb;

_out << sp;
emitGeneratedCodeAttribute();
_out << nl << "public static bool operator ==(" << name << "? lhs, " << name << "? rhs) => ";
_out << "lhs is not null ? lhs.Equals(rhs) : rhs is null;";

_out << sp;
emitGeneratedCodeAttribute();
_out << nl << "public static bool operator !=(" << name << "? lhs, " << name << "? rhs) => !(lhs == rhs);";
}

_out << sp;
emitGeneratedCodeAttribute();
_out << nl << "public void ice_writeMembers(Ice.OutputStream ostr)";
Expand Down Expand Up @@ -2453,77 +2413,6 @@ Slice::Gen::TypesVisitor::visitDataMember(const DataMemberPtr& p)
}
}

void
Slice::Gen::TypesVisitor::writeMemberHashCode(const DataMemberList& dataMembers)
{
_out << nl << "var hash = new global::System.HashCode();";
assert(dataMembers.size() > 0); // a Slice struct must have at least one field.

for (const auto& q : dataMembers)
{
TypePtr memberType = q->type();
string memberName = fixId(q->name(), DotNet::ICloneable);

if (dynamic_pointer_cast<Sequence>(memberType) || dynamic_pointer_cast<Dictionary>(memberType))
{
_out << nl << "Ice.UtilInternal.Collections.HashCodeAdd(ref hash, this." << memberName << ");";
}
else
{
_out << nl << "hash.Add(this." << memberName << ");";
}
}
_out << nl << "return hash.ToHashCode();";
}

void
Slice::Gen::TypesVisitor::writeMemberEquals(const DataMemberList& dataMembers)
{
_out << nl << "if (ReferenceEquals(this, other))";
_out << sb;
_out << nl << "return true;";
_out << eb;
_out << nl << "return other is not null";

assert(!dataMembers.empty()); // a Slice struct must have at least one field.

_out.inc();

for (DataMemberList::const_iterator q = dataMembers.begin(); q != dataMembers.end(); ++q)
{
assert(!(*q)->optional()); // a Slice struct does not have optional fields.

_out << " && " << nl;

string memberName = fixId((*q)->name(), DotNet::ICloneable);
TypePtr memberType = (*q)->type();

if (SequencePtr seq = dynamic_pointer_cast<Sequence>(memberType))
{
_out << "Ice.UtilInternal.Collections.NullableSequenceEqual(this." << memberName << ", other." << memberName
<< ")";
}
else if (DictionaryPtr dict = dynamic_pointer_cast<Dictionary>(memberType))
{
// Equals() for generic types does not have value semantics.
_out << "Ice.UtilInternal.Collections.DictionaryEquals(this." << memberName << ", other." << memberName
<< ")";
}
else if (isProxyType(memberType))
{
// We need to cast it to the base concrete type to get ==
_out << "(Ice.ObjectPrxHelperBase?)this." << memberName << " == (Ice.ObjectPrxHelperBase?)other."
<< memberName;
}
else
{
_out << "this." << memberName << " == other." << memberName;
}
}
_out.dec();
_out << ";";
}

Slice::Gen::ResultVisitor::ResultVisitor(::IceUtilInternal::Output& out) : CsVisitor(out) {}

namespace
Expand Down
5 changes: 0 additions & 5 deletions cpp/src/slice2cs/Gen.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,6 @@ namespace Slice
virtual void visitEnum(const EnumPtr&);
virtual void visitConst(const ConstPtr&);
virtual void visitDataMember(const DataMemberPtr&);

private:
// For Slice structs mapped to C# classes
void writeMemberHashCode(const DataMemberList&);
void writeMemberEquals(const DataMemberList&);
};

class ResultVisitor : public CsVisitor
Expand Down
2 changes: 1 addition & 1 deletion csharp/src/Ice/Internal/Reference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public Reference changeIdentity(Ice.Identity newIdentity)
}
Reference r = _instance.referenceFactory().copy(this);
// Identity is a reference type, therefore we make a copy of newIdentity.
r._identity = newIdentity.Clone();
r._identity = newIdentity with { };
return r;
}

Expand Down
2 changes: 1 addition & 1 deletion csharp/src/Ice/Proxy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,7 @@ public string ice_id(Dictionary<string, string>? context = null)
/// Returns the identity embedded in this proxy.
/// <returns>The identity of the target object.</returns>
/// </summary>
public Identity ice_getIdentity() => _reference.getIdentity().Clone();
public Identity ice_getIdentity() => _reference.getIdentity() with { };

/// <summary>
/// Creates a new proxy that is identical to this proxy, except for the per-proxy context.
Expand Down
26 changes: 5 additions & 21 deletions csharp/src/Ice/UtilInternal/Collections.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,10 @@

namespace Ice.UtilInternal;

public sealed class Collections
public static class Collections
{
// Just like Enumerable.SequenceEqual except it also handles null sequences.
// TODO: Note that this code would be removed by proposal #2115.
public static bool NullableSequenceEqual<TSource>(IEnumerable<TSource> lhs, IEnumerable<TSource> rhs)
{
if (lhs is null)
{
return rhs is null || !rhs.Any();
}
if (rhs is null)
{
return !lhs.Any();
}

return lhs.SequenceEqual(rhs);
}

// Seq is a nullable seq.
public static void HashCodeAdd<TSource>(ref HashCode hash, IEnumerable<TSource> seq)
internal static void HashCodeAdd<TSource>(ref HashCode hash, IEnumerable<TSource> seq)
{
if (seq is not null)
{
Expand All @@ -35,7 +19,7 @@ public static void HashCodeAdd<TSource>(ref HashCode hash, IEnumerable<TSource>
}
}

public static bool DictionaryEquals(IDictionary d1, IDictionary d2)
internal static bool DictionaryEquals(IDictionary d1, IDictionary d2)
Copy link
Member Author

Choose a reason for hiding this comment

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

This code is still bogus and used internally. Will cleanup in a follow-up PR.

{
if (ReferenceEquals(d1, d2))
{
Expand Down Expand Up @@ -77,7 +61,7 @@ public static bool DictionaryEquals(IDictionary d1, IDictionary d2)
return false;
}

public static void HashCodeAdd<TKey, TValue>(ref HashCode hash, IDictionary<TKey, TValue> d) where TKey : notnull
internal static void HashCodeAdd<TKey, TValue>(ref HashCode hash, IDictionary<TKey, TValue> d) where TKey : notnull
{
if (d is not null)
{
Expand Down Expand Up @@ -107,7 +91,7 @@ public static void Shuffle<T>(ref List<T> l)
}
}

public static void Sort<T>(ref List<T> array, IComparer<T> comparator)
internal static void Sort<T>(ref List<T> array, IComparer<T> comparator)
{
//
// This Sort method implements the merge sort algorithm
Expand Down
2 changes: 1 addition & 1 deletion csharp/test/Ice/objects/AllTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public partial record struct S1
}
}

public partial class SC1
public partial record class SC1
{
partial void ice_initialize()
{
Expand Down
Loading
Loading