[Mono-dev] [PATCH] Friend access for class members

Marek Safar marek.safar at seznam.cz
Fri Oct 28 09:51:22 EDT 2005


Hello Carlos,

My comments.

+        foreach (object o in attrs) {
+            InternalsVisibleToAttribute attr = o as 
InternalsVisibleToAttribute;

I think you can use InternalsVisibleToAttribute directly in foreach

+    static string this_fullname;
+   
+    static void Error_FriendAccessNameNotMatching (string other_name)
+    {
+        if (this_fullname == null)
+            this_fullname = CodeGen.Assembly.Name.FullName;
+       
+        Report.Error (281, "Friend access was granted to `" + other_name +
+                "', but the output assembly is named `" + this_fullname +
+                "'. Try adding a reference to `" + other_name +
+                "' or change the output assembly name to match it.");
+    }

1. Do you really need this `string this_fullname'
2. Please don't end error message with '.'
3. Please use "bla `{0}' bla" syntax, it is easier to read.

Marek


>I forgot to attach the patch ;-)
>
>Carlos.
>
>
>El vie, 28-10-2005 a las 16:23 +0530, Raja R Harinath escribió:
>  
>
>>Hi,
>>
>>Carlos Alberto Cortez <calberto.cortez at gmail.com> writes:
>>
>>    
>>
>>>The attached patch implements friend access for class members (methods,
>>>properties, fields). 
>>>
>>>I'm not including the type-check section, since that part will be
>>>modified (next merge to gmcs) and I'm waiting for that to happen.
>>>However, that change should be small (I will send the patch).
>>>      
>>>
>>I have completed the merge of the relevant parts.  Please post an
>>updated patch.
>>
>>Meanwhile, I have some comments:
>>
>>[snip]
>>    
>>
>>>@@ -2646,25 +2726,38 @@
>>> 				MethodBase mb = (MethodBase) m;
>>> 				MethodAttributes ma = mb.Attributes & MethodAttributes.MemberAccessMask;
>>> 
>>>+				if (ma == MethodAttributes.Public)
>>>+					return true;
>>>      
>>>
>>Ok.
>>
>>    
>>
>>> 				if (ma == MethodAttributes.Private)
>>> 					return private_ok ||
>>> 						IsPrivateAccessible (invocation_type, m.DeclaringType) ||
>>> 						IsNestedChildOf (invocation_type, m.DeclaringType);
>>> 
>>>-				if (invocation_assembly == mb.DeclaringType.Assembly) {
>>>+				if (invocation_assembly == mb.DeclaringType.Assembly)
>>> 					if (ma == MethodAttributes.Assembly || ma == MethodAttributes.FamORAssem)
>>> 						return true;
>>>-				} else {
>>>-					if (ma == MethodAttributes.Assembly || ma == MethodAttributes.FamANDAssem)
>>>-						return false;
>>>      
>>>
>>I would retain the old code, and change the check to:
>>
>>  if (invocation_assembly == mb.DeclaringType.Assembly ||
>>      TypeManager.InternalsVisibleTo (invocation_assembly, mb.DeclaringType.Assembly)) 
>>
>>    
>>
>>>+					
>>>+				if (ma == MethodAttributes.Family ||
>>>+				    ma == MethodAttributes.FamANDAssem ||
>>>+				    ma == MethodAttributes.FamORAssem) {
>>>+					if (!CheckValidFamilyAccess (mb.IsStatic, m)) {
>>>+						if (ma == MethodAttributes.Family || ma == MethodAttributes.FamANDAssem)
>>>+							return false;
>>>+					} else {
>>>+						// We are valid
>>>+						if (ma == MethodAttributes.Family || ma == MethodAttributes.FamORAssem)
>>>+							return true;
>>>+						
>>>+						// Check for FamANDAssem
>>>+						if (invocation_assembly == mb.DeclaringType.Assembly)
>>>+							return true;
>>>+					}
>>> 				}
>>>-				if (ma == MethodAttributes.Family ||
>>>-				    ma == MethodAttributes.FamANDAssem ||
>>>-				    ma == MethodAttributes.FamORAssem)
>>>-					return CheckValidFamilyAccess (mb.IsStatic, m);
>>>      
>>>
>>I don't like this too much.  I'd much rather keep the old code.
>>
>>- Hari
>>    
>>
>>------------------------------------------------------------------------
>>
>>Index: typemanager.cs
>>===================================================================
>>--- typemanager.cs	(revisión: 52315)
>>+++ typemanager.cs	(copia de trabajo)
>>@@ -252,6 +252,8 @@
>> 	static Hashtable fieldbuilders_to_fields;
>> 	static Hashtable fields;
>> 
>>+	static PtrHashtable assembly_internals_vis_attrs;
>>+
>> 	struct Signature {
>> 		public string name;
>> 		public Type [] args;
>>@@ -274,6 +276,8 @@
>> 		priv_fields_events = null;
>> 
>> 		type_hash = null;
>>+
>>+		assembly_internals_vis_attrs = null;
>> 		
>> 		CleanUpGenerics ();
>> 		TypeHandle.CleanUp ();
>>@@ -375,6 +379,8 @@
>> 		fieldbuilders_to_fields = new Hashtable ();
>> 		fields = new Hashtable ();
>> 		type_hash = new DoubleHash ();
>>+
>>+		assembly_internals_vis_attrs = new PtrHashtable ();
>> 		
>> 		InitGenerics ();
>> 	}
>>@@ -1654,6 +1660,80 @@
>> 		return false;
>> 	}
>> 
>>+	//
>>+	// Checks whether `extern_type' is friend of the output assembly
>>+	//
>>+	public static bool IsFriendAssembly (Assembly assembly)
>>+	{
>>+		if (assembly_internals_vis_attrs.Contains (assembly))
>>+			return (bool)(assembly_internals_vis_attrs [assembly]);
>>+		
>>+		object [] attrs = assembly.GetCustomAttributes (internals_visible_attr_type, false);
>>+		if (attrs.Length == 0) {
>>+			AddFriendAssembly (assembly, false);
>>+			return false;
>>+		}
>>+
>>+		AssemblyName this_name = CodeGen.Assembly.Name;
>>+		byte [] this_token = this_name.GetPublicKeyToken ();
>>+		bool is_friend = false;
>>+		foreach (object o in attrs) {
>>+			InternalsVisibleToAttribute attr = o as InternalsVisibleToAttribute;
>>+			if (attr.AssemblyName == null || attr.AssemblyName.Length == 0)
>>+				continue;
>>+			
>>+			AssemblyName aname = null;
>>+			try {
>>+				aname = new AssemblyName (attr.AssemblyName);
>>+			} catch (FileLoadException) {
>>+			} catch (ArgumentException) {
>>+			}
>>+
>>+			if (aname == null || aname.Name != this_name.Name)
>>+				continue;
>>+			
>>+			byte [] key_token = aname.GetPublicKeyToken ();
>>+			if (key_token != null) {
>>+				if (this_token == null) {
>>+					// Same name, but key token is null
>>+					Error_FriendAccessNameNotMatching (aname.FullName);
>>+					break;
>>+				}
>>+				
>>+				if (!CompareKeyTokens (this_token, key_token))
>>+					continue;
>>+			}
>>+
>>+			is_friend = true;
>>+			break;
>>+		}
>>+
>>+		AddFriendAssembly (assembly, is_friend);
>>+		return is_friend;
>>+	}
>>+
>>+	static bool CompareKeyTokens (byte [] token1, byte [] token2)
>>+	{
>>+		for (int i = 0; i < token1.Length; i++)
>>+			if (token1 [i] != token2 [i])
>>+				return false;
>>+
>>+		return true;
>>+	}
>>+
>>+	static string this_fullname;
>>+	
>>+	static void Error_FriendAccessNameNotMatching (string other_name)
>>+	{
>>+		if (this_fullname == null)
>>+			this_fullname = CodeGen.Assembly.Name.FullName;
>>+		
>>+		Report.Error (281, "Friend access was granted to `" + other_name + 
>>+				"', but the output assembly is named `" + this_fullname +
>>+				"'. Try adding a reference to `" + other_name + 
>>+				"' or change the output assembly name to match it.");
>>+	}
>>+	
>>         //
>>         // Do the right thing when returning the element type of an
>>         // array type based on whether we are compiling corlib or not
>>@@ -2478,12 +2558,16 @@
>> 				MethodBase mb = (MethodBase) m;
>> 				MethodAttributes ma = mb.Attributes & MethodAttributes.MemberAccessMask;
>> 
>>+				if (ma == MethodAttributes.Public)
>>+					return true;
>>+				
>> 				if (ma == MethodAttributes.Private)
>> 					return private_ok ||
>> 						IsPrivateAccessible (invocation_type, m.DeclaringType) ||
>> 						IsNestedChildOf (invocation_type, m.DeclaringType);
>> 
>>-				if (invocation_assembly == mb.DeclaringType.Assembly) {
>>+				if (invocation_assembly == mb.DeclaringType.Assembly ||
>>+						TypeManager.IsFriendAssembly (mb.DeclaringType.Assembly)) {
>> 					if (ma == MethodAttributes.Assembly || ma == MethodAttributes.FamORAssem)
>> 						return true;
>> 				} else {
>>@@ -2491,25 +2575,24 @@
>> 						return false;
>> 				}
>> 
>>-				if (ma == MethodAttributes.Family ||
>>-				    ma == MethodAttributes.FamANDAssem ||
>>-				    ma == MethodAttributes.FamORAssem)
>>-					return CheckValidFamilyAccess (mb.IsStatic, m);
>>-				
>>-				// Public.
>>-				return true;
>>+				// Family, FamORAssem or FamANDAssem
>>+				return CheckValidFamilyAccess (mb.IsStatic, m);
>> 			}
>> 			
>> 			if (m is FieldInfo){
>> 				FieldInfo fi = (FieldInfo) m;
>> 				FieldAttributes fa = fi.Attributes & FieldAttributes.FieldAccessMask;
>> 				
>>+				if (fa == FieldAttributes.Public)
>>+					return true;
>>+				
>> 				if (fa == FieldAttributes.Private)
>> 					return private_ok ||
>> 						IsPrivateAccessible (invocation_type, m.DeclaringType) ||
>> 						IsNestedChildOf (invocation_type, m.DeclaringType);
>> 
>>-				if (invocation_assembly == fi.DeclaringType.Assembly) {
>>+				if (invocation_assembly == fi.DeclaringType.Assembly ||
>>+						TypeManager.IsFriendAssembly (fi.DeclaringType.Assembly)) {
>> 					if (fa == FieldAttributes.Assembly || fa == FieldAttributes.FamORAssem)
>> 						return true;
>> 				} else {
>>@@ -2517,13 +2600,8 @@
>> 						return false;
>> 				}
>> 
>>-				if (fa == FieldAttributes.Family ||
>>-				    fa == FieldAttributes.FamANDAssem ||
>>-				    fa == FieldAttributes.FamORAssem)
>>-					return CheckValidFamilyAccess (fi.IsStatic, m);
>>-				
>>-				// Public.
>>-				return true;
>>+				// Family, FamORAssem or FamANDAssem
>>+				return CheckValidFamilyAccess (fi.IsStatic, m);
>> 			}
>> 
>> 			//
>>@@ -2784,6 +2862,11 @@
>> 		}
>> 		return false;
>> 	}
>>+
>>+	static void AddFriendAssembly (Assembly assembly, bool is_friend)
>>+	{
>>+		assembly_internals_vis_attrs.Add (assembly, is_friend);
>>+	}
>> 		
>> #endregion
>> 	
>>Index: namespace.cs
>>===================================================================
>>--- namespace.cs	(revisión: 52314)
>>+++ namespace.cs	(copia de trabajo)
>>@@ -112,12 +112,17 @@
>> 			if (t.IsPointer)
>> 				throw new InternalErrorException ("Use GetPointerType() to get a pointer");
>> 			
>>+			
>> 			TypeAttributes ta = t.Attributes & TypeAttributes.VisibilityMask;
>>+			if (ta == TypeAttributes.NestedPrivate)
>>+				return null;
>>+			
>> 			if (ta == TypeAttributes.NotPublic ||
>>-			    ta == TypeAttributes.NestedPrivate ||
>>-			    ta == TypeAttributes.NestedAssembly ||
>>-			    ta == TypeAttributes.NestedFamANDAssem)
>>-				return null;
>>+					ta == TypeAttributes.NestedAssembly ||
>>+					/*ta == TypeAttributes.NestedFamORAssem ||*/
>>+					ta == TypeAttributes.NestedFamANDAssem)
>>+				if (!TypeManager.IsFriendAssembly (t.Assembly))
>>+					return null;
>> 
>> 			return t;
>> 		}
>>Index: ecore.cs
>>===================================================================
>>--- ecore.cs	(revisión: 52315)
>>+++ ecore.cs	(copia de trabajo)
>>@@ -156,6 +156,9 @@
>> 
>> 			must_do_cs1540_check = false; // by default we do not check for this
>> 
>>+			if (ma == MethodAttributes.Public)
>>+				return true;
>>+			
>> 			//
>> 			// If only accessible to the current class or children
>> 			//
>>@@ -163,7 +166,8 @@
>> 				return TypeManager.IsPrivateAccessible (invocation_type, mi.DeclaringType) ||
>> 					TypeManager.IsNestedChildOf (invocation_type, mi.DeclaringType);
>> 
>>-			if (mi.DeclaringType.Assembly == invocation_type.Assembly) {
>>+			if (mi.DeclaringType.Assembly == invocation_type.Assembly ||
>>+					TypeManager.IsFriendAssembly (mi.DeclaringType.Assembly)) {
>> 				if (ma == MethodAttributes.Assembly || ma == MethodAttributes.FamORAssem)
>> 					return true;
>> 			} else {
>>@@ -173,18 +177,12 @@
>> 
>> 			// Family and FamANDAssem require that we derive.
>> 			// FamORAssem requires that we derive if in different assemblies.
>>-			if (ma == MethodAttributes.Family ||
>>-			    ma == MethodAttributes.FamANDAssem ||
>>-			    ma == MethodAttributes.FamORAssem) {
>>-				if (!TypeManager.IsNestedFamilyAccessible (invocation_type, mi.DeclaringType))
>>-					return false;
>>+			if (!TypeManager.IsNestedFamilyAccessible (invocation_type, mi.DeclaringType))
>>+				return false;
>> 
>>-				if (!TypeManager.IsNestedChildOf (invocation_type, mi.DeclaringType))
>>-					must_do_cs1540_check = true;
>>+			if (!TypeManager.IsNestedChildOf (invocation_type, mi.DeclaringType))
>>+				must_do_cs1540_check = true;
>> 
>>-				return true;
>>-			}
>>-
>> 			return true;
>> 		}
>> 
>>Index: decl.cs
>>===================================================================
>>--- decl.cs	(revisión: 52314)
>>+++ decl.cs	(copia de trabajo)
>>@@ -925,7 +925,8 @@
>>   				//
>>   				// This test should probably use the declaringtype.
>>   				//
>>-				return check_type.Assembly == TypeBuilder.Assembly;
>>+				return check_type.Assembly == TypeBuilder.Assembly ||
>>+					TypeManager.IsFriendAssembly (check_type.Assembly);
>> 
>> 			case TypeAttributes.NestedPublic:
>> 				return true;
>>@@ -940,15 +941,18 @@
>> 				return FamilyAccessible (tb, check_type);
>> 
>> 			case TypeAttributes.NestedFamANDAssem:
>>-				return (check_type.Assembly == tb.Assembly) &&
>>+				return ((check_type.Assembly == tb.Assembly) || 
>>+						TypeManager.IsFriendAssembly (check_type.Assembly)) && 
>> 					FamilyAccessible (tb, check_type);
>> 
>> 			case TypeAttributes.NestedFamORAssem:
>> 				return (check_type.Assembly == tb.Assembly) ||
>>-					FamilyAccessible (tb, check_type);
>>+					FamilyAccessible (tb, check_type) ||
>>+					TypeManager.IsFriendAssembly (check_type.Assembly);
>> 
>> 			case TypeAttributes.NestedAssembly:
>>-				return check_type.Assembly == tb.Assembly;
>>+				return check_type.Assembly == tb.Assembly ||
>>+					TypeManager.IsFriendAssembly (check_type.Assembly);
>> 			}
>> 
>> 			Console.WriteLine ("HERE: " + check_attr);
>>    
>>
>>------------------------------------------------------------------------
>>
>>_______________________________________________
>>Mono-devel-list mailing list
>>Mono-devel-list at lists.ximian.com
>>http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>    
>>




More information about the Mono-devel-list mailing list