[Mono-dev] [Patch] Extern alias (with modifications)
Raja R Harinath
rharinath at novell.com
Thu Oct 6 03:10:43 EDT 2005
Hi,
Carlos Alberto Cortez <calberto.cortez at gmail.com> writes:
> I applied the corrections made by Hari and all my tests are working
> fine.
>
> Comments and suggestions are welcome.
Ok. I like this much much better :-) However, I have some nits.
Nothing earth-shattering, but will help keep the code maintenable.
> Index: typemanager.cs
> ===================================================================
> --- typemanager.cs (revision: 51315)
> +++ typemanager.cs (copia de trabajo)
[snip]
> @@ -578,60 +593,33 @@
> return (Type) ret;
> }
>
> - public static Type LookupTypeReflection (string name, Location loc)
> + public static Type LookupTypeInModules (string name)
Why keep this here? I think this can be moved to GlobalRootNamespace too.
> +
> + //
> + // We use this for retrieving GetNamespaces method if avalaible
> + //
> + static MethodInfo assembly_get_namespaces;
>
> /// <summary>
> /// Computes the namespaces that we import from the assemblies we reference.
> /// </summary>
> public static void ComputeNamespaces ()
> {
> - MethodInfo assembly_get_namespaces = typeof (Assembly).GetMethod ("GetNamespaces", BindingFlags.Instance|BindingFlags.NonPublic);
> + if (assembly_get_namespaces == null)
> + assembly_get_namespaces = typeof (Assembly).GetMethod ("GetNamespaces", BindingFlags.Instance|BindingFlags.NonPublic);
>
> + foreach (Assembly assembly in assemblies)
> + Namespace.Root.AddAssemblyReference (assembly);
> +
> Hashtable cache = null;
>
> //
> @@ -682,8 +670,43 @@
> cache.Add (ns, null);
> }
> }
> +
> }
>
> + public static Namespace ComputeNamespacesForAlias (string name)
> + {
> + Assembly assembly = (Assembly) external_aliases [name];
> + if (assembly == null)
> + return null;
> +
> + if (assembly_get_namespaces == null)
> + assembly_get_namespaces = typeof (Assembly).GetMethod ("GetNamespaces", BindingFlags.Instance|BindingFlags.NonPublic);
> +
> + Namespace retval = Namespace.DefineRootNamespace (name, assembly);
> + if (assembly_get_namespaces != null) {
> + string [] namespaces = (string []) assembly_get_namespaces.Invoke (assembly, null);
> + foreach (string ns in namespaces) {
> + if (ns.Length == 0)
> + continue;
> +
> + retval.GetNamespace (ns, true);
> + }
> + } else {
> + Hashtable alias_cache = new Hashtable ();
> + alias_cache.Add ("", null);
> + foreach (Type t in assembly.GetExportedTypes ()) {
> + string ns = t.Namespace;
> + if (ns == null || alias_cache.Contains (ns))
> + continue;
> +
> + retval.GetNamespace (ns, true);
> + alias_cache.Add (ns, null);
> + }
> + }
> +
> + return retval;
> + }
Don't duplicate code, re-factor. You can move this to RootNamespace.
You can pretty much invoke it in it's constructor. Similarly, you can
invoke it incrementally when each assembly is added to
GlobalRootNamespace.
> Index: namespace.cs
> ===================================================================
> --- namespace.cs (revision: 51315)
> +++ namespace.cs (copia de trabajo)
> @@ -9,9 +9,101 @@
> using System;
> using System.Collections;
> using System.Collections.Specialized;
> +using System.Reflection;
>
> namespace Mono.CSharp {
>
> + public class RootNamespace : Namespace
> + {
> + Assembly referenced_assembly;
> +
> + public RootNamespace (Assembly assembly) : base (null, String.Empty)
> + {
> + this.referenced_assembly = assembly;
> + this.root = this;
> + }
> +
> + public virtual Type LookupTypeReflection (string name, Location loc)
> + {
> + Console.WriteLine ("Looking for type = {0} in
> + assembly = {1}", name, referenced_assembly.FullName);
Remove debugging code.
> + Type t = referenced_assembly.GetType (name);
> + if (t == null)
> + return null;
> +
> + if (t.IsPointer)
> + throw new InternalErrorException ("Use GetPointerType() to get a pointer");
> +
> + TypeAttributes ta = t.Attributes & TypeAttributes.VisibilityMask;
> + if (ta != TypeAttributes.NotPublic && ta != TypeAttributes.NestedPrivate &&
> + ta != TypeAttributes.NestedAssembly && ta != TypeAttributes.NestedFamANDAssem)
> + return t;
> +
> + return null;
> + }
> +
> + }
Again, don't duplicate code, re-factor.
> + public class GlobalRootNamespace : RootNamespace
> + {
> + Assembly [] assemblies;
> +
> + public GlobalRootNamespace () : base (null)
> + {
> + assemblies = new Assembly [0];
> + }
> +
> + public void AddAssemblyReference (Assembly assembly)
> + {
> + Assembly [] tmp = new Assembly [assemblies.Length + 1];
> + Array.Copy (assemblies, 0, tmp, 0, assemblies.Length);
> + tmp [assemblies.Length] = assembly;
> +
> + assemblies = tmp;
> + }
As I said, we can compute the namespaces here.
> /// <summary>
> /// Keeps track of the namespaces defined in the C# code.
> ///
> @@ -21,6 +113,7 @@
> public class Namespace : FullNamedExpression {
> static ArrayList all_namespaces;
> static Hashtable namespaces_map;
> + static Hashtable root_namespaces;
>
> Namespace parent;
> string fullname;
> @@ -28,10 +121,11 @@
> Hashtable namespaces;
> IDictionary declspaces;
> Hashtable cached_types;
> + protected RootNamespace root;
>
> public readonly MemberName MemberName;
>
> - public static Namespace Root;
> + public static GlobalRootNamespace Root;
I would prefer this to be renamed Global.
> static Namespace ()
> {
> @@ -42,8 +136,9 @@
> {
> all_namespaces = new ArrayList ();
> namespaces_map = new Hashtable ();
> + root_namespaces = new Hashtable ();
>
> - Root = new Namespace (null, "");
> + Root = new GlobalRootNamespace ();
> }
>
> /// <summary>
> @@ -60,6 +155,12 @@
>
> this.parent = parent;
>
> + if (parent != null)
> + if (parent is RootNamespace)
> + this.root = parent as RootNamespace;
> + else
> + this.root = parent.root;
> +
Somewhat unclean. I think it suffices to say
if (parent != null)
this.root = parent.root;
Afterall, if parent is RootNamespace, parent.root == parent.
> string pname = parent != null ? parent.Name : "";
>
> if (pname == "")
> @@ -81,10 +182,12 @@
> namespaces = new Hashtable ();
> cached_types = new Hashtable ();
>
> - all_namespaces.Add (this);
> - if (namespaces_map.Contains (fullname))
> - return;
> - namespaces_map [fullname] = true;
> + if (this.root == Root) {
> + all_namespaces.Add (this);
> + if (namespaces_map.Contains (fullname))
> + return;
> + namespaces_map [fullname] = true;
> + }
> }
Hmm, somewhat unclean. Move the whole danged thing to
GlobalRootNamespace, along with IsNamespace and DefineNamespaces. Add a
void RootNamespace::RegisterNamespace (Namespace n)
that does nothing by default, but the above stuff in GlobalRootNamespace.
> public override Expression DoResolve (EmitContext ec)
> @@ -138,6 +241,16 @@
> return Root.GetNamespace (name, create);
> }
>
> + public static RootNamespace DefineRootNamespace (string name, Assembly assembly)
> + {
> + RootNamespace retval = (RootNamespace) root_namespaces [name];
> + if (retval != null)
> + return retval;
> +
> + retval = new RootNamespace (assembly);
> + return retval;
> + }
This can be moved to RootNamespace.
> TypeExpr LookupType (string name, Location loc)
> {
> if (cached_types.Contains (name))
> @@ -161,7 +274,7 @@
> }
> }
> string lookup = t != null ? t.FullName : (fullname == "" ? name : fullname + "." + name);
> - Type rt = TypeManager.LookupTypeReflection (lookup, loc);
> + Type rt = root.LookupTypeReflection (lookup, loc);
> if (t == null)
> t = rt;
Neat :-)
> @@ -256,6 +369,7 @@
> Hashtable aliases;
> ArrayList using_clauses;
> public bool DeclarationFound = false;
> + public bool UsingFound;
Hmm. I would prefer handling this in the parser. You're just assuming
that the aliases are resolved in the order that they were added -- not
false, but not guaranteed either. Nothing else depends on that order, IIRC.
> //
> // This class holds the location where a using definition is
> @@ -304,24 +418,34 @@
> }
> }
>
> - public class AliasEntry {
> + public abstract class AliasEntry {
> public readonly string Name;
> public readonly Expression Alias;
> public readonly NamespaceEntry NamespaceEntry;
> public readonly Location Location;
>
> - public AliasEntry (NamespaceEntry entry, string name, MemberName alias, Location loc)
> + protected AliasEntry (NamespaceEntry entry, string name, MemberName alias, Location loc)
> {
> Name = name;
> - Alias = alias.GetTypeExpression ();
> + Alias = alias != null ? alias.GetTypeExpression () : null;
> NamespaceEntry = entry;
> Location = loc;
> }
I'd prefer to move 'Alias' to LocalAliasEntry, and update the caller, if
it isn't too unclean.
> @@ -641,7 +811,12 @@
> foreach (DictionaryEntry de in aliases) {
> AliasEntry alias = (AliasEntry) de.Value;
> if (alias.Resolve () == null)
> - Error_NamespaceNotFound (alias.Location, alias.Alias.ToString ());
> + if (alias is LocalAliasEntry)
> + Error_NamespaceNotFound (alias.Location, alias.Alias.ToString ());
> + else if (alias is ExternAliasEntry) {
> + Report.Error (430, alias.Location, "The extern alias '" + alias.Name +
> + "' was not specified in a /reference option");
> + }
Hmm, unclean. I think you can move the error reporting inside Resolve ().
At the very least, make it a virtual.
- Hari
More information about the Mono-devel-list
mailing list