boo-lang / boo

The Boo Programming Language.
BSD 3-Clause "New" or "Revised" License
874 stars 148 forks source link

tweak in assembly equality comparer #101

Closed lafar6502 closed 9 years ago

lafar6502 commented 9 years ago

Sometimes I got strange 'An item with the same key has already been added' from the MemoizedFunction, but this shouldn't be possible by looking at the Invoke code. I suspected this happens for dynamically generated assemblies (because I have seen this many times when compiling some boo-based dsl scripts) and by heuristics concluded that there might be a problem with equality comparer used with memoized function. Sorry, no definite test case as it was a 'heisenbug' almost impossible to reproduce reliably. Modified the assemblyequalitycomparer class, the change should not break anything - just replaced the hashcode generating method.

lafar6502 commented 9 years ago

Stack trace example:

BCE0055: Boo.Lang.Compiler.CompilerError: Internal compiler error: An item with the same key has already been added.. ---> System.ArgumentException: An item with the same key has already been added.
   at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
   at Boo.Lang.Compiler.Util.MemoizedFunction`2.Invoke(TArg arg)
   at Boo.Lang.Compiler.TypeSystem.Reflection.AssemblyReference.Map(Type type)
   at Boo.Lang.Compiler.TypeSystem.Reflection.ReflectionTypeSystemProvider.Map(Type type)
   at Boo.Lang.Compiler.TypeSystem.Reflection.ReflectionNamespace.AddModule(Type type)
   at Boo.Lang.Compiler.TypeSystem.Reflection.ReflectionNamespace.Add(Type type)
   at Boo.Lang.Compiler.TypeSystem.Reflection.ReflectionNamespaceBuilder.CatalogPublicTypes(IEnumerable`1 types)
   at Boo.Lang.Compiler.TypeSystem.Reflection.ReflectionNamespaceBuilder.Build()
   at Boo.Lang.Compiler.TypeSystem.Reflection.AssemblyReference.CreateRootNamespace()
   at Boo.Lang.Compiler.TypeSystem.Reflection.AssemblyReference.get_RootNamespace()
   at Boo.Lang.Compiler.TypeSystem.Core.GlobalNamespace.<RootNamespaces>d__0.MoveNext()
   at Boo.Lang.Compiler.TypeSystem.Core.Namespaces.ResolveCoalescingNamespaces(INamespace parent, IEnumerable`1 namespacesToResolveAgainst, String name, EntityType typesToConsider, ICollection`1 resultingSet)
   at Boo.Lang.Compiler.TypeSystem.Core.GlobalNamespace.Resolve(ICollection`1 resultingSet, String name, EntityType typesToConsider)
   at Boo.Lang.Compiler.TypeSystem.Core.Namespaces.ResolveCoalescingNamespaces(INamespace parent, INamespace namespaceToResolveAgainst, String name, EntityType typesToConsider, ICollection`1 resultingSet)
   at Boo.Lang.Compiler.TypeSystem.Services.NameResolutionService.Resolve(ICollection`1 targetList, String name, EntityType flags)
   at Boo.Lang.Compiler.TypeSystem.Services.NameResolutionService.ResolveImpl(String name, EntityType flags)
   at Boo.Lang.Compiler.Util.MemoizedFunction`3.Invoke(T1 arg1, T2 arg2)
   at Boo.Lang.Compiler.TypeSystem.Services.NameResolutionService.Resolve(String name, EntityType flags)
   at Boo.Lang.Compiler.TypeSystem.Services.NameResolutionService.ResolveQualifiedName(String name, EntityType flags)
   at Boo.Lang.Compiler.TypeSystem.Services.NameResolutionService.ResolveQualifiedName(String name)
   at Boo.Lang.Compiler.Steps.MacroProcessing.MacroExpander.ResolvePreferringInternalMacros(String macroTypeName)
   at Boo.Lang.Compiler.Steps.MacroProcessing.MacroExpander.ResolveMacroName(MacroStatement node)
   at Boo.Lang.Compiler.Steps.MacroProcessing.MacroExpander.OnMacroStatement(MacroStatement node)
   at Boo.Lang.Compiler.Ast.MacroStatement.Accept(IAstVisitor visitor)
   at Boo.Lang.Compiler.Ast.DepthFirstTransformer.OnNode(Node node)
   at Boo.Lang.Compiler.Ast.DepthFirstTransformer.VisitNode(Node node)
   --- End of inner exception stack trace ---
   at Boo.Lang.Compiler.Ast.DepthFirstTransformer.OnError(Node node, Exception error)
   at Boo.Lang.Compiler.Ast.DepthFirstTransformer.VisitNode(Node node)
   at Boo.Lang.Compiler.Ast.DepthFirstTransformer.Visit[T](NodeCollection`1 collection)
   at Boo.Lang.Compiler.Ast.DepthFirstTransformer.OnBlock(Block node)
   at Boo.Lang.Compiler.Ast.Block.Accept(IAstVisitor visitor)
   at Boo.Lang.Compiler.Ast.DepthFirstTransformer.OnNode(Node node)
   at Boo.Lang.Compiler.Ast.DepthFirstTransformer.VisitNode(Node node)
   at Boo.Lang.Compiler.Ast.DepthFirstTransformer.OnMethod(Method node)
   at Boo.Lang.Compiler.Ast.Method.Accept(IAstVisitor visitor)
drslump commented 9 years ago

@masonwheeler Does this look right to you? Shall we merge it?

masonwheeler commented 9 years ago

Looks good to me. If we want to hash assemblies, it's better to hash the assembly objects themselves than their names, especially if the current system is causing collisions.

drslump commented 9 years ago

thanks @lafar6502 and @masonwheeler!