Hi,
If I look at it form an API design point of view netiher LoadMscorlib
nor SetMscorlib are good.
LoadMscorlib was designed to meet your very specific needs of being able
to load mscorlib.dll from a specified file.
SetMscorlib would remove the restriction of loading from the file and
also would be future proof in the sense that it will continue to be
compatible with new assembly loading/creating methods and new overloads
of existing methods without any modification required.
As I see the only was mscorlib.dll could be compiled is that you don't
load another mscorlib.dll in DefineDynamicAssembly or in AssemblyBuilder
constructor. This would be the defined behavior. Right not I cannot
think of any possible requirement for loading mscorlib.dll during
DefineDynamicAssembly. Since mscorlib.dll is a corner case if you will
have to load mscolib.dll anyway intorducing another solution would only
break a marginal set of all the use cases (mainly mcs when building
mscorlib.dll) so such a breaking change would be acceptable. And I think
that if such a requirement would be introduced that would be a breaking
change in MS.NET as well.
As a conclusion I think that there are only two properly designed solutions:
1. Remove LoadMscorlib, make HasMscorlib internal, and rely on
AssemblyResolve event handler.
2. Remove LoadMscorlib, remove HasMscorlib, and add two new methods:
__AddAssembly(string refname, Assembly asm)
__TryGetAssembly(string refname, out Assembly asm)
The first solution is very straightforward, and I've attached a patch
for the second one (it's very straightforwad as well but was easier to
express myself by using a patch).
I consider the second one to be a more proper solution because compiler
writers usually prefer to load a set of assemblier themselves rather
than rely on automatic assembly resolution. Since Universe design in
based on mapping System.Types to IKVM.Reflection.Types, compiler writers
might actually use typeof for other assemblies (not just mscorlib.dll)
and they might want those assembiles to be resolved to their preferred
assembly. An example for such a behavior is a VB.NET compiler that has
its own Microsoft.VisualBasic.dll runtime library treated very similar
to how mscorlib.dll is treated by a C# compiler. VB runtime is written
in VB and a /novbruntimeref option is required to build the runtime.
Regards,
Kornél
Jeroen Frijters wrote:
Please see the attached patch.
I belive that this is actually an easier and cleaner solution than the
current one.
I disagree. You have to look at it from an API development point of view. This
creates a huge window for undefined behavior and dependencies on implementation
specifics, between calling DefineDynamicAssembly and SetMscorlib. It also makes
it impossible for DefineDynamicAssembly (or the AssemblyBuilder constructor) to
do anything that requires mscorlib (and who knows, maybe some future change to
reflection will require this).
Regards,
Jeroen
Index: Universe.cs
===================================================================
RCS file: /cvsroot/ikvm/ikvm/reflect/Universe.cs,v
retrieving revision 1.13
diff -u -r1.13 Universe.cs
--- Universe.cs 7 May 2010 16:35:47 -0000 1.13
+++ Universe.cs 9 May 2010 10:26:13 -0000
@@ -387,33 +387,22 @@
get { return
typeof_System_Security_Permissions_SecurityAction ??
(typeof_System_Security_Permissions_SecurityAction =
Import(typeof(System.Security.Permissions.SecurityAction))); }
}
- public bool HasMscorlib
- {
- get { return importedTypes.Count != 0 ||
assembliesByName.Count != 0; }
- }
-
public event ResolveEventHandler AssemblyResolve
{
add { resolvers.Add(value); }
remove { resolvers.Remove(value); }
+ }
+
+ public void __AddAssembly(string refname, Assembly asm)
+ {
+ assembliesByName.Add(refname, asm);
}
- public void LoadMscorlib(string path)
- {
- if (HasMscorlib)
- {
- throw new InvalidOperationException();
- }
- Assembly asm = LoadFile(path);
- if (asm.FullName != typeof(object).Assembly.FullName)
- {
- // make the current mscorlib full name an alias
for the specified mscorlib,
- // this ensures that all the runtime built in
type are resolved against the
- // specified mscorlib.
-
assembliesByName.Add(typeof(object).Assembly.FullName, asm);
- }
- }
-
+ public bool __TryGetAssembly(string refname, out Assembly asm)
+ {
+ return assembliesByName.TryGetValue(refname, out asm);
+ }
+
public Type Import(System.Type type)
{
Type imported;
@@ -680,14 +669,9 @@
private AssemblyBuilder DefineDynamicAssemblyImpl(AssemblyName
name, AssemblyBuilderAccess access, string dir, PermissionSet
requiredPermissions, PermissionSet optionalPermissions, PermissionSet
refusedPermissions)
{
- bool mscorlib = !HasMscorlib && name.Name == "mscorlib";
AssemblyBuilder asm = new AssemblyBuilder(this, name,
dir, requiredPermissions, optionalPermissions, refusedPermissions);
assembliesByName.Add(GetAssemblyIdentityName(asm.GetName()), asm);
assemblies.Add(asm);
- if (mscorlib)
- {
-
assembliesByName[typeof(object).Assembly.FullName] = asm;
- }
return asm;
}
Index: Emit/AssemblyBuilder.cs
===================================================================
RCS file: /cvsroot/ikvm/ikvm/reflect/Emit/AssemblyBuilder.cs,v
retrieving revision 1.18
diff -u -r1.18 AssemblyBuilder.cs
--- Emit/AssemblyBuilder.cs 7 May 2010 16:38:13 -0000 1.18
+++ Emit/AssemblyBuilder.cs 9 May 2010 10:26:13 -0000
@@ -102,10 +102,11 @@
this.dir = dir ?? ".";
this.requiredPermissions = requiredPermissions;
this.optionalPermissions = optionalPermissions;
- this.refusedPermissions = refusedPermissions;
- if (universe.HasMscorlib && universe.System_Object !=
null)
- {
- this.imageRuntimeVersion =
universe.System_Object.Assembly.ImageRuntimeVersion;
+ this.refusedPermissions = refusedPermissions;
+ Assembly asm;
+ if
(universe.__TryGetAssembly(typeof(object).Assembly.FullName, out asm))
+ {
+ this.imageRuntimeVersion =
asm.ImageRuntimeVersion;
}
else
{
_______________________________________________
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
