2002-05-13 Miguel de Icaza <miguel@ximian.com>
authorMiguel de Icaza <miguel@gnome.org>
Mon, 13 May 2002 02:16:40 +0000 (02:16 -0000)
committerMiguel de Icaza <miguel@gnome.org>
Mon, 13 May 2002 02:16:40 +0000 (02:16 -0000)
* class.cs: (MethodSignature.InheritableMemberSignatureCompare):
We have to compare the assembly property here when dealing with
FamANDAssem and Assembly access modifiers, because we might be
creating an assembly from *modules* (that means that we are not
getting TypeBuilders for types defined in other modules that are
part of this assembly).

(TypeContainer.DefineMembers): If both the defined member and the
parent name match are methods, then do not emit any warnings: let
the Method.Define routine take care of flagging warnings.  But if
there is a mismatch (method overrides something else, or method is
overriwritten by something, then emit warning).

(MethodSignature.MemberSignatureCompare): If the sig.ret_type is
set to null, this means `do not check for the return type on the
signature'.

(Method.Define): set the return type for the method signature to
null, so that we get methods with the same name and parameters and
different return types.  This is used to flag warning 114 (you are
hiding a method, and you probably want to use the new/override
keywords instead).

* typemanager.cs (MemberLookup): Implemented proper access
control, closing a long standing set of bug reports.  The problem
was that the Framework only has two bits: Public and NonPublic,
and NonPublic includes private and protected methods, but we need
to enforce the FamANDAssem, FamOrAssem and Family.

svn path=/trunk/mcs/; revision=4576

mcs/mcs/ChangeLog
mcs/mcs/TODO
mcs/mcs/class.cs
mcs/mcs/codegen.cs
mcs/mcs/typemanager.cs

index fe27904c1f68a617b764ee309ad555df70829057..5e34a628f7b5c9a34489f1b7eea4c15928c541a7 100755 (executable)
@@ -1,3 +1,34 @@
+2002-05-13  Miguel de Icaza  <miguel@ximian.com>
+
+       * class.cs: (MethodSignature.InheritableMemberSignatureCompare):
+       We have to compare the assembly property here when dealing with
+       FamANDAssem and Assembly access modifiers, because we might be
+       creating an assembly from *modules* (that means that we are not
+       getting TypeBuilders for types defined in other modules that are
+       part of this assembly).
+
+       (TypeContainer.DefineMembers): If both the defined member and the
+       parent name match are methods, then do not emit any warnings: let
+       the Method.Define routine take care of flagging warnings.  But if
+       there is a mismatch (method overrides something else, or method is
+       overriwritten by something, then emit warning).
+
+       (MethodSignature.MemberSignatureCompare): If the sig.ret_type is
+       set to null, this means `do not check for the return type on the
+       signature'. 
+
+       (Method.Define): set the return type for the method signature to
+       null, so that we get methods with the same name and parameters and
+       different return types.  This is used to flag warning 114 (you are
+       hiding a method, and you probably want to use the new/override
+       keywords instead).
+
+       * typemanager.cs (MemberLookup): Implemented proper access
+       control, closing a long standing set of bug reports.  The problem
+       was that the Framework only has two bits: Public and NonPublic,
+       and NonPublic includes private and protected methods, but we need
+       to enforce the FamANDAssem, FamOrAssem and Family. 
+
 2002-05-11  Miguel de Icaza  <miguel@ximian.com>
 
        * statement.cs (GotoCase): Return true: Ammounts to giving up
index 80cab84525bc988a7acc54735429064704e1956e..71d3f20b231d59cda8a41301e081180c7eef2cae 100644 (file)
@@ -27,12 +27,6 @@ BUGS
 * Check for Final when overriding, if the parent is Final, then we cant
   allow an override.
 
-* Currently the code path for 108/109 reporting is not being ran for methods
-  as we need to compare method signatures.  But since we retrieve the expensive
-  method arguments in the method, we probably should do 108/109 processing there.
-
-* Emit warning on hiding members without NEW not only in members.
-
 * Implement visibility.
 
 * Visibility
index 82c83a074f27a2cfe76ef440747827a65c80a2bb..3293a300dbb773c9dc14632700fb36fb9311e9b4 100755 (executable)
@@ -987,38 +987,29 @@ namespace Mono.CSharp {
                                                
                                if (defined_names == null)
                                        continue;
-                               
+
                                idx = Array.BinarySearch (defined_names, mc.Name, mif_compare);
-                               
                                if (idx < 0){
                                        if (RootContext.WarningLevel >= 4){
                                                if ((mc.ModFlags & Modifiers.NEW) != 0)
-                                                       Report109 (mc.Location, mc);
+                                                       Warning_KewywordNewNotRequired (mc.Location, mc);
                                        }
                                        continue;
                                }
 
-                               if (defined_names [idx] is PropertyInfo &&
-                                   ((mc.ModFlags & Modifiers.OVERRIDE) != 0)){
+                               MemberInfo match = defined_names [idx];
+
+                               if (match is PropertyInfo && ((mc.ModFlags & Modifiers.OVERRIDE) != 0))
                                        continue;
-                               }
-                                   
-#if WANT_TO_VERIFY_SIGNATURES_HERE
-                               if (defined_names [idx] is MethodBase && mc is MethodCore){
-                                       MethodBase mb = (MethodBase) defined_names [idx];
-                                       MethodCore met = (MethodCore) mc;
-                                       
-                                       if ((mb.IsVirtual || mb.IsAbstract) &&
-                                           (mc.ModFlags & Modifiers.OVERRIDE) != 0)
-                                               continue;
 
-                                       //
-                                       // FIXME: Compare the signatures here.  If they differ,
-                                       // then: `continue;' 
-                               }
-#endif
+                               //
+                               // If we are both methods, let the method resolution emit warnings
+                               //
+                               if (match is MethodBase && mc is MethodCore)
+                                       continue; 
+                               
                                if ((mc.ModFlags & Modifiers.NEW) == 0)
-                                       Report108 (mc.Location, defined_names [idx]);
+                                       Warning_KeywordNewRequired (mc.Location, defined_names [idx]);
                        }
                        
                        foreach (object o in remove_list)
@@ -1094,7 +1085,7 @@ namespace Mono.CSharp {
                                default_static_constructor.Define (this);
                        
                        if (methods != null)
-                               DefineMembers (methods, null);
+                               DefineMembers (methods, defined_names);
 
                        if (properties != null)
                                DefineMembers (properties, defined_names);
@@ -1195,7 +1186,6 @@ namespace Mono.CSharp {
                        ArrayList members = new ArrayList ();
                        bool priv = (bf & BindingFlags.NonPublic) != 0;
 
-                       priv = true;
                        if (filter == null)
                                filter = accepting_filter; 
                        
@@ -1744,7 +1734,7 @@ namespace Mono.CSharp {
                        return "`" + Name + "." + n + "'";
                }
 
-               public void Report108 (Location l, MemberInfo mi)
+               public void Warning_KeywordNewRequired (Location l, MemberInfo mi)
                {
                        Report.Warning (
                                108, l, "The keyword new is required on " + 
@@ -1752,7 +1742,7 @@ namespace Mono.CSharp {
                                mi.ReflectedType.Name + "." + mi.Name + "'");
                }
 
-               public void Report109 (Location l, MemberCore mc)
+               public void Warning_KewywordNewNotRequired (Location l, MemberCore mc)
                {
                        Report.Warning (
                                109, l, "The member " + MakeName (mc.Name) + " does not hide an " +
@@ -2208,7 +2198,7 @@ namespace Mono.CSharp {
 
                        // ptype is only null for System.Object while compiling corlib.
                        if (ptype != null){
-                               MethodSignature ms = new MethodSignature (Name, ret_type, parameters);
+                               MethodSignature ms = new MethodSignature (Name, null, parameters);
                                MemberInfo [] mi, mi_static, mi_instance;
 
                                mi_static = TypeContainer.FindMembers (
@@ -2228,7 +2218,7 @@ namespace Mono.CSharp {
                                        mi = mi_static;
                                else
                                        mi = null;
-                               
+
                                if (mi != null && mi.Length > 0){
                                        if (!CheckMethodAgainstBase (parent, flags, (MethodInfo) mi [0])){
                                                return false;
@@ -4129,8 +4119,13 @@ namespace Mono.CSharp {
                        
                        mi = (MethodInfo) m;
 
-                       if (mi.ReturnType != sig.RetType)
-                               return false;
+                       //
+                       // we use sig.RetType == null to mean `do not check the
+                       // method return value.  
+                       //
+                       if (sig.RetType != null)
+                               if (mi.ReturnType != sig.RetType)
+                                       return false;
 
                        Type [] args = TypeManager.GetArgumentTypes (mi);
                        Type [] sigp = sig.Parameters;
@@ -4148,7 +4143,13 @@ namespace Mono.CSharp {
 
                //
                // This filter should be used when we are requesting methods that
-               // we want to override.  
+               // we want to override.
+               //
+               // This makes a number of assumptions, for example
+               // that the methods being extracted are of a parent
+               // class (this means we know implicitly that we are
+               // being called to find out about members by a derived
+               // class).
                // 
                static bool InheritableMemberSignatureCompare (MemberInfo m, object filter_criteria)
                {
@@ -4163,7 +4164,7 @@ namespace Mono.CSharp {
                                // If only accessible to the defining assembly or 
                                if (prot == MethodAttributes.FamANDAssem ||
                                    prot == MethodAttributes.Assembly){
-                                       if (m is MethodBuilder)
+                                       if (m.DeclaringType.Assembly == CodeGen.AssemblyBuilder)
                                                return true;
                                        else
                                                return false;
index 6c1680068317b3bc153bbbc4f867fd4e272b0cda..983765501bf7e25413a45ac643391ce6c65dd9a9 100755 (executable)
@@ -20,8 +20,8 @@ namespace Mono.CSharp {
        /// </summary>
        public class CodeGen {
                static AppDomain current_domain;
-               static AssemblyBuilder assembly_builder;
-               static ModuleBuilder   module_builder;
+               public static AssemblyBuilder AssemblyBuilder;
+               public static ModuleBuilder   ModuleBuilder;
 
                static public ISymbolWriter SymbolWriter;
 
@@ -81,7 +81,7 @@ namespace Mono.CSharp {
                //
                static void InitializeSymbolWriter (string basename)
                {
-                       SymbolWriter = module_builder.GetSymWriter ();
+                       SymbolWriter = ModuleBuilder.GetSymWriter ();
 
                        //
                        // If we got an ISymbolWriter instance, initialize it.
@@ -124,7 +124,7 @@ namespace Mono.CSharp {
                        an = new AssemblyName ();
                        an.Name = TrimExt (Basename (name));
                        current_domain = AppDomain.CurrentDomain;
-                       assembly_builder = current_domain.DefineDynamicAssembly (
+                       AssemblyBuilder = current_domain.DefineDynamicAssembly (
                                an, AssemblyBuilderAccess.RunAndSave, Dirname (name));
 
                        //
@@ -135,29 +135,17 @@ namespace Mono.CSharp {
                        // If the third argument is true, the ModuleBuilder will dynamically
                        // load the default symbol writer.
                        //
-                       module_builder = assembly_builder.DefineDynamicModule (
+                       ModuleBuilder = AssemblyBuilder.DefineDynamicModule (
                                Basename (name), Basename (output), want_debugging_support);
 
                        if (want_debugging_support)
                                InitializeSymbolWriter (an.Name);
                }
 
-               static public AssemblyBuilder AssemblyBuilder {
-                       get {
-                               return assembly_builder;
-                       }
-               }
-               
-               static public ModuleBuilder ModuleBuilder {
-                       get {
-                               return module_builder;
-                       }
-               }
-
                static public void Save (string name)
                {
                        try {
-                               assembly_builder.Save (Basename (name));
+                               AssemblyBuilder.Save (Basename (name));
                        } catch (System.IO.IOException io){
                                Report.Error (16, "Coult not write to file `"+name+"', cause: " + io.Message);
                        }
index f389a5fb376e0b1097b6b8a0e67c19871c7ea524..fea0840a4d9b3ffa1534a4b8fe383a524ab483bb 100755 (executable)
@@ -1203,6 +1203,95 @@ public class TypeManager {
                return target_list;
        }
 
+#region MemberLookup implementation
+       
+       //
+       // Name of the member
+       //
+       static string   closure_name;
+
+       //
+       // Whether we allow private members in the result (since FindMembers
+       // uses NonPublic for both protected and private), we need to distinguish.
+       //
+       static bool     closure_private_ok;
+
+       //
+       // Who is invoking us and which type is being queried currently.
+       //
+       static Type     closure_invocation_type;
+       static Type     closure_queried_type;
+
+       //
+       // The assembly that defines the type is that is calling us
+       //
+       static Assembly closure_invocation_assembly;
+
+       //
+       // This filter filters by name + whether it is ok to include private
+       // members in the search
+       //
+       static internal bool FilterWithClosure (MemberInfo m, object filter_criteria)
+       {
+               //
+               // Hack: we know that the filter criteria will always be in the `closure'
+               // fields. 
+               //
+
+               if (m.Name != closure_name)
+                       return false;
+
+               //
+               // Ugly: we need to find out the type of `m', and depending
+               // on this, tell whether we accept or not
+               //
+               if (m is MethodBase){
+                       MethodBase mb = (MethodBase) m;
+                       MethodAttributes ma = mb.Attributes & MethodAttributes.MemberAccessMask;
+
+                       if (ma == MethodAttributes.Private)
+                               return closure_private_ok;
+
+                       //
+                       // FamAndAssem requires that we not only derivate, but we are on the
+                       // same assembly.  
+                       //
+                       if (ma == MethodAttributes.FamANDAssem){
+                               if (closure_invocation_assembly != mb.DeclaringType.Assembly)
+                                       return false;
+                       }
+
+                       // FamORAssem, Family and Public:
+                       return true;
+               }
+
+               if (m is FieldInfo){
+                       FieldInfo fi = (FieldInfo) m;
+                       FieldAttributes fa = fi.Attributes & FieldAttributes.FieldAccessMask;
+
+                       if (fa == FieldAttributes.Private)
+                               return closure_private_ok;
+
+                       //
+                       // FamAndAssem requires that we not only derivate, but we are on the
+                       // same assembly.  
+                       //
+                       if (fa == FieldAttributes.FamANDAssem){
+                               if (closure_invocation_assembly != fi.DeclaringType.Assembly)
+                                       return false;
+                       }
+                       // FamORAssem, Family and Public:
+                       return true;
+               }
+
+               //
+               // EventInfos and PropertyInfos, return true
+               //
+               return true;
+       }
+
+       static MemberFilter FilterWithClosure_delegate = new MemberFilter (FilterWithClosure);
+       
        //
        // Looks up a member called `name' in the `queried_type'.  This lookup
        // is done by code that is contained in the definition for `invocation_type'.
@@ -1213,40 +1302,56 @@ public class TypeManager {
        // that might return multiple matches.
        //
        public static MemberInfo [] MemberLookup (Type invocation_type, Type queried_type, 
-                                                 MemberTypes mt, BindingFlags bf, string name)
+                                                 MemberTypes mt, BindingFlags original_bf, string name)
        {
-               if (invocation_type != null){
-                       if (invocation_type == queried_type || invocation_type.IsSubclassOf (queried_type))
-                               bf |= BindingFlags.NonPublic;
-               }
-
+               BindingFlags bf = original_bf;
+               
                ArrayList method_list = null;
                Type current_type = queried_type;
-               bool searching = (bf & BindingFlags.DeclaredOnly) == 0;
+               bool searching = (original_bf & BindingFlags.DeclaredOnly) == 0;
+               bool private_ok;
+               
+               closure_name = name;
+               closure_invocation_type = invocation_type;
+               closure_invocation_assembly = invocation_type != null ? invocation_type.Assembly : null;
+               
                do {
                        MemberInfo [] mi;
+
+                       //
+                       // `NonPublic' is lame, because it includes both protected and
+                       // private methods, so we need to control this behavior by
+                       // explicitly tracking if a private method is ok or not.
+                       //
+                       // The possible cases are:
+                       //    public, private and protected (internal does not come into the
+                       //    equation)
+                       //
+                       if (invocation_type != null){
+                               if (invocation_type == current_type)
+                                       private_ok = true;
+                               else
+                                       private_ok = false;
+                               
+                               if (private_ok || invocation_type.IsSubclassOf (current_type))
+                                       bf = original_bf | BindingFlags.NonPublic;
+                       } else {
+                               private_ok = false;
+                               bf = original_bf & ~BindingFlags.NonPublic;
+                       }
+
+                       closure_private_ok = private_ok;
+                       closure_queried_type = current_type;
                        
                        mi = TypeManager.FindMembers (
                                current_type, mt, bf | BindingFlags.DeclaredOnly,
-                               System.Type.FilterName, name);
+                               FilterWithClosure_delegate, name);
                        
                        if (current_type == TypeManager.object_type)
                                searching = false;
                        else {
                                current_type = current_type.BaseType;
                                
-                               //
-                               // Only do private searches on the current type,
-                               // never in our parents.
-                               //
-                               // FIXME: we should really be checking the actual
-                               // permissions on the members returned, rather than 
-                               // depend on NonPublic/Public in the BindingFlags,
-                               // as the BindingFlags is basically useless (does not
-                               // give us the granularity we need)
-                               //
-                               // bf &= ~BindingFlags.NonPublic;
-                               
                                //
                                // This happens with interfaces, they have a null
                                // basetype
@@ -1268,8 +1373,10 @@ public class TypeManager {
                        // searches, which means that our above FindMembers will
                        // return two copies of the same.
                        //
-                       if (count == 1 && !(mi [0] is MethodBase))
+                       if (count == 1 && !(mi [0] is MethodBase)){
                                return mi;
+                       }
+                       
                        if (count == 2 && (mi [0] is EventInfo))
                                return mi;
                        
@@ -1304,6 +1411,8 @@ public class TypeManager {
                                        
                return null;
        }
+#endregion
+       
 }
 
 }