From 569b5260430dd7b73603348f5d2586f62e6392cb Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Fri, 6 Dec 2019 22:54:25 +0100 Subject: [PATCH] Simplify handling of accessibilities. --- .../TypeSystem/Accessibility.cs | 80 ++++++++++++++++--- .../Implementation/MetadataProperty.cs | 29 +------ ILSpy/Analyzers/AnalyzerScope.cs | 42 ++++------ 3 files changed, 85 insertions(+), 66 deletions(-) diff --git a/ICSharpCode.Decompiler/TypeSystem/Accessibility.cs b/ICSharpCode.Decompiler/TypeSystem/Accessibility.cs index 5d7b5a13b..9cee2d55e 100644 --- a/ICSharpCode.Decompiler/TypeSystem/Accessibility.cs +++ b/ICSharpCode.Decompiler/TypeSystem/Accessibility.cs @@ -16,6 +16,9 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. +using System.Diagnostics; +using ICSharpCode.Decompiler.Util; + namespace ICSharpCode.Decompiler.TypeSystem { /// @@ -23,8 +26,8 @@ namespace ICSharpCode.Decompiler.TypeSystem /// public enum Accessibility : byte { - // note: some code depends on the fact that these values are within the range 0-7 - + // Note: some code depends on the fact that these values are within the range 0-7 + /// /// The entity is completely inaccessible. This is used for C# explicit interface implementations. /// @@ -34,26 +37,83 @@ namespace ICSharpCode.Decompiler.TypeSystem /// Private, /// - /// The entity is accessible everywhere. + /// The entity is accessible in derived classes within the same assembly. + /// This corresponds to C# private protected. /// - Public, + ProtectedAndInternal, /// /// The entity is only accessible within the same class and in derived classes. /// Protected, /// - /// The entity is accessible within the same project content. + /// The entity is accessible within the same assembly. /// Internal, /// - /// The entity is accessible both everywhere in the project content, and in all derived classes. + /// The entity is accessible both everywhere in the assembly, and in all derived classes. + /// This corresponds to C# protected internal. /// - /// This corresponds to C# 'protected internal'. ProtectedOrInternal, /// - /// The entity is accessible in derived classes within the same project content. + /// The entity is accessible everywhere. /// - /// This corresponds to C# 'private protected'. - ProtectedAndInternal, + Public, + } + + public static class AccessibilityExtensions + { + // This code depends on the fact that the enum values are sorted similar to the partial order + // where an accessibility is smaller than another if it makes an entity visibible to a subset of the code: + // digraph Accessibilities { + // none -> private -> protected_and_internal -> protected -> protected_or_internal -> public; + // none -> private -> protected_and_internal -> internal -> protected_or_internal -> public; + // } + + /// + /// Gets whether a <= b in the partial order of accessibilities: + /// return true if b is accessible everywhere where a is accessible. + /// + public static bool LessThanOrEqual(this Accessibility a, Accessibility b) + { + // Exploit the enum order being similar to the partial order to dramatically simplify the logic here: + // protected vs. internal is the only pair for which the enum value order doesn't match the partial order + return a <= b && !(a == Accessibility.Protected && b == Accessibility.Internal); + } + + /// + /// Computes the intersection of the two accessibilities: + /// The result is accessible from any given point in the code + /// iff both a and b are accessible from that point. + /// + public static Accessibility Intersect(this Accessibility a, Accessibility b) + { + if (a > b) { + ExtensionMethods.Swap(ref a, ref b); + } + if (a == Accessibility.Protected && b == Accessibility.Internal) { + return Accessibility.ProtectedAndInternal; + } else { + Debug.Assert(!(a == Accessibility.Internal && b == Accessibility.Protected)); + return a; + } + } + + /// + /// Computes the union of the two accessibilities: + /// The result is accessible from any given point in the code + /// iff at least one of a or b is accessible from that point. + /// + public static Accessibility Union(this Accessibility a, Accessibility b) + { + if (a > b) { + ExtensionMethods.Swap(ref a, ref b); + } + if (a == Accessibility.Protected && b == Accessibility.Internal) { + return Accessibility.ProtectedOrInternal; + } else { + Debug.Assert(!(a == Accessibility.Internal && b == Accessibility.Protected)); + return b; + } + } } } diff --git a/ICSharpCode.Decompiler/TypeSystem/Implementation/MetadataProperty.cs b/ICSharpCode.Decompiler/TypeSystem/Implementation/MetadataProperty.cs index 264bb5e02..cd98cf907 100644 --- a/ICSharpCode.Decompiler/TypeSystem/Implementation/MetadataProperty.cs +++ b/ICSharpCode.Decompiler/TypeSystem/Implementation/MetadataProperty.cs @@ -196,37 +196,10 @@ namespace ICSharpCode.Decompiler.TypeSystem.Implementation return baseMember.Accessibility; } } - return MergePropertyAccessibility( + return AccessibilityExtensions.Union( this.Getter?.Accessibility ?? Accessibility.None, this.Setter?.Accessibility ?? Accessibility.None); } - - static internal Accessibility MergePropertyAccessibility(Accessibility left, Accessibility right) - { - if (left == Accessibility.Public || right == Accessibility.Public) - return Accessibility.Public; - - if (left == Accessibility.ProtectedOrInternal || right == Accessibility.ProtectedOrInternal) - return Accessibility.ProtectedOrInternal; - - if (left == Accessibility.Protected && right == Accessibility.Internal || - left == Accessibility.Internal && right == Accessibility.Protected) - return Accessibility.ProtectedOrInternal; - - if (left == Accessibility.Protected || right == Accessibility.Protected) - return Accessibility.Protected; - - if (left == Accessibility.Internal || right == Accessibility.Internal) - return Accessibility.Internal; - - if (left == Accessibility.ProtectedAndInternal || right == Accessibility.ProtectedAndInternal) - return Accessibility.ProtectedAndInternal; - - if (left == Accessibility.Private || right == Accessibility.Private) - return Accessibility.Private; - - return left; - } #endregion public bool IsStatic => AnyAccessor?.IsStatic ?? false; diff --git a/ILSpy/Analyzers/AnalyzerScope.cs b/ILSpy/Analyzers/AnalyzerScope.cs index 7e6c6c901..68eac7f9d 100644 --- a/ILSpy/Analyzers/AnalyzerScope.cs +++ b/ILSpy/Analyzers/AnalyzerScope.cs @@ -45,7 +45,7 @@ namespace ICSharpCode.ILSpy.Analyzers public ITypeDefinition TypeScope => typeScope; - Accessibility memberAccessibility, typeAccessibility; + Accessibility effectiveAccessibility; public AnalyzerScope(AssemblyList assemblyList, IEntity entity) { @@ -53,13 +53,12 @@ namespace ICSharpCode.ILSpy.Analyzers AnalyzedSymbol = entity; if (entity is ITypeDefinition type) { typeScope = type; - memberAccessibility = Accessibility.None; + effectiveAccessibility = DetermineEffectiveAccessibility(ref typeScope); } else { typeScope = entity.DeclaringTypeDefinition; - memberAccessibility = entity.Accessibility; + effectiveAccessibility = DetermineEffectiveAccessibility(ref typeScope, entity.Accessibility); } - typeAccessibility = DetermineTypeAccessibility(ref typeScope); - IsLocal = memberAccessibility == Accessibility.Private || typeAccessibility == Accessibility.Private; + IsLocal = effectiveAccessibility.LessThanOrEqual(Accessibility.Private); } public IEnumerable GetModulesInScope(CancellationToken ct) @@ -67,10 +66,7 @@ namespace ICSharpCode.ILSpy.Analyzers if (IsLocal) return new[] { TypeScope.ParentModule.PEFile }; - if (memberAccessibility == Accessibility.Internal || - memberAccessibility == Accessibility.ProtectedOrInternal || - typeAccessibility == Accessibility.Internal || - typeAccessibility == Accessibility.ProtectedAndInternal) + if (effectiveAccessibility.LessThanOrEqual(Accessibility.Internal)) return GetModuleAndAnyFriends(TypeScope, ct); return GetReferencingModules(TypeScope.ParentModule.PEFile, ct); @@ -89,11 +85,7 @@ namespace ICSharpCode.ILSpy.Analyzers { if (IsLocal) { var typeSystem = new DecompilerTypeSystem(TypeScope.ParentModule.PEFile, TypeScope.ParentModule.PEFile.GetAssemblyResolver()); - ITypeDefinition scope = typeScope; - if (memberAccessibility != Accessibility.Private && typeScope.DeclaringTypeDefinition != null) { - scope = typeScope.DeclaringTypeDefinition; - } - foreach (var type in TreeTraversal.PreOrder(scope, t => t.NestedTypes)) { + foreach (var type in TreeTraversal.PreOrder(typeScope, t => t.NestedTypes)) { yield return type; } } else { @@ -106,23 +98,17 @@ namespace ICSharpCode.ILSpy.Analyzers } } - Accessibility DetermineTypeAccessibility(ref ITypeDefinition typeScope) + static Accessibility DetermineEffectiveAccessibility(ref ITypeDefinition typeScope, Accessibility memberAccessibility = Accessibility.Public) { - var typeAccessibility = typeScope.Accessibility; - while (typeScope.DeclaringType != null) { - Accessibility accessibility = typeScope.Accessibility; - if ((int)typeAccessibility > (int)accessibility) { - typeAccessibility = accessibility; - if (typeAccessibility == Accessibility.Private) - break; - } + Accessibility accessibility = memberAccessibility; + while (typeScope.DeclaringTypeDefinition != null && !accessibility.LessThanOrEqual(Accessibility.Private)) { + accessibility = accessibility.Intersect(typeScope.Accessibility); typeScope = typeScope.DeclaringTypeDefinition; } - - if ((int)typeAccessibility > (int)Accessibility.Internal) { - typeAccessibility = Accessibility.Internal; - } - return typeAccessibility; + // Once we reach a private entity, we leave the loop with typeScope set to the class that + // contains the private entity = the scope that needs to be searched. + // Otherwise (if we don't find a private entity) we return the top-level class. + return accessibility; } #region Find modules