From 585ae50c2f4128df518013ca52a83c32e91ffe28 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sun, 17 Jun 2012 17:15:32 +0200 Subject: [PATCH] CSharpAstResolver: don't return the same ResolveResult for two different nodes. (make clones when using caches) Closes icsharpcode/NRefactory#46. --- .../Resolver/CSharpResolver.cs | 20 ++++++++++--------- .../Resolver/ResolveVisitor.cs | 19 ++++++++++++++---- .../ResolverTest.cs | 4 ++++ .../CSharp/Resolver/LinqTests.cs | 6 +++--- .../Semantics/MemberResolveResult.cs | 10 ++++++++++ .../Semantics/ResolveResult.cs | 5 +++++ .../Implementation/DefaultMemberReference.cs | 4 +++- 7 files changed, 51 insertions(+), 17 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Resolver/CSharpResolver.cs b/ICSharpCode.NRefactory.CSharp/Resolver/CSharpResolver.cs index 168bebca6b..ec47908926 100644 --- a/ICSharpCode.NRefactory.CSharp/Resolver/CSharpResolver.cs +++ b/ICSharpCode.NRefactory.CSharp/Resolver/CSharpResolver.cs @@ -41,8 +41,6 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver public class CSharpResolver { static readonly ResolveResult ErrorResult = ErrorResolveResult.UnknownError; - static readonly ResolveResult DynamicResult = new ResolveResult(SpecialType.Dynamic); - static readonly ResolveResult NullResult = new ResolveResult(SpecialType.NullType); readonly ICompilation compilation; internal readonly CSharpConversions conversions; @@ -1381,7 +1379,9 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver foundInCache = cache.TryGetValue(identifier, out r); } } - if (!foundInCache) { + if (foundInCache) { + r = (r != null ? r.ShallowClone() : null); + } else { r = LookInCurrentType(identifier, typeArguments, lookupMode, parameterizeResultType); if (cache != null) { // also cache missing members (r==null) @@ -1398,9 +1398,11 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver r = LookInUsingScopeNamespace(null, compilation.RootNamespace, identifier, typeArguments, parameterizeResultType); } else { if (k == 0 && lookupMode != NameLookupMode.TypeInUsingDeclaration) { - if (!context.CurrentUsingScope.ResolveCache.TryGetValue(identifier, out r)) { + if (context.CurrentUsingScope.ResolveCache.TryGetValue(identifier, out r)) { + r = (r != null ? r.ShallowClone() : null); + } else { r = LookInCurrentUsingScope(identifier, typeArguments, false, false); - r = context.CurrentUsingScope.ResolveCache.GetOrAdd(identifier, r); + context.CurrentUsingScope.ResolveCache.TryAdd(identifier, r); } } else { r = LookInCurrentUsingScope(identifier, typeArguments, lookupMode == NameLookupMode.TypeInUsingDeclaration, parameterizeResultType); @@ -1477,7 +1479,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver if (!(isInUsingDeclaration && u == currentUsingScope)) { foreach (var pair in u.UsingAliases) { if (pair.Key == identifier) { - return pair.Value; + return pair.Value.ShallowClone(); } } } @@ -1589,7 +1591,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver } if (target.Type.Kind == TypeKind.Dynamic) - return DynamicResult; + return new ResolveResult(SpecialType.Dynamic); MemberLookup lookup = CreateMemberLookup(lookupMode); ResolveResult result; @@ -1887,7 +1889,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver // C# 4.0 spec: ยง7.6.5 if (target.Type.Kind == TypeKind.Dynamic) - return DynamicResult; + return new ResolveResult(SpecialType.Dynamic); MethodGroupResolveResult mgrr = target as MethodGroupResolveResult; if (mgrr != null) { @@ -2297,7 +2299,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver public ResolveResult ResolvePrimitive(object value) { if (value == null) { - return NullResult; + return new ResolveResult(SpecialType.NullType); } else { TypeCode typeCode = Type.GetTypeCode(value.GetType()); IType type = compilation.FindType(typeCode); diff --git a/ICSharpCode.NRefactory.CSharp/Resolver/ResolveVisitor.cs b/ICSharpCode.NRefactory.CSharp/Resolver/ResolveVisitor.cs index 884560b128..84e8f74526 100644 --- a/ICSharpCode.NRefactory.CSharp/Resolver/ResolveVisitor.cs +++ b/ICSharpCode.NRefactory.CSharp/Resolver/ResolveVisitor.cs @@ -59,7 +59,6 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver // The ResolveVisitor is also responsible for handling lambda expressions. static readonly ResolveResult errorResult = ErrorResolveResult.UnknownError; - readonly ResolveResult voidResult; CSharpResolver resolver; /// Resolve result of the current LINQ query. @@ -112,13 +111,18 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver this.resolver = resolver; this.parsedFile = parsedFile; this.navigator = skipAllNavigator; - this.voidResult = new ResolveResult(resolver.Compilation.FindType(KnownTypeCode.Void)); } internal void SetNavigator(IResolveVisitorNavigator navigator) { this.navigator = navigator ?? skipAllNavigator; } + + ResolveResult voidResult { + get { + return new ResolveResult(resolver.Compilation.FindType(KnownTypeCode.Void)); + } + } #endregion #region ResetContext @@ -1594,7 +1598,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver ResolveResult IAstVisitor.VisitTypeReferenceExpression(TypeReferenceExpression typeReferenceExpression) { if (resolverEnabled) { - return Resolve(typeReferenceExpression.Type); + return Resolve(typeReferenceExpression.Type).ShallowClone(); } else { Scan(typeReferenceExpression.Type); return null; @@ -2420,6 +2424,13 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver internal abstract AstNode BodyExpression { get; } internal abstract void EnforceMerge(ResolveVisitor parentVisitor); + + public override ResolveResult ShallowClone() + { + if (IsUndecided) + throw new NotSupportedException(); + return base.ShallowClone(); + } } void MergeUndecidedLambdas() @@ -3261,7 +3272,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver foreach (var clause in queryExpression.Clauses) { currentQueryResult = Resolve(clause); } - return currentQueryResult; + return WrapResult(currentQueryResult); } finally { currentQueryResult = oldQueryResult; cancellationToken = oldCancellationToken; diff --git a/ICSharpCode.NRefactory.ConsistencyCheck/ResolverTest.cs b/ICSharpCode.NRefactory.ConsistencyCheck/ResolverTest.cs index 87c0ff5311..a5b191edbb 100644 --- a/ICSharpCode.NRefactory.ConsistencyCheck/ResolverTest.cs +++ b/ICSharpCode.NRefactory.ConsistencyCheck/ResolverTest.cs @@ -55,6 +55,7 @@ namespace ICSharpCode.NRefactory.ConsistencyCheck } Dictionary resolvedNodes = new Dictionary(); + HashSet resolveResults = new HashSet(); HashSet nodesWithConversions = new HashSet(); public ResolveVisitorNavigationMode Scan(AstNode node) @@ -69,6 +70,9 @@ namespace ICSharpCode.NRefactory.ConsistencyCheck resolvedNodes.Add(node, result); if (CSharpAstResolver.IsUnresolvableNode(node)) throw new InvalidOperationException("Resolved unresolvable node"); + if (!ParenthesizedExpression.ActsAsParenthesizedExpression(node)) + if (!resolveResults.Add(result)) + throw new InvalidOperationException("Duplicate resolve result"); if (result.IsError && !allowErrors) { Console.WriteLine("Compiler error at " + fileName + ":" + node.StartLocation + ": " + result); diff --git a/ICSharpCode.NRefactory.Tests/CSharp/Resolver/LinqTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/Resolver/LinqTests.cs index 708d643e9f..379b803f43 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/Resolver/LinqTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/Resolver/LinqTests.cs @@ -103,8 +103,8 @@ class TestClass { } } "; - var rr = Resolve(program); - Assert.AreEqual("System.Linq.Enumerable.Select", rr.Member.FullName); + var rr = Resolve(program); + Assert.IsTrue(rr.Conversion.IsIdentityConversion); Assert.AreEqual("System.Collections.Generic.IEnumerable", rr.Type.FullName); Assert.AreEqual("System.Int32", ((ParameterizedType)rr.Type).TypeArguments[0].FullName); } @@ -114,7 +114,7 @@ class TestClass { { string program = @"using System; class TestClass { static void M() { - $(from a in new XYZ() select a.ToUpper())$.ToString(); + (from a in new XYZ() $select a.ToUpper()$).ToString(); }} class XYZ { public int Select(Func f) { return 42; } diff --git a/ICSharpCode.NRefactory/Semantics/MemberResolveResult.cs b/ICSharpCode.NRefactory/Semantics/MemberResolveResult.cs index c0991eef8d..e9998ad0ea 100644 --- a/ICSharpCode.NRefactory/Semantics/MemberResolveResult.cs +++ b/ICSharpCode.NRefactory/Semantics/MemberResolveResult.cs @@ -75,6 +75,16 @@ namespace ICSharpCode.NRefactory.Semantics this.constantValue = constantValue; } + public MemberResolveResult(ResolveResult targetResult, IMember member, IType returnType, bool isConstant, object constantValue, bool isVirtualCall) + : base(returnType) + { + this.targetResult = targetResult; + this.member = member; + this.isConstant = isConstant; + this.constantValue = constantValue; + this.isVirtualCall = isVirtualCall; + } + public ResolveResult TargetResult { get { return targetResult; } } diff --git a/ICSharpCode.NRefactory/Semantics/ResolveResult.cs b/ICSharpCode.NRefactory/Semantics/ResolveResult.cs index d73ecf6e41..fd8406efb8 100644 --- a/ICSharpCode.NRefactory/Semantics/ResolveResult.cs +++ b/ICSharpCode.NRefactory/Semantics/ResolveResult.cs @@ -70,5 +70,10 @@ namespace ICSharpCode.NRefactory.Semantics { return DomRegion.Empty; } + + public virtual ResolveResult ShallowClone() + { + return (ResolveResult)MemberwiseClone(); + } } } diff --git a/ICSharpCode.NRefactory/TypeSystem/Implementation/DefaultMemberReference.cs b/ICSharpCode.NRefactory/TypeSystem/Implementation/DefaultMemberReference.cs index 5c630f9c09..c82a3a3017 100644 --- a/ICSharpCode.NRefactory/TypeSystem/Implementation/DefaultMemberReference.cs +++ b/ICSharpCode.NRefactory/TypeSystem/Implementation/DefaultMemberReference.cs @@ -63,7 +63,9 @@ namespace ICSharpCode.NRefactory.TypeSystem.Implementation IType type = typeReference.Resolve(context); IEnumerable members; if (entityType == EntityType.Accessor) { - members = type.GetAccessors(m => m.Name == name && !m.IsExplicitInterfaceImplementation); + members = type.GetAccessors( + m => m.Name == name && !m.IsExplicitInterfaceImplementation, + GetMemberOptions.IgnoreInheritedMembers); } else if (entityType == EntityType.Method) { members = type.GetMethods( m => m.Name == name && m.EntityType == EntityType.Method