[Mono-dev] [PATCH] Friend access for class members
Carlos Alberto Cortez
calberto.cortez at gmail.com
Fri Oct 28 08:00:15 EDT 2005
Hey Marek,
Comments below:
El vie, 28-10-2005 a las 14:51 +0100, Marek Safar escribió:
> 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'
Well, every time we access to AssemblyName.FullName, a new string is
created. I know keeping this temporary string could be a little ugly, so
I created this method.
> 2. Please don't end error message with '.'
> 3. Please use "bla `{0}' bla" syntax, it is easier to read.
Ok, fixed the styles of the error messages.
Carlos.
>
> 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