Map struct to record classes (and structs), use default Equals/GetHas… #2235
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bernardnormier
commented
May 29, 2024
| { | ||
| baseNames.push_back("System.IEquatable<" + name + ">"); | ||
| } | ||
| _out << nl << "public " << (classMapping ? "sealed partial record class" : "partial record struct") << ' ' << name; |
There was a problem hiding this comment.
It is common for record classes and structs to be read-only. However, here they are read-write for backwards source compatibility.
| @@ -2235,40 +2229,6 @@ Slice::Gen::TypesVisitor::visitStructEnd(const StructPtr& p) | |||
| _out << nl << "ice_initialize();"; | |||
| _out << eb; | |||
|
|
|||
| if (classMapping) | |||
There was a problem hiding this comment.
We get all this (and more) for free from the record class.
| @@ -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) | |||
There was a problem hiding this comment.
This code is still bogus and used internally. Will cleanup in a follow-up PR.
| @@ -28,40 +28,38 @@ private static void allTests(Ice.Communicator communicator) | |||
| { | |||
| S2 v; | |||
|
|
|||
| v = def_s2.Clone(); | |||
| test(v.Equals(def_s2)); | |||
| v = def_s2 with { }; | |||
There was a problem hiding this comment.
I kept mostly the same logic in this test. Just cleaned it up.
externl
approved these changes
May 29, 2024
pepone
approved these changes
May 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…hCode
This PR changes the struct-to-C# class mapping: the mapped class is not a (read-write) record class with its default Equals and GetHashCode implementation.
Fixes #2214.