Simplify handling of accessibilities.

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
 {
 	/// <summary>
@@ -23,8 +26,8 @@ namespace ICSharpCode.Decompiler.TypeSystem
 	/// </summary>
 	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
-		
+
 		/// <summary>
 		/// The entity is completely inaccessible. This is used for C# explicit interface implementations.
 		/// </summary>
@@ -34,26 +37,83 @@ namespace ICSharpCode.Decompiler.TypeSystem
 		/// </summary>
 		Private,
 		/// <summary>
-		/// The entity is accessible everywhere.
+		/// The entity is accessible in derived classes within the same assembly.
+		/// This corresponds to C# <c>private protected</c>.
 		/// </summary>
-		Public,
+		ProtectedAndInternal,
 		/// <summary>
 		/// The entity is only accessible within the same class and in derived classes.
 		/// </summary>
 		Protected,
 		/// <summary>
-		/// The entity is accessible within the same project content.
+		/// The entity is accessible within the same assembly.
 		/// </summary>
 		Internal,
 		/// <summary>
-		/// 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# <c>protected internal</c>.
 		/// </summary>
-		/// <remarks>This corresponds to C# 'protected internal'.</remarks>
 		ProtectedOrInternal,
 		/// <summary>
-		/// The entity is accessible in derived classes within the same project content.
+		/// The entity is accessible everywhere.
 		/// </summary>
-		/// <remarks>This corresponds to C# 'private protected'.</remarks>
+		Public,
-		ProtectedAndInternal,
+	}
+
+	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;
+		// }
+
+		/// <summary>
+		/// Gets whether a &lt;= b in the partial order of accessibilities:
+		/// return true if b is accessible everywhere where a is accessible.
+		/// </summary>
+		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);
+		}
+
+		/// <summary>
+		/// 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.
+		/// </summary>
+		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;
+			}
+		}
+
+		/// <summary>
+		/// 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.
+		/// </summary>
+		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;
+			}
+		}
 	}
 }

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;

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 = effectiveAccessibility.LessThanOrEqual(Accessibility.Private);
-			IsLocal = memberAccessibility == Accessibility.Private || typeAccessibility == Accessibility.Private;
 		}
 
 		public IEnumerable<PEFile> GetModulesInScope(CancellationToken ct)
@@ -67,10 +66,7 @@ namespace ICSharpCode.ILSpy.Analyzers
 			if (IsLocal)
 				return new[] { TypeScope.ParentModule.PEFile };
 
-			if (memberAccessibility == Accessibility.Internal ||
+			if (effectiveAccessibility.LessThanOrEqual(Accessibility.Internal))
-				memberAccessibility == Accessibility.ProtectedOrInternal ||
-				typeAccessibility == Accessibility.Internal ||
-				typeAccessibility == Accessibility.ProtectedAndInternal)
 				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;
+				foreach (var type in TreeTraversal.PreOrder(typeScope, t => t.NestedTypes)) {
-				if (memberAccessibility != Accessibility.Private && typeScope.DeclaringTypeDefinition != null) {
-					scope = typeScope.DeclaringTypeDefinition;
-				}
-				foreach (var type in TreeTraversal.PreOrder(scope, 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;
+			Accessibility accessibility = memberAccessibility;
-			while (typeScope.DeclaringType != null) {
+			while (typeScope.DeclaringTypeDefinition != null && !accessibility.LessThanOrEqual(Accessibility.Private)) {
-				Accessibility accessibility = typeScope.Accessibility;
+				accessibility = accessibility.Intersect(typeScope.Accessibility);
-				if ((int)typeAccessibility > (int)accessibility) {
-					typeAccessibility = accessibility;
-					if (typeAccessibility == Accessibility.Private)
-						break;
-				}
 				typeScope = typeScope.DeclaringTypeDefinition;
 			}
-
+			// Once we reach a private entity, we leave the loop with typeScope set to the class that
-			if ((int)typeAccessibility > (int)Accessibility.Internal) {
+			// contains the private entity = the scope that needs to be searched.
-				typeAccessibility = Accessibility.Internal;
+			// Otherwise (if we don't find a private entity) we return the top-level class.
-			}
+			return accessibility;
-			return typeAccessibility;
 		}
 
 		#region Find modules