Skip to content

Improve mutable struct detection #12430

@SeeminglyScience

Description

@SeeminglyScience

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:

  1. Any public instance fields
  2. Any public instance properties with a set accessor
  3. 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:

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Issue-Enhancementthe issue is more of a feature request than a bugUp-for-GrabsUp-for-grabs issues are not high priorities, and may be opportunities for external contributorsWG-Enginecore PowerShell engine, interpreter, and runtime

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions