-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Description
Summary of the new feature/enhancement
When assigning a struct to a variable, some checks are done to determine if the struct is mutable. The current code path checks if there are:
- Any public instance fields
- Any public instance properties with a set accessor
- Any public instance methods
This can be simplified by checking if the struct is decorated with the IsReadOnlyAttribute. This would allow PS to assume any struct that wasn't declared explicitly as read only with the C# readonly modifier (SharpLab example) to be mutable. This would also probably result in less false positives because most structs have a public field (even if initonly) or a public method, even these explicitly readonly structs.
Proposed technical implementation details (optional)
This method:
PowerShell/src/System.Management.Automation/engine/runtime/Binding/Binders.cs
Lines 2030 to 2063 in 947bddf
| internal static bool IsValueTypeMutable(Type type) | |
| { | |
| if (type.IsPrimitive || type.IsEnum) | |
| { | |
| return false; | |
| } | |
| // If there are any fields, the type is mutable. | |
| if (type.GetFields(BindingFlags.Public | BindingFlags.Instance).Any()) | |
| { | |
| return true; | |
| } | |
| // If there are any properties with setters, the type is mutable. | |
| var properties = type.GetProperties(BindingFlags.Public | BindingFlags.Instance); | |
| for (int index = 0; index < properties.Length; index++) | |
| { | |
| var property = properties[index]; | |
| if (property.CanWrite) | |
| { | |
| return true; | |
| } | |
| } | |
| // If there are any methods other than the property getters, the type might | |
| // be mutable, so assume the type is mutable. | |
| var methods = type.GetMethods(BindingFlags.Public | BindingFlags.Instance); | |
| if (methods.Length != properties.Length) | |
| { | |
| return true; | |
| } | |
| return false; | |
| } |
Should be changed like this:
diff --git a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs
index b54fbdfd5..7a0254ccb 100644
--- a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs
+++ b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs
@@ -2034,32 +2034,7 @@ namespace System.Management.Automation.Language
return false;
}
- // If there are any fields, the type is mutable.
- if (type.GetFields(BindingFlags.Public | BindingFlags.Instance).Any())
- {
- return true;
- }
-
- // If there are any properties with setters, the type is mutable.
- var properties = type.GetProperties(BindingFlags.Public | BindingFlags.Instance);
- for (int index = 0; index < properties.Length; index++)
- {
- var property = properties[index];
- if (property.CanWrite)
- {
- return true;
- }
- }
-
- // If there are any methods other than the property getters, the type might
- // be mutable, so assume the type is mutable.
- var methods = type.GetMethods(BindingFlags.Public | BindingFlags.Instance);
- if (methods.Length != properties.Length)
- {
- return true;
- }
-
- return false;
+ return !type.IsDefined<System.Runtime.CompilerServices.IsReadOnlyAttribute>();
}
private static readonly ConcurrentDictionary<Type, bool> s_mutableValueTypesWithInstanceMembers =Related issue #12411
/cc @mklement0