diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index f7b7ab0489db1f742fb479d8df4f6e2de81a5a5b..adcf88ecc0450260b9694d64ba078fd3fa873a8b 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -31,3 +31,7 @@ How did we miss it? What tests are we adding to guard against it in the future? **How was the bug found?** (E.g. customer reported it vs. ad hoc testing) + +**Test documentation updated?** + +If this is a new non-compiler feature or a significant improvement to an existing feature, update https://github.com/dotnet/roslyn/wiki/Manual-Testing once you know which release it is targeting. diff --git a/build/Targets/Packages.props b/build/Targets/Packages.props index 1a70204d60e876f6fa362fe4b875518d65e483a4..14fc3cdfd8e9f344ad96dfbd44ccf31eb3f5f79f 100644 --- a/build/Targets/Packages.props +++ b/build/Targets/Packages.props @@ -35,9 +35,9 @@ 0.8.31-beta 1.0.35 1.2.0-beta1-62008-01 - 1.1.0-beta1-62008-01 + 1.1.0-beta1-62016-01 1.7.0-private-25604 - 1.3.0-beta1-62008-01 + 1.4.0-beta1-62016-01 4.7.2-alpha-00001 1.0.27-prerelease-01811-02 3.13.8 diff --git a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs index ca882a93a4ccf5daf6e8f8e131b8f0376eaff18b..b66f89f5f07d2b68788d908ffaeb239f544abb28 100644 --- a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs +++ b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs @@ -660,7 +660,7 @@ private void CompileSynthesizedMethods(TypeCompilationState compilationState) var diagnosticsThisMethod = DiagnosticBag.GetInstance(); var method = methodWithBody.Method; - var lambda = method as SynthesizedLambdaMethod; + var lambda = method as SynthesizedClosureMethod; var variableSlotAllocatorOpt = ((object)lambda != null) ? _moduleBeingBuiltOpt.TryCreateVariableSlotAllocator(lambda, lambda.TopLevelMethod, diagnosticsThisMethod) : _moduleBeingBuiltOpt.TryCreateVariableSlotAllocator(method, method, diagnosticsThisMethod); diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.Tree.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.Tree.cs index 3995278c7913a8e601f7e6b9026002ee9848eea3..769035d013dceca21cdad84c6a6aa3f366fb2653 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.Tree.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.Tree.cs @@ -112,6 +112,11 @@ public sealed class Closure /// public readonly MethodSymbol OriginalMethodSymbol; + /// + /// Syntax for the block of the nested function. + /// + public readonly SyntaxReference BlockSyntax; + public readonly PooledHashSet CapturedVariables = PooledHashSet.GetInstance(); public readonly ArrayBuilder CapturedEnvironments @@ -136,10 +141,13 @@ public bool CapturesThis } } - public Closure(MethodSymbol symbol) + public SynthesizedClosureMethod SynthesizedLoweredMethod; + + public Closure(MethodSymbol symbol, SyntaxReference blockSyntax) { Debug.Assert(symbol != null); OriginalMethodSymbol = symbol; + BlockSyntax = blockSyntax; } public void Free() @@ -154,9 +162,8 @@ public sealed class ClosureEnvironment public readonly SetWithInsertionOrder CapturedVariables; /// - /// Represents a that had its environment - /// pointer (a local pointing to the environment) captured like a captured - /// variable. Assigned in + /// True if this environment captures a reference to a class environment + /// declared in a higher scope. Assigned by /// /// public bool CapturesParent; @@ -435,7 +442,7 @@ private BoundNode VisitClosure(MethodSymbol closureSymbol, BoundBlock body) // Closure is declared (lives) in the parent scope, but its // variables are in a nested scope - var closure = new Closure(closureSymbol); + var closure = new Closure(closureSymbol, body.Syntax.GetReference()); _currentScope.Closures.Add(closure); var oldClosure = _currentClosure; diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.LocalFunctionReferenceRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.LocalFunctionReferenceRewriter.cs deleted file mode 100644 index 95bb6cd7400f17aa24c0a34edf91bbe8620d754c..0000000000000000000000000000000000000000 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.LocalFunctionReferenceRewriter.cs +++ /dev/null @@ -1,293 +0,0 @@ -// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using Microsoft.CodeAnalysis.CSharp.Symbols; -using Microsoft.CodeAnalysis.PooledObjects; -using System.Collections.Generic; -using System.Collections.Immutable; -using System.Diagnostics; - -namespace Microsoft.CodeAnalysis.CSharp -{ - internal sealed partial class LambdaRewriter : MethodToClassRewriter - { - /// - /// This pass is expected to run on partially lowered methods - /// previously containing one or more local functions. At this - /// point all local functions should have been rewritten into - /// proper closure classes and have frames and proxies generated - /// for them. - /// - /// The only thing left is to visit all "references" to local functions - /// and rewrite them to be references to the rewritten form. - /// - private sealed class LocalFunctionReferenceRewriter : BoundTreeRewriterWithStackGuard - { - private readonly LambdaRewriter _lambdaRewriter; - - public LocalFunctionReferenceRewriter(LambdaRewriter lambdaRewriter) - { - _lambdaRewriter = lambdaRewriter; - } - - public override BoundNode Visit(BoundNode node) - { - var partiallyLowered = node as PartiallyLoweredLocalFunctionReference; - if (partiallyLowered != null) - { - var underlying = partiallyLowered.UnderlyingNode; - Debug.Assert(underlying.Kind == BoundKind.Call || - underlying.Kind == BoundKind.DelegateCreationExpression || - underlying.Kind == BoundKind.Conversion); - var oldProxies = _lambdaRewriter.proxies; - _lambdaRewriter.proxies = partiallyLowered.Proxies; - - var result = base.Visit(underlying); - - _lambdaRewriter.proxies = oldProxies; - - return result; - } - return base.Visit(node); - } - - public override BoundNode VisitCall(BoundCall node) - { - if (node.Method.MethodKind == MethodKind.LocalFunction) - { - BoundExpression receiver; - MethodSymbol method; - var arguments = node.Arguments; - _lambdaRewriter.RemapLocalFunction( - node.Syntax, - node.Method, - out receiver, - out method, - ref arguments); - node = node.Update(receiver, method, arguments); - } - - return base.VisitCall(node); - } - - public override BoundNode VisitDelegateCreationExpression(BoundDelegateCreationExpression node) - { - if (node.MethodOpt?.MethodKind == MethodKind.LocalFunction) - { - BoundExpression receiver; - MethodSymbol method; - var arguments = default(ImmutableArray); - _lambdaRewriter.RemapLocalFunction( - node.Syntax, - node.MethodOpt, - out receiver, - out method, - ref arguments); - - return new BoundDelegateCreationExpression( - node.Syntax, receiver, method, isExtensionMethod: false, type: node.Type); - } - - return base.VisitDelegateCreationExpression(node); - } - } - - /// - /// Visit all references to local functions (calls, delegate - /// conversions, delegate creations) and rewrite them to point - /// to the rewritten local function method instead of the original. - /// - public BoundStatement RewriteLocalFunctionReferences(BoundStatement loweredBody) - { - var rewriter = new LocalFunctionReferenceRewriter(this); - - Debug.Assert(_currentMethod == _topLevelMethod); - - // Visit the body first since the state is already set - // for the top-level method - var newBody = (BoundStatement)rewriter.Visit(loweredBody); - - // Visit all the rewritten methods as well - var synthesizedMethods = _synthesizedMethods; - if (synthesizedMethods != null) - { - var newMethods = ArrayBuilder.GetInstance( - synthesizedMethods.Count); - - foreach (var oldMethod in synthesizedMethods) - { - var synthesizedLambda = oldMethod.Method as SynthesizedLambdaMethod; - if (synthesizedLambda == null) - { - // The only methods synthesized by the rewriter should - // be lowered closures and frame constructors - Debug.Assert(oldMethod.Method.MethodKind == MethodKind.Constructor || - oldMethod.Method.MethodKind == MethodKind.StaticConstructor); - newMethods.Add(oldMethod); - continue; - } - - _currentMethod = synthesizedLambda; - var closureKind = synthesizedLambda.ClosureKind; - if (closureKind == ClosureKind.Static || closureKind == ClosureKind.Singleton) - { - // no link from a static lambda to its container - _innermostFramePointer = _currentFrameThis = null; - } - else - { - _currentFrameThis = synthesizedLambda.ThisParameter; - _innermostFramePointer = null; - _framePointers.TryGetValue(synthesizedLambda.ContainingType, out _innermostFramePointer); - } - - var containerAsFrame = synthesizedLambda.ContainingType as SynthesizedClosureEnvironment; - - // Includes type parameters from the containing type iff - // the containing type is a frame. If it is a frame then - // the type parameters are captured, meaning that the - // type parameters should be included. - // If it is not a frame then the local function is being - // directly lowered into the method's containing type and - // the parameters should never be substituted. - _currentTypeParameters = containerAsFrame?.TypeParameters.Concat(synthesizedLambda.TypeParameters) - ?? synthesizedLambda.TypeParameters; - _currentLambdaBodyTypeMap = synthesizedLambda.TypeMap; - - var rewrittenBody = (BoundStatement)rewriter.Visit(oldMethod.Body); - - var newMethod = new TypeCompilationState.MethodWithBody( - synthesizedLambda, rewrittenBody, oldMethod.ImportChainOpt); - newMethods.Add(newMethod); - } - - _synthesizedMethods = newMethods; - synthesizedMethods.Free(); - } - - return newBody; - } - - /// - /// Rewrites a reference to an unlowered local function to the newly - /// lowered local function. - /// - private void RemapLocalFunction( - SyntaxNode syntax, - MethodSymbol localFunc, - out BoundExpression receiver, - out MethodSymbol method, - ref ImmutableArray parameters) - { - Debug.Assert(localFunc.MethodKind == MethodKind.LocalFunction); - - var mappedLocalFunction = _localFunctionMap[(LocalFunctionSymbol)localFunc.OriginalDefinition]; - var loweredSymbol = mappedLocalFunction.Symbol; - - // If the local function captured variables then they will be stored - // in frames and the frames need to be passed as extra parameters. - var frameCount = loweredSymbol.ExtraSynthesizedParameterCount; - if (frameCount != 0) - { - Debug.Assert(!parameters.IsDefault); - - // Build a new list of parameters to pass to the local function - // call that includes any necessary capture frames - var parametersBuilder = ArrayBuilder.GetInstance(); - parametersBuilder.AddRange(parameters); - - var start = loweredSymbol.ParameterCount - frameCount; - for (int i = start; i < loweredSymbol.ParameterCount; i++) - { - // will always be a LambdaFrame, it's always a capture frame - var frameType = (NamedTypeSymbol)loweredSymbol.Parameters[i].Type.OriginalDefinition; - - Debug.Assert(frameType is SynthesizedClosureEnvironment); - - if (frameType.Arity > 0) - { - var typeParameters = ((SynthesizedClosureEnvironment)frameType).ConstructedFromTypeParameters; - Debug.Assert(typeParameters.Length == frameType.Arity); - var subst = this.TypeMap.SubstituteTypeParameters(typeParameters); - frameType = frameType.Construct(subst); - } - - var frame = FrameOfType(syntax, frameType); - parametersBuilder.Add(frame); - } - parameters = parametersBuilder.ToImmutableAndFree(); - } - - method = loweredSymbol; - NamedTypeSymbol constructedFrame; - - RemapLambdaOrLocalFunction(syntax, - localFunc, - SubstituteTypeArguments(localFunc.TypeArguments), - mappedLocalFunction.ClosureKind, - ref method, - out receiver, - out constructedFrame); - } - - /// - /// Substitutes references from old type arguments to new type arguments - /// in the lowered methods. - /// - /// - /// Consider the following method: - /// void M() { - /// void L<T>(T t) => Console.Write(t); - /// L("A"); - /// } - /// - /// In this example, L<T> is a local function that will be - /// lowered into its own method and the type parameter T will be - /// alpha renamed to something else (let's call it T'). In this case, - /// all references to the original type parameter T in L must be - /// rewritten to the renamed parameter, T'. - /// - private ImmutableArray SubstituteTypeArguments(ImmutableArray typeArguments) - { - Debug.Assert(!typeArguments.IsDefault); - - if (typeArguments.IsEmpty) - { - return typeArguments; - } - - // We must perform this process repeatedly as local - // functions may nest inside one another and capture type - // parameters from the enclosing local functions. Each - // iteration of nesting will cause alpha-renaming of the captured - // parameters, meaning that we must replace until there are no - // more alpha-rename mappings. - // - // The method symbol references are different from all other - // substituted types in this context because the method symbol in - // local function references is not rewritten until all local - // functions have already been lowered. Everything else is rewritten - // by the visitors as the definition is lowered. This means that - // only one substition happens per lowering, but we need to do - // N substitutions all at once, where N is the number of lowerings. - - var builder = ArrayBuilder.GetInstance(); - foreach (var typeArg in typeArguments) - { - TypeSymbol oldTypeArg; - TypeSymbol newTypeArg = typeArg; - do - { - oldTypeArg = newTypeArg; - newTypeArg = this.TypeMap.SubstituteType(typeArg).Type; - } - while (oldTypeArg != newTypeArg); - - Debug.Assert((object)oldTypeArg == newTypeArg); - - builder.Add(newTypeArg); - } - - return builder.ToImmutableAndFree(); - } - } -} diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs index 9a8c3dc5de783f6331c12bf0d07d02589642b1da..4c43b74d4a1ad7feae40a951ee8adbcaac0bce11 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs @@ -78,20 +78,6 @@ internal sealed partial class LambdaRewriter : MethodToClassRewriter // A mapping from every lambda parameter to its corresponding method's parameter. private readonly Dictionary _parameterMap = new Dictionary(); - // A mapping from every local function to its lowered method - private struct MappedLocalFunction - { - public readonly SynthesizedLambdaMethod Symbol; - public readonly ClosureKind ClosureKind; - public MappedLocalFunction(SynthesizedLambdaMethod symbol, ClosureKind closureKind) - { - Symbol = symbol; - ClosureKind = closureKind; - } - } - - private readonly Dictionary _localFunctionMap = new Dictionary(); - // for each block with lifted (captured) variables, the corresponding frame type private readonly Dictionary _frames = new Dictionary(); @@ -268,18 +254,11 @@ protected override bool NeedsProxy(Symbol localOrParameter) assignLocals); rewriter.SynthesizeClosureEnvironments(closureDebugInfoBuilder); + rewriter.SynthesizeLoweredFunctionMethods(); - // First, lower everything but references (calls, delegate conversions) - // to local functions var body = rewriter.AddStatementsIfNeeded( (BoundStatement)rewriter.Visit(loweredBody)); - // Now lower the references - if (rewriter._localFunctionMap.Count != 0) - { - body = rewriter.RewriteLocalFunctionReferences(body); - } - // Add the completed methods to the compilation state if (rewriter._synthesizedMethods != null) { @@ -404,10 +383,94 @@ SynthesizedClosureEnvironment MakeFrame(Analysis.Scope scope, bool isStruct) syntax, methodId, closureId); + } } - } - private SynthesizedClosureEnvironment GetStaticFrame(DiagnosticBag diagnostics, IBoundLambdaOrFunction lambda) + /// + /// Synthesize the final signature for all closures. + /// + private void SynthesizeLoweredFunctionMethods() + { + Analysis.VisitClosures(_analysis.ScopeTree, (scope, closure) => + { + var originalMethod = closure.OriginalMethodSymbol; + var syntax = originalMethod.DeclaringSyntaxReferences[0].GetSyntax(); + var structClosures = closure.CapturedEnvironments + .Where(env => env.IsStruct).Select(env => env.SynthesizedEnvironment).AsImmutable(); + + int closureOrdinal; + ClosureKind closureKind; + NamedTypeSymbol translatedLambdaContainer; + SynthesizedClosureEnvironment containerAsFrame; + DebugId topLevelMethodId; + DebugId lambdaId; + if (closure.ContainingEnvironmentOpt != null) + { + containerAsFrame = closure.ContainingEnvironmentOpt?.SynthesizedEnvironment; + + closureKind = ClosureKind.General; + translatedLambdaContainer = containerAsFrame; + closureOrdinal = containerAsFrame.ClosureOrdinal; + } + else if (closure.CapturesThis) + { + containerAsFrame = null; + translatedLambdaContainer = _topLevelMethod.ContainingType; + closureKind = ClosureKind.ThisOnly; + closureOrdinal = LambdaDebugInfo.ThisOnlyClosureOrdinal; + } + else if (closure.CapturedEnvironments.Count == 0) + { + if (_analysis.MethodsConvertedToDelegates.Contains(originalMethod)) + { + translatedLambdaContainer = containerAsFrame = GetStaticFrame(Diagnostics, syntax); + closureKind = ClosureKind.Singleton; + closureOrdinal = LambdaDebugInfo.StaticClosureOrdinal; + } + else + { + containerAsFrame = null; + translatedLambdaContainer = _topLevelMethod.ContainingType; + closureKind = ClosureKind.Static; + closureOrdinal = LambdaDebugInfo.StaticClosureOrdinal; + } + } + else + { + // Lower directly onto the containing type + containerAsFrame = null; + closureKind = ClosureKind.Static; // not exactly... but we've rewritten the receiver to be a by-ref parameter + translatedLambdaContainer = _topLevelMethod.ContainingType; + closureOrdinal = LambdaDebugInfo.StaticClosureOrdinal; + } + + // Move the body of the lambda to a freshly generated synthetic method on its frame. + topLevelMethodId = _analysis.GetTopLevelMethodId(); + lambdaId = GetLambdaId(syntax, closureKind, closureOrdinal); + + var synthesizedMethod = new SynthesizedClosureMethod( + translatedLambdaContainer, + structClosures, + closureKind, + _topLevelMethod, + topLevelMethodId, + originalMethod, + closure.BlockSyntax, + lambdaId); + closure.SynthesizedLoweredMethod = synthesizedMethod; + }); + } + + /// + /// Get the static container for closures or create one if one doesn't already exist. + /// + /// + /// associate the frame with the first lambda that caused it to exist. + /// we need to associate this with some syntax. + /// unfortunately either containing method or containing class could be synthetic + /// therefore could have no syntax. + /// + private SynthesizedClosureEnvironment GetStaticFrame(DiagnosticBag diagnostics, SyntaxNode syntax) { if (_lazyStaticLambdaFrame == null) { @@ -458,12 +521,6 @@ private SynthesizedClosureEnvironment GetStaticFrame(DiagnosticBag diagnostics, MethodCompiler.BindMethodBody(frame.Constructor, CompilationState, null), frame.Constructor)); - // associate the frame with the first lambda that caused it to exist. - // we need to associate this with some syntax. - // unfortunately either containing method or containing class could be synthetic - // therefore could have no syntax. - SyntaxNode syntax = lambda.Syntax; - // add cctor // Frame.inst = new Frame() var F = new SyntheticBoundNodeFactory(frame.StaticConstructor, syntax, CompilationState, diagnostics); @@ -512,7 +569,7 @@ protected override BoundExpression FramePointer(SyntaxNode syntax, NamedTypeSymb } // If the current method has by-ref struct closure parameters, and one of them is correct, use it. - var lambda = _currentMethod as SynthesizedLambdaMethod; + var lambda = _currentMethod as SynthesizedClosureMethod; if (lambda != null) { var start = lambda.ParameterCount - lambda.ExtraSynthesizedParameterCount; @@ -737,6 +794,129 @@ public override BoundNode VisitBaseReference(BoundBaseReference node) : FramePointer(node.Syntax, _topLevelMethod.ContainingType); // technically, not the correct static type } + /// + /// Rewrites a reference to an unlowered local function to the newly + /// lowered local function. + /// + private void RemapLocalFunction( + SyntaxNode syntax, + MethodSymbol localFunc, + out BoundExpression receiver, + out MethodSymbol method, + ref ImmutableArray parameters) + { + Debug.Assert(localFunc.MethodKind == MethodKind.LocalFunction); + + var closure = Analysis.GetClosureInTree(_analysis.ScopeTree, localFunc.OriginalDefinition); + var loweredSymbol = closure.SynthesizedLoweredMethod; + + // If the local function captured variables then they will be stored + // in frames and the frames need to be passed as extra parameters. + var frameCount = loweredSymbol.ExtraSynthesizedParameterCount; + if (frameCount != 0) + { + Debug.Assert(!parameters.IsDefault); + + // Build a new list of parameters to pass to the local function + // call that includes any necessary capture frames + var parametersBuilder = ArrayBuilder.GetInstance(); + parametersBuilder.AddRange(parameters); + + var start = loweredSymbol.ParameterCount - frameCount; + for (int i = start; i < loweredSymbol.ParameterCount; i++) + { + // will always be a LambdaFrame, it's always a capture frame + var frameType = (NamedTypeSymbol)loweredSymbol.Parameters[i].Type.OriginalDefinition; + + Debug.Assert(frameType is SynthesizedClosureEnvironment); + + if (frameType.Arity > 0) + { + var typeParameters = ((SynthesizedClosureEnvironment)frameType).ConstructedFromTypeParameters; + Debug.Assert(typeParameters.Length == frameType.Arity); + var subst = this.TypeMap.SubstituteTypeParameters(typeParameters); + frameType = frameType.Construct(subst); + } + + var frame = FrameOfType(syntax, frameType); + parametersBuilder.Add(frame); + } + parameters = parametersBuilder.ToImmutableAndFree(); + } + + method = loweredSymbol; + NamedTypeSymbol constructedFrame; + + RemapLambdaOrLocalFunction(syntax, + localFunc, + SubstituteTypeArguments(localFunc.TypeArguments), + loweredSymbol.ClosureKind, + ref method, + out receiver, + out constructedFrame); + } + + /// + /// Substitutes references from old type arguments to new type arguments + /// in the lowered methods. + /// + /// + /// Consider the following method: + /// void M() { + /// void L<T>(T t) => Console.Write(t); + /// L("A"); + /// } + /// + /// In this example, L<T> is a local function that will be + /// lowered into its own method and the type parameter T will be + /// alpha renamed to something else (let's call it T'). In this case, + /// all references to the original type parameter T in L must be + /// rewritten to the renamed parameter, T'. + /// + private ImmutableArray SubstituteTypeArguments(ImmutableArray typeArguments) + { + Debug.Assert(!typeArguments.IsDefault); + + if (typeArguments.IsEmpty) + { + return typeArguments; + } + + // We must perform this process repeatedly as local + // functions may nest inside one another and capture type + // parameters from the enclosing local functions. Each + // iteration of nesting will cause alpha-renaming of the captured + // parameters, meaning that we must replace until there are no + // more alpha-rename mappings. + // + // The method symbol references are different from all other + // substituted types in this context because the method symbol in + // local function references is not rewritten until all local + // functions have already been lowered. Everything else is rewritten + // by the visitors as the definition is lowered. This means that + // only one substition happens per lowering, but we need to do + // N substitutions all at once, where N is the number of lowerings. + + var builder = ArrayBuilder.GetInstance(); + foreach (var typeArg in typeArguments) + { + TypeSymbol oldTypeArg; + TypeSymbol newTypeArg = typeArg; + do + { + oldTypeArg = newTypeArg; + newTypeArg = this.TypeMap.SubstituteType(typeArg).Type; + } + while (oldTypeArg != newTypeArg); + + Debug.Assert((object)oldTypeArg == newTypeArg); + + builder.Add(newTypeArg); + } + + return builder.ToImmutableAndFree(); + } + private void RemapLambdaOrLocalFunction( SyntaxNode syntax, MethodSymbol originalMethod, @@ -796,22 +976,24 @@ public override BoundNode VisitBaseReference(BoundBaseReference node) } } - /// - /// This pass doesn't rewrite the local function calls themselves - /// because we may encounter a call to a local function that has yet - /// to be lowered. Here we just want to make sure we lower the - /// arguments as they may contain references to captured variables. - /// The final lowering of the call will be in the - /// - /// public override BoundNode VisitCall(BoundCall node) { if (node.Method.MethodKind == MethodKind.LocalFunction) { - var withArguments = node.Update( - node.ReceiverOpt, + var args = VisitList(node.Arguments); + var type = VisitType(node.Type); + + RemapLocalFunction( + node.Syntax, node.Method, - this.VisitList(node.Arguments), + out var receiver, + out var method, + ref args); + + return node.Update( + receiver, + method, + args, node.ArgumentNamesOpt, node.ArgumentRefKindsOpt, node.IsDelegateCall, @@ -820,9 +1002,7 @@ public override BoundNode VisitCall(BoundCall node) node.ArgsToParamsOpt, node.ResultKind, node.BinderOpt, - this.VisitType(node.Type)); - - return PartiallyLowerLocalFunctionReference(withArguments); + type); } var visited = base.VisitCall(node); @@ -856,17 +1036,6 @@ public override BoundNode VisitCall(BoundCall node) return rewritten; } - private PartiallyLoweredLocalFunctionReference PartiallyLowerLocalFunctionReference( - BoundExpression underlyingNode) - { - Debug.Assert(underlyingNode.Kind == BoundKind.Call || - underlyingNode.Kind == BoundKind.DelegateCreationExpression || - underlyingNode.Kind == BoundKind.Conversion); - return new PartiallyLoweredLocalFunctionReference( - underlyingNode, - new Dictionary(proxies)); - } - private BoundSequence RewriteSequence(BoundSequence node, ArrayBuilder prologue, ArrayBuilder newLocals) { RewriteLocals(node.Locals, newLocals); @@ -1078,13 +1247,20 @@ public override BoundNode VisitDelegateCreationExpression(BoundDelegateCreationE if (node.MethodOpt?.MethodKind == MethodKind.LocalFunction) { - var rewritten = node.Update( - node.Argument, + var arguments = default(ImmutableArray); + RemapLocalFunction( + node.Syntax, node.MethodOpt, - node.IsExtensionMethod, - this.VisitType(node.Type)); + out var receiver, + out var method, + ref arguments); - return PartiallyLowerLocalFunctionReference(rewritten); + return new BoundDelegateCreationExpression( + node.Syntax, + receiver, + method, + node.IsExtensionMethod, + VisitType(node.Type)); } return base.VisitDelegateCreationExpression(node); } @@ -1188,7 +1364,7 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos return lambdaId; } - private SynthesizedLambdaMethod RewriteLambdaOrLocalFunction( + private SynthesizedClosureMethod RewriteLambdaOrLocalFunction( IBoundLambdaOrFunction node, out ClosureKind closureKind, out NamedTypeSymbol translatedLambdaContainer, @@ -1198,18 +1374,17 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos out DebugId lambdaId) { Analysis.Closure closure = Analysis.GetClosureInTree(_analysis.ScopeTree, node.Symbol); + var synthesizedMethod = closure.SynthesizedLoweredMethod; + Debug.Assert(synthesizedMethod != null); - var structClosures = closure.CapturedEnvironments - .Where(env => env.IsStruct).Select(env => env.SynthesizedEnvironment).AsImmutable(); + closureKind = synthesizedMethod.ClosureKind; + translatedLambdaContainer = synthesizedMethod.ContainingType; + containerAsFrame = translatedLambdaContainer as SynthesizedClosureEnvironment; + topLevelMethodId = _analysis.GetTopLevelMethodId(); + lambdaId = synthesizedMethod.LambdaId; - int closureOrdinal; if (closure.ContainingEnvironmentOpt != null) { - containerAsFrame = closure.ContainingEnvironmentOpt?.SynthesizedEnvironment; - - closureKind = ClosureKind.General; - translatedLambdaContainer = containerAsFrame; - closureOrdinal = containerAsFrame.ClosureOrdinal; // Find the scope of the containing environment BoundNode tmpScope = null; Analysis.VisitScopeTree(_analysis.ScopeTree, scope => @@ -1222,46 +1397,11 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos Debug.Assert(tmpScope != null); lambdaScope = tmpScope; } - else if (closure.CapturesThis) - { - lambdaScope = null; - containerAsFrame = null; - translatedLambdaContainer = _topLevelMethod.ContainingType; - closureKind = ClosureKind.ThisOnly; - closureOrdinal = LambdaDebugInfo.ThisOnlyClosureOrdinal; - } - else if (closure.CapturedEnvironments.Count == 0) - { - if (_analysis.MethodsConvertedToDelegates.Contains(node.Symbol)) - { - translatedLambdaContainer = containerAsFrame = GetStaticFrame(Diagnostics, node); - closureKind = ClosureKind.Singleton; - closureOrdinal = LambdaDebugInfo.StaticClosureOrdinal; - } - else - { - containerAsFrame = null; - translatedLambdaContainer = _topLevelMethod.ContainingType; - closureKind = ClosureKind.Static; - closureOrdinal = LambdaDebugInfo.StaticClosureOrdinal; - } - lambdaScope = null; - } else { - // Lower directly onto the containing type - containerAsFrame = null; lambdaScope = null; - closureKind = ClosureKind.Static; // not exactly... but we've rewritten the receiver to be a by-ref parameter - translatedLambdaContainer = _topLevelMethod.ContainingType; - closureOrdinal = LambdaDebugInfo.StaticClosureOrdinal; } - - // Move the body of the lambda to a freshly generated synthetic method on its frame. - topLevelMethodId = _analysis.GetTopLevelMethodId(); - lambdaId = GetLambdaId(node.Syntax, closureKind, closureOrdinal); - - var synthesizedMethod = new SynthesizedLambdaMethod(translatedLambdaContainer, structClosures, closureKind, _topLevelMethod, topLevelMethodId, node, lambdaId); + CompilationState.ModuleBuilderOpt.AddSynthesizedDefinition(translatedLambdaContainer, synthesizedMethod); foreach (var parameter in node.Symbol.Parameters) @@ -1269,11 +1409,6 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos _parameterMap.Add(parameter, synthesizedMethod.Parameters[parameter.Ordinal]); } - if (node is BoundLocalFunctionStatement) - { - _localFunctionMap[((BoundLocalFunctionStatement)node).Symbol] = new MappedLocalFunction(synthesizedMethod, closureKind); - } - // rewrite the lambda body as the generated method's body var oldMethod = _currentMethod; var oldFrameThis = _currentFrameThis; @@ -1354,7 +1489,7 @@ private BoundNode RewriteLambdaConversion(BoundLambda node) BoundNode lambdaScope; DebugId topLevelMethodId; DebugId lambdaId; - SynthesizedLambdaMethod synthesizedMethod = RewriteLambdaOrLocalFunction( + SynthesizedClosureMethod synthesizedMethod = RewriteLambdaOrLocalFunction( node, out closureKind, out translatedLambdaContainer, diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/PartiallyLoweredLocalFunctionReference.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/PartiallyLoweredLocalFunctionReference.cs deleted file mode 100644 index c79d1ca0f04bc9fb93d5d1f8199a062f184ac24f..0000000000000000000000000000000000000000 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/PartiallyLoweredLocalFunctionReference.cs +++ /dev/null @@ -1,33 +0,0 @@ -using System; -using System.Collections.Generic; -using Microsoft.CodeAnalysis.Semantics; - -namespace Microsoft.CodeAnalysis.CSharp -{ - /// - /// This represents a partially lowered local function reference (e.g., - /// a local function call or delegate conversion) with relevant proxies - /// attached. It will later be rewritten by the - /// into a - /// proper call. - /// - internal class PartiallyLoweredLocalFunctionReference : BoundExpression - { - private const BoundKind s_privateKind = (BoundKind)byte.MaxValue; - - public BoundExpression UnderlyingNode { get; } - public Dictionary Proxies { get; } - - public PartiallyLoweredLocalFunctionReference( - BoundExpression underlying, - Dictionary proxies) - : base(s_privateKind, underlying.Syntax, underlying.Type) - { - UnderlyingNode = underlying; - Proxies = proxies; - } - - public override BoundNode Accept(BoundTreeVisitor visitor) => - visitor.Visit(this); - } -} diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedLambdaMethod.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedClosureMethod.cs similarity index 77% rename from src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedLambdaMethod.cs rename to src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedClosureMethod.cs index 9bb6e7418db6da311c5a3a66d8df7d2a150c19a3..6d27d06955a7c30a22a0aad05573a5a5d1082c9c 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedLambdaMethod.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedClosureMethod.cs @@ -12,51 +12,62 @@ namespace Microsoft.CodeAnalysis.CSharp /// /// A method that results from the translation of a single lambda expression. /// - internal sealed class SynthesizedLambdaMethod : SynthesizedMethodBaseSymbol, ISynthesizedMethodBodyImplementationSymbol + internal sealed class SynthesizedClosureMethod : SynthesizedMethodBaseSymbol, ISynthesizedMethodBodyImplementationSymbol { private readonly MethodSymbol _topLevelMethod; private readonly ImmutableArray _structEnvironments; - internal SynthesizedLambdaMethod( + internal readonly DebugId LambdaId; + + internal SynthesizedClosureMethod( NamedTypeSymbol containingType, ImmutableArray structEnvironments, ClosureKind closureKind, MethodSymbol topLevelMethod, DebugId topLevelMethodId, - IBoundLambdaOrFunction lambdaNode, + MethodSymbol originalMethod, + SyntaxReference blockSyntax, DebugId lambdaId) : base(containingType, - lambdaNode.Symbol, + originalMethod, null, - lambdaNode.Syntax.SyntaxTree.GetReference(lambdaNode.Body.Syntax), - lambdaNode.Syntax.GetLocation(), - lambdaNode is BoundLocalFunctionStatement ? - MakeName(topLevelMethod.Name, lambdaNode.Symbol.Name, topLevelMethodId, closureKind, lambdaId) : - MakeName(topLevelMethod.Name, topLevelMethodId, closureKind, lambdaId), - (closureKind == ClosureKind.ThisOnly ? DeclarationModifiers.Private : DeclarationModifiers.Internal) - | (closureKind == ClosureKind.Static ? DeclarationModifiers.Static : 0) - | (lambdaNode.Symbol.IsAsync ? DeclarationModifiers.Async : 0)) + blockSyntax, + originalMethod.DeclaringSyntaxReferences[0].GetLocation(), + originalMethod is LocalFunctionSymbol + ? MakeName(topLevelMethod.Name, originalMethod.Name, topLevelMethodId, closureKind, lambdaId) + : MakeName(topLevelMethod.Name, topLevelMethodId, closureKind, lambdaId), + MakeDeclarationModifiers(closureKind, originalMethod)) { _topLevelMethod = topLevelMethod; ClosureKind = closureKind; + LambdaId = lambdaId; TypeMap typeMap; ImmutableArray typeParameters; ImmutableArray constructedFromTypeParameters; - SynthesizedClosureEnvironment lambdaFrame; - lambdaFrame = this.ContainingType as SynthesizedClosureEnvironment; + var lambdaFrame = ContainingType as SynthesizedClosureEnvironment; switch (closureKind) { case ClosureKind.Singleton: // all type parameters on method (except the top level method's) case ClosureKind.General: // only lambda's type parameters on method (rest on class) Debug.Assert(lambdaFrame != null); - typeMap = lambdaFrame.TypeMap.WithConcatAlphaRename(lambdaNode.Symbol, this, out typeParameters, out constructedFromTypeParameters, lambdaFrame.OriginalContainingMethodOpt); + typeMap = lambdaFrame.TypeMap.WithConcatAlphaRename( + originalMethod, + this, + out typeParameters, + out constructedFromTypeParameters, + lambdaFrame.OriginalContainingMethodOpt); break; case ClosureKind.ThisOnly: // all type parameters on method case ClosureKind.Static: Debug.Assert(lambdaFrame == null); - typeMap = TypeMap.Empty.WithConcatAlphaRename(lambdaNode.Symbol, this, out typeParameters, out constructedFromTypeParameters, null); + typeMap = TypeMap.Empty.WithConcatAlphaRename( + originalMethod, + this, + out typeParameters, + out constructedFromTypeParameters, + stopAt: null); break; default: throw ExceptionUtilities.UnexpectedValue(closureKind); @@ -90,6 +101,23 @@ internal sealed class SynthesizedLambdaMethod : SynthesizedMethodBaseSymbol, ISy AssignTypeMapAndTypeParameters(typeMap, typeParameters); } + private static DeclarationModifiers MakeDeclarationModifiers(ClosureKind closureKind, MethodSymbol originalMethod) + { + var mods = closureKind == ClosureKind.ThisOnly ? DeclarationModifiers.Private : DeclarationModifiers.Internal; + + if (closureKind == ClosureKind.Static) + { + mods |= DeclarationModifiers.Static; + } + + if (originalMethod.IsAsync) + { + mods |= DeclarationModifiers.Async; + } + + return mods; + } + private static string MakeName(string topLevelMethodName, string localFunctionName, DebugId topLevelMethodId, ClosureKind closureKind, DebugId lambdaId) { return GeneratedNames.MakeLocalFunctionName( diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenClosureLambdaTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenClosureLambdaTests.cs index 3a544712af69d3693e5e3e760c282c5ecdca66c9..bc76ce3893e60adcb6da917a38bb7165d1438eb7 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenClosureLambdaTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenClosureLambdaTests.cs @@ -4889,15 +4889,15 @@ .maxstack 3 IL_0018: ldc.i4.1 IL_0019: callvirt ""System.Func System.Func>.Invoke(int)"" IL_001e: pop - IL_001f: ldsfld ""System.Func> Program.<>c.<>9__1_2"" + IL_001f: ldsfld ""System.Func> Program.<>c.<>9__1_1"" IL_0024: dup IL_0025: brtrue.s IL_003e IL_0027: pop IL_0028: ldsfld ""Program.<>c Program.<>c.<>9"" - IL_002d: ldftn ""System.Func Program.<>c.b__1_2(int)"" + IL_002d: ldftn ""System.Func Program.<>c.b__1_1(int)"" IL_0033: newobj ""System.Func>..ctor(object, System.IntPtr)"" IL_0038: dup - IL_0039: stsfld ""System.Func> Program.<>c.<>9__1_2"" + IL_0039: stsfld ""System.Func> Program.<>c.<>9__1_1"" IL_003e: ldc.i4.1 IL_003f: callvirt ""System.Func System.Func>.Invoke(int)"" IL_0044: pop @@ -4905,7 +4905,7 @@ .maxstack 3 } "); - verifier.VerifyIL("Program.<>c.b__1_2(int)", + verifier.VerifyIL("Program.<>c.b__1_1(int)", @" { // Code size 44 (0x2c) @@ -4943,17 +4943,17 @@ .locals init (System.Func V_0) IL_0004: ldc.i4.s 123 IL_0006: call ""void System.Console.WriteLine(int)"" IL_000b: ldarg.0 - IL_000c: ldfld ""System.Func Program.<>c__DisplayClass1_0.<>9__1"" + IL_000c: ldfld ""System.Func Program.<>c__DisplayClass1_0.<>9__2"" IL_0011: dup IL_0012: brtrue.s IL_002a IL_0014: pop IL_0015: ldarg.0 IL_0016: ldarg.0 - IL_0017: ldftn ""int Program.<>c__DisplayClass1_0.b__1()"" + IL_0017: ldftn ""int Program.<>c__DisplayClass1_0.b__2()"" IL_001d: newobj ""System.Func..ctor(object, System.IntPtr)"" IL_0022: dup IL_0023: stloc.0 - IL_0024: stfld ""System.Func Program.<>c__DisplayClass1_0.<>9__1"" + IL_0024: stfld ""System.Func Program.<>c__DisplayClass1_0.<>9__2"" IL_0029: ldloc.0 IL_002a: ret IL_002b: ldnull diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs index efc27dfc74c8f01668ec416fe47bbb4c101740e0..51c13fbea4bfb9e26c64627836fefcc463260e64 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs @@ -274,23 +274,23 @@ .maxstack 2 .maxstack 2 IL_0000: ldarg.0 IL_0001: ldarg.1 - IL_0002: call ""void C.g__L32_2(ref C.<>c__DisplayClass2_0)"" + IL_0002: call ""void C.g__L32_3(ref C.<>c__DisplayClass2_0)"" IL_0007: ret }"); // Skip some... L5 - verifier.VerifyIL("C.g__L52_4(ref C.<>c__DisplayClass2_0, ref C.<>c__DisplayClass2_1)", @" + verifier.VerifyIL("C.g__L52_5(ref C.<>c__DisplayClass2_0, ref C.<>c__DisplayClass2_1)", @" { // Code size 10 (0xa) .maxstack 3 IL_0000: ldarg.0 IL_0001: ldarg.1 IL_0002: ldarg.2 - IL_0003: call ""int C.g__L62_5(ref C.<>c__DisplayClass2_0, ref C.<>c__DisplayClass2_1)"" + IL_0003: call ""int C.g__L62_6(ref C.<>c__DisplayClass2_0, ref C.<>c__DisplayClass2_1)"" IL_0008: pop IL_0009: ret }"); // L6 - verifier.VerifyIL("C.g__L62_5(ref C.<>c__DisplayClass2_0, ref C.<>c__DisplayClass2_1)", @" + verifier.VerifyIL("C.g__L62_6(ref C.<>c__DisplayClass2_0, ref C.<>c__DisplayClass2_1)", @" { // Code size 25 (0x19) .maxstack 4 diff --git a/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueClosureTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueClosureTests.cs index 748e6c12d1ca5a9ad53f40649b7c92a00c02f726..419a4db6a282cd2e24c0fc0328eff200ee38369f 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueClosureTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueClosureTests.cs @@ -549,10 +549,10 @@ void F() // no new synthesized members generated (with #1 in names): diff1.VerifySynthesizedMembers( - "C: {b__1_2, b__1_4, <>c__DisplayClass1_0, <>c__DisplayClass1_1, <>c}", - "C.<>c: {<>9__1_0, <>9__1_1, <>9__1_6, b__1_0, b__1_1, b__1_6}", - "C.<>c__DisplayClass1_0: {<>h__TransparentIdentifier0, b__3}", - "C.<>c__DisplayClass1_1: {<>h__TransparentIdentifier0, b__5}", + "C.<>c__DisplayClass1_1: {<>h__TransparentIdentifier0, b__6}", + "C.<>c__DisplayClass1_0: {<>h__TransparentIdentifier0, b__5}", + "C.<>c: {<>9__1_0, <>9__1_1, <>9__1_4, b__1_0, b__1_1, b__1_4}", + "C: {b__1_2, b__1_3, <>c__DisplayClass1_0, <>c__DisplayClass1_1, <>c}", "<>f__AnonymousType0<j__TPar, j__TPar>: {Equals, GetHashCode, ToString}"); var md1 = diff1.GetMetadata(); @@ -632,10 +632,10 @@ void F() // no new synthesized members generated (with #1 in names): diff1.VerifySynthesizedMembers( - "C: {b__1_0, <>c__DisplayClass1_0, <>c}", - "C.<>c: {<>9__1_2, b__1_2}", - "C.<>c__DisplayClass1_0: {a, b__1}", - "<>f__AnonymousType0<j__TPar, j__TPar>: {Equals, GetHashCode, ToString}"); + "C.<>c: {<>9__1_1, b__1_1}", + "<>f__AnonymousType0<j__TPar, j__TPar>: {Equals, GetHashCode, ToString}", + "C.<>c__DisplayClass1_0: {a, b__2}", + "C: {b__1_0, <>c__DisplayClass1_0, <>c}"); var md1 = diff1.GetMetadata(); var reader1 = md1.Reader; @@ -714,10 +714,10 @@ public void F() // no new synthesized members generated (with #1 in names): diff1.VerifySynthesizedMembers( - "C.<>c__DisplayClass1_2: {a, b, b__5}", - "C.<>c__DisplayClass1_1: {b, b__3}", - "C: {b__1_0, b__1_2, b__1_4, <>c__DisplayClass1_0, <>c__DisplayClass1_1, <>c__DisplayClass1_2}", - "C.<>c__DisplayClass1_0: {a, b__1}"); + "C.<>c__DisplayClass1_1: {b, b__4}", + "C.<>c__DisplayClass1_2: {a, b, b__5}", + "C.<>c__DisplayClass1_0: {a, b__3}", + "C: {b__1_0, b__1_1, b__1_2, <>c__DisplayClass1_0, <>c__DisplayClass1_1, <>c__DisplayClass1_2}"); var md1 = diff1.GetMetadata(); var reader1 = md1.Reader; diff --git a/src/Compilers/CSharp/Test/Symbol/DocumentationComments/CrefTests.cs b/src/Compilers/CSharp/Test/Symbol/DocumentationComments/CrefTests.cs index 5a4a59b5b45111c9240107cb5c51c50d53a575fb..65e296244a8086254f4bcb246c5d721dd1bcc03f 100644 --- a/src/Compilers/CSharp/Test/Symbol/DocumentationComments/CrefTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/DocumentationComments/CrefTests.cs @@ -6599,5 +6599,35 @@ private static Symbol[] GetCrefOriginalDefinitions(SemanticModel model, IEnumera { return crefs.Select(syntax => model.GetSymbolInfo(syntax).Symbol).Select(symbol => (object)symbol == null ? null : (Symbol)symbol.OriginalDefinition).ToArray(); } + + [Fact] + [WorkItem(410932, "https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems?id=410932")] + public void LookupOnCrefTypeParameter() + { + var source = @" +class Test +{ + T F() + { + } + + /// + /// + /// + void S() + { } +} +"; + + var compilation = CreateCompilationWithMscorlibAndDocumentationComments(source); + var tree = compilation.SyntaxTrees[0]; + var model = compilation.GetSemanticModel(tree); + var crefSyntax = (NameMemberCrefSyntax)GetCrefSyntaxes(compilation).Single(); + + var name = ((GenericNameSyntax)crefSyntax.Name).TypeArgumentList.Arguments.Single(); + Assert.Equal("U", name.ToString()); + var typeParameter = (TypeParameterSymbol)model.GetSymbolInfo(name).Symbol; + Assert.Empty(model.LookupSymbols(name.SpanStart, typeParameter, "GetAwaiter")); + } } } diff --git a/src/Compilers/VisualBasic/Portable/Binding/Binder_Lookup.vb b/src/Compilers/VisualBasic/Portable/Binding/Binder_Lookup.vb index ffce0cae9a64108e5fe84ea9502f84747af31f1f..432d2c6e2ca026a01a37042d3f5994f6bf8a9db5 100644 --- a/src/Compilers/VisualBasic/Portable/Binding/Binder_Lookup.vb +++ b/src/Compilers/VisualBasic/Portable/Binding/Binder_Lookup.vb @@ -1995,6 +1995,10 @@ ExitForFor: typeParameter As TypeParameterSymbol, options As LookupOptions, binder As Binder) + If typeParameter.TypeParameterKind = TypeParameterKind.Cref Then + Return + End If + AddLookupSymbolsInfoInTypeParameterNoExtensionMethods(nameSet, typeParameter, options, binder) ' Search for extension methods. diff --git a/src/Compilers/VisualBasic/Test/Symbol/DocumentationComments/DocCommentTests.vb b/src/Compilers/VisualBasic/Test/Symbol/DocumentationComments/DocCommentTests.vb index f1ea771a0d20817a56eb5fe2a7cbaa35ee333c3e..80f478126559dc6902aae8a26f4c34da9e0d7e2c 100644 --- a/src/Compilers/VisualBasic/Test/Symbol/DocumentationComments/DocCommentTests.vb +++ b/src/Compilers/VisualBasic/Test/Symbol/DocumentationComments/DocCommentTests.vb @@ -12410,5 +12410,40 @@ DashDash stringMapper:=Function(o) StringReplace(o, System.IO.Path.Combine(TestHelpers.AsXmlCommentText(path), "- - -.xml"), "**FILE**"), ensureEnglishUICulture:=True) End Sub + + + Public Sub LookupOnCrefTypeParameter() + + Dim sources = + + + + ''' + ''' + Public Sub S() + End Sub +End Class +]]> + + + + Dim compilation = CreateCompilationWithMscorlibAndVBRuntime( + sources, + options:=TestOptions.ReleaseDll) + + + Dim tree = compilation.SyntaxTrees(0) + Dim model = compilation.GetSemanticModel(tree) + + Dim name = FindNodesOfTypeFromText(Of NameSyntax)(tree, "U").Single() + Dim typeParameter = DirectCast(model.GetSymbolInfo(name).Symbol, TypeParameterSymbol) + Assert.Empty(model.LookupSymbols(name.SpanStart, typeParameter, "GetAwaiter")) + End Sub + End Class End Namespace diff --git a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/SymbolCompletionProviderTests.cs b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/SymbolCompletionProviderTests.cs index 48e479cc99afed1c7aac5676ce9fe95825b678e5..ee88dc7c53fd333e2e23e7af7e1c71b9624ab48f 100644 --- a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/SymbolCompletionProviderTests.cs +++ b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/SymbolCompletionProviderTests.cs @@ -2058,6 +2058,48 @@ class Program await VerifyItemExistsAsync(markup, "CompareTo"); } + [WorkItem(21596, "https://github.com/dotnet/roslyn/issues/21596")] + [Fact, Trait(Traits.Feature, Traits.Features.Completion)] + public async Task AmbiguityBetweenExpressionAndLocalFunctionReturnType() + { + var markup = @" +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +class Program +{ + static void Main(string[] args) + { + AwaitTest test = new AwaitTest(); + test.Test1().Wait(); + } +} + +class AwaitTest +{ + List stringList = new List(); + + public async Task Test1() + { + stringList.$$ + + await Test2(); + + return true; + } + + public async Task Test2() + { + return true; + } +}"; + + await VerifyItemExistsAsync(markup, "Add"); + } + [WorkItem(540750, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/540750")] [Fact, Trait(Traits.Feature, Traits.Features.Completion)] public async Task CompletionAfterNewInScript() diff --git a/src/EditorFeatures/CSharpTest/EditAndContinue/CSharpEditAndContinueAnalyzerTests.cs b/src/EditorFeatures/CSharpTest/EditAndContinue/CSharpEditAndContinueAnalyzerTests.cs index 93eeac430f91411dc556adbab64e582614376c41..2d0faa1dfb214cba207af47a2f5b088491e03f69 100644 --- a/src/EditorFeatures/CSharpTest/EditAndContinue/CSharpEditAndContinueAnalyzerTests.cs +++ b/src/EditorFeatures/CSharpTest/EditAndContinue/CSharpEditAndContinueAnalyzerTests.cs @@ -161,7 +161,7 @@ public void ErrorSpans_TopLevel() } "; - TestSpans(source, kind => TopSyntaxComparer.HasLabel(kind, ignoreVariableDeclarations: false)); + TestSpans(source, kind => TopSyntaxComparer.HasLabel(kind)); } [Fact] @@ -227,7 +227,7 @@ void M() public void ErrorSpansAllKinds() { TestErrorSpansAllKinds(StatementSyntaxComparer.IgnoreLabeledChild); - TestErrorSpansAllKinds(kind => TopSyntaxComparer.HasLabel(kind, ignoreVariableDeclarations: false)); + TestErrorSpansAllKinds(kind => TopSyntaxComparer.HasLabel(kind)); } [Fact] diff --git a/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs b/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs index 8508aa6d4bec3670d3c91d1810b36c804afea60c..0b1f32a206c2884715c82a3a33b1dccc302c4b15 100644 --- a/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs +++ b/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs @@ -3210,6 +3210,28 @@ static void Main(string[] args) edits.VerifyRudeDiagnostics(); } + [Fact] + public void MethodUpdate_LocalFunctionsParameterRefnessInBody() + { + var src1 = @"class C { public void M(int a) { void f(ref int b) => b = 1; } }"; + var src2 = @"class C { public void M(int a) { void f(out int b) => b = 1; } } "; + + var edits = GetTopEdits(src1, src2); + edits.VerifyEdits( + "Update [public void M(int a) { void f(ref int b) => b = 1; }]@10 -> [public void M(int a) { void f(out int b) => b = 1; }]@10"); + } + + [Fact] + public void MethodUpdate_LambdaParameterRefnessInBody() + { + var src1 = @"class C { public void M(int a) { f((ref int b) => b = 1); } }"; + var src2 = @"class C { public void M(int a) { f((out int b) => b = 1); } } "; + + var edits = GetTopEdits(src1, src2); + edits.VerifyEdits( + "Update [public void M(int a) { f((ref int b) => b = 1); }]@10 -> [public void M(int a) { f((out int b) => b = 1); }]@10"); + } + #endregion #region Operators diff --git a/src/EditorFeatures/CSharpTest/QuickInfo/SemanticQuickInfoSourceTests.cs b/src/EditorFeatures/CSharpTest/QuickInfo/SemanticQuickInfoSourceTests.cs index 781275e92b2c61e7ce4a428e5d04f2ac6ffe1936..cf4965cb04230e9224c07889579ddb2940f5bbbb 100644 --- a/src/EditorFeatures/CSharpTest/QuickInfo/SemanticQuickInfoSourceTests.cs +++ b/src/EditorFeatures/CSharpTest/QuickInfo/SemanticQuickInfoSourceTests.cs @@ -4992,5 +4992,28 @@ static void Main(string[] args) }", MainDescription($"({FeaturesResources.local_variable}) ref int i")); } + + [Fact, Trait(Traits.Feature, Traits.Features.QuickInfo)] + [WorkItem(410932, "https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems?id=410932")] + public async Task TestGenericMethodInDocComment() + { + await TestAsync( +@" +class Test +{ + T F() + { + F(); + } + + /// + /// + /// + void S() + { } +} +", + MainDescription("T Test.F()")); + } } } diff --git a/src/EditorFeatures/VisualBasicTest/QuickInfo/SemanticQuickInfoSourceTests.vb b/src/EditorFeatures/VisualBasicTest/QuickInfo/SemanticQuickInfoSourceTests.vb index 0a79649f0f19595bd1c37ae5785fdbca4a17242b..5343154a1226cb428871b8a7fdb0f91b77fc9392 100644 --- a/src/EditorFeatures/VisualBasicTest/QuickInfo/SemanticQuickInfoSourceTests.vb +++ b/src/EditorFeatures/VisualBasicTest/QuickInfo/SemanticQuickInfoSourceTests.vb @@ -2089,5 +2089,25 @@ End Class ", Documentation("String http://microsoft.com Nothing cat")) End Function + + + + Public Async Function TestGenericMethodInDocComment() As Task + Await TestWithImportsAsync( + ''' + ''' + Public Sub S() + End Sub +End Class + ]]>.NormalizedValue, + MainDescription("Function Test.F(Of T)() As T")) + End Function + End Class End Namespace diff --git a/src/Features/CSharp/Portable/EditAndContinue/TopSyntaxComparer.cs b/src/Features/CSharp/Portable/EditAndContinue/TopSyntaxComparer.cs index f0354817cebf528b0a81a426735b1d79d3be14d4..502464db7007cdfcb8383c2ef3dfa90d5e80610a 100644 --- a/src/Features/CSharp/Portable/EditAndContinue/TopSyntaxComparer.cs +++ b/src/Features/CSharp/Portable/EditAndContinue/TopSyntaxComparer.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Diagnostics; using Microsoft.CodeAnalysis.CSharp.Syntax; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.EditAndContinue { @@ -60,7 +61,7 @@ private static bool HasChildren(SyntaxNode node) { // Leaves are labeled statements that don't have a labeled child. // We also return true for non-labeled statements. - Label label = Classify(node.Kind(), out var isLeaf, ignoreVariableDeclarations: false); + Label label = Classify(node.Kind(), out var isLeaf); // ignored should always be reported as leaves Debug.Assert(label != Label.Ignored || isLeaf); @@ -160,7 +161,7 @@ private static int TiedToAncestor(Label label) } // internal for testing - internal static Label Classify(SyntaxKind kind, out bool isLeaf, bool ignoreVariableDeclarations) + internal static Label Classify(SyntaxKind kind, out bool isLeaf) { switch (kind) { @@ -205,12 +206,12 @@ internal static Label Classify(SyntaxKind kind, out bool isLeaf, bool ignoreVari return Label.FieldDeclaration; case SyntaxKind.VariableDeclaration: - isLeaf = ignoreVariableDeclarations; - return ignoreVariableDeclarations ? Label.Ignored : Label.FieldVariableDeclaration; + isLeaf = false; + return Label.FieldVariableDeclaration; case SyntaxKind.VariableDeclarator: isLeaf = true; - return ignoreVariableDeclarations ? Label.Ignored : Label.FieldVariableDeclarator; + return Label.FieldVariableDeclarator; case SyntaxKind.MethodDeclaration: isLeaf = false; @@ -306,13 +307,13 @@ protected internal override int GetLabel(SyntaxNode node) internal static Label GetLabel(SyntaxKind kind) { - return Classify(kind, out var isLeaf, ignoreVariableDeclarations: false); + return Classify(kind, out var isLeaf); } // internal for testing - internal static bool HasLabel(SyntaxKind kind, bool ignoreVariableDeclarations) + internal static bool HasLabel(SyntaxKind kind) { - return Classify(kind, out var isLeaf, ignoreVariableDeclarations) != Label.Ignored; + return Classify(kind, out var isLeaf) != Label.Ignored; } protected internal override int LabelCount @@ -346,13 +347,22 @@ public override bool ValuesEqual(SyntaxNode left, SyntaxNode right) case SyntaxKind.RemoveAccessorDeclaration: // When comparing method bodies we need to NOT ignore VariableDeclaration and VariableDeclarator children, // but when comparing field definitions we should ignore VariableDeclarations children. - ignoreChildFunction = childKind => HasLabel(childKind, ignoreVariableDeclarations: true); + + var leftBody = GetBody(left); + var rightBody = GetBody(right); + + if (!SyntaxFactory.AreEquivalent(leftBody, rightBody, null)) + { + return false; + } + + ignoreChildFunction = childKind => childKind == SyntaxKind.Block || childKind == SyntaxKind.ArrowExpressionClause || HasLabel(childKind); break; default: if (HasChildren(left)) { - ignoreChildFunction = childKind => HasLabel(childKind, ignoreVariableDeclarations: false); + ignoreChildFunction = childKind => HasLabel(childKind); } else { @@ -365,6 +375,16 @@ public override bool ValuesEqual(SyntaxNode left, SyntaxNode right) return SyntaxFactory.AreEquivalent(left, right, ignoreChildFunction); } + private static SyntaxNode GetBody(SyntaxNode node) + { + switch (node) + { + case BaseMethodDeclarationSyntax baseMethodDeclarationSyntax: return baseMethodDeclarationSyntax.Body ?? (SyntaxNode)baseMethodDeclarationSyntax.ExpressionBody?.Expression; + case AccessorDeclarationSyntax accessorDeclarationSyntax: return accessorDeclarationSyntax.Body ?? (SyntaxNode)accessorDeclarationSyntax.ExpressionBody?.Expression; + default: throw ExceptionUtilities.UnexpectedValue(node); + } + } + protected override bool TryComputeWeightedDistance(SyntaxNode leftNode, SyntaxNode rightNode, out double distance) { SyntaxNodeOrToken? leftName = TryGetName(leftNode); diff --git a/src/Features/Core/Portable/AddImport/AddImportFixData.cs b/src/Features/Core/Portable/AddImport/AddImportFixData.cs index a3def502798c4e7f7161ad87939f5f024699146e..d3f16b9dea5a86c83769703776930b8c942e1d85 100644 --- a/src/Features/Core/Portable/AddImport/AddImportFixData.cs +++ b/src/Features/Core/Portable/AddImport/AddImportFixData.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis.CodeActions; @@ -19,7 +20,7 @@ internal class AddImportFixData /// May be empty for fixes that don't need to add an import and only do something like /// add a project/metadata reference. /// - public ImmutableArray TextChanges { get; } + public IList TextChanges { get; } /// /// String to display in the lightbulb menu. @@ -29,7 +30,7 @@ internal class AddImportFixData /// /// Tags that control what glyph is displayed in the lightbulb menu. /// - public ImmutableArray Tags { get; private set; } + public IList Tags { get; private set; } /// /// The priority this item should have in the lightbulb list. diff --git a/src/Features/Core/Portable/AddImport/CodeActions/AddImportCodeAction.cs b/src/Features/Core/Portable/AddImport/CodeActions/AddImportCodeAction.cs index a71cd6e1531a276271ba46f6194a118e28779b72..42636d2194259c3b6d176129328434ae09babc13 100644 --- a/src/Features/Core/Portable/AddImport/CodeActions/AddImportCodeAction.cs +++ b/src/Features/Core/Portable/AddImport/CodeActions/AddImportCodeAction.cs @@ -5,6 +5,7 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.Text; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.AddImport { @@ -44,9 +45,9 @@ private abstract class AddImportCodeAction : CodeAction FixData = fixData; Title = fixData.Title; - Tags = fixData.Tags; + Tags = fixData.Tags.ToImmutableArrayOrEmpty(); Priority = fixData.Priority; - _textChanges = fixData.TextChanges; + _textChanges = fixData.TextChanges.ToImmutableArrayOrEmpty(); } protected async Task GetUpdatedDocumentAsync(CancellationToken cancellationToken) diff --git a/src/VisualStudio/Core/Next/Remote/ServiceHubRemoteHostClient.cs b/src/VisualStudio/Core/Next/Remote/ServiceHubRemoteHostClient.cs index 2bc0a576faf0f4f72e4014d60d1c5c67ca0a2f64..3e0182fd2cd94841c81d238abfe0b5675cf5b753 100644 --- a/src/VisualStudio/Core/Next/Remote/ServiceHubRemoteHostClient.cs +++ b/src/VisualStudio/Core/Next/Remote/ServiceHubRemoteHostClient.cs @@ -66,7 +66,8 @@ internal sealed partial class ServiceHubRemoteHostClient : RemoteHostClient } } - private static async Task RegisterWorkspaceHostAsync(Workspace workspace, RemoteHostClient client) + // internal for debugging purpose + internal static async Task RegisterWorkspaceHostAsync(Workspace workspace, RemoteHostClient client) { var vsWorkspace = workspace as VisualStudioWorkspaceImpl; if (vsWorkspace == null) diff --git a/src/VisualStudio/RemoteHostClientMock/Remote/RemoteHostClientFactory.cs b/src/VisualStudio/RemoteHostClientMock/Remote/RemoteHostClientFactory.cs index aae957db8581a95db9014d6ba0a178ee1905de12..748d9fc44b56b66ba8af2c8e1fd3ac1109366f3b 100644 --- a/src/VisualStudio/RemoteHostClientMock/Remote/RemoteHostClientFactory.cs +++ b/src/VisualStudio/RemoteHostClientMock/Remote/RemoteHostClientFactory.cs @@ -19,7 +19,12 @@ public async Task CreateAsync(Workspace workspace, Cancellatio // this is the point where we can create different kind of remote host client in future (cloud or etc) if (workspace.Options.GetOption(RemoteHostClientFactoryOptions.RemoteHost_InProc)) { - return await InProcRemoteHostClient.CreateAsync(workspace, runCacheCleanup: true, cancellationToken: cancellationToken).ConfigureAwait(false); + var client = await InProcRemoteHostClient.CreateAsync(workspace, runCacheCleanup: true, cancellationToken: cancellationToken).ConfigureAwait(false); + + // register workspace host for in proc remote host client + await ServiceHubRemoteHostClient.RegisterWorkspaceHostAsync(workspace, client).ConfigureAwait(false); + + return client; } return await ServiceHubRemoteHostClient.CreateAsync(workspace, cancellationToken).ConfigureAwait(false); diff --git a/src/Workspaces/CSharp/Portable/Recommendations/CSharpRecommendationService.cs b/src/Workspaces/CSharp/Portable/Recommendations/CSharpRecommendationService.cs index c78dbb56db41ac161b9ac08689b56dddf5a41e20..edb5514108e27700285b6dc02d7ca9354bdaddb4 100644 --- a/src/Workspaces/CSharp/Portable/Recommendations/CSharpRecommendationService.cs +++ b/src/Workspaces/CSharp/Portable/Recommendations/CSharpRecommendationService.cs @@ -296,13 +296,21 @@ private static ImmutableArray GetSymbolsForNamespaceDeclarationNameCont // int i = 5; // i. // <-- here // List ml = new List(); + // + // The problem is that "i.List" gets parsed as a type. In this case we need + // to try binding again as if "i" is an expression and not a type. In order to do + // that, we need to speculate as to what 'i' meant if it wasn't part of a local + // declaration's type. + // + // Another interesting case is something like: + // + // stringList. + // await Test2(); + // + // Here "stringList.await" is thought of as the return type of a local function. - // The problem is that "i.List" gets parsed as a type. In this case we need to - // try binding again as if "i" is an expression and not a type. In order to do that, we - // need to speculate as to what 'i' meant if it wasn't part of a local declaration's - // type. - - if (name.IsFoundUnder(d => d.Declaration.Type) || + if (name.IsFoundUnder(d => d.ReturnType) || + name.IsFoundUnder(d => d.Declaration.Type) || name.IsFoundUnder(d => d.Declaration.Type)) { var speculativeBinding = context.SemanticModel.GetSpeculativeSymbolInfo( diff --git a/src/Workspaces/Core/Desktop/Workspace/SQLite/Interop/OpenFlags.cs b/src/Workspaces/Core/Desktop/Workspace/SQLite/Interop/OpenFlags.cs index 8262d65d78ab4b1658203bb0f894f118e2fee724..4504758846d432525486049b6793eb7e943aee87 100644 --- a/src/Workspaces/Core/Desktop/Workspace/SQLite/Interop/OpenFlags.cs +++ b/src/Workspaces/Core/Desktop/Workspace/SQLite/Interop/OpenFlags.cs @@ -24,7 +24,7 @@ internal enum OpenFlags // SQLITE_OPEN_MASTER_JOURNAL = 0x00004000, /* VFS only */ // SQLITE_OPEN_NOMUTEX = 0x00008000, /* Ok for sqlite3_open_v2() */ // SQLITE_OPEN_FULLMUTEX = 0x00010000, /* Ok for sqlite3_open_v2() */ - // SQLITE_OPEN_SHAREDCACHE = 0x00020000, /* Ok for sqlite3_open_v2() */ + SQLITE_OPEN_SHAREDCACHE = 0x00020000, /* Ok for sqlite3_open_v2() */ // SQLITE_OPEN_PRIVATECACHE = 0x00040000, /* Ok for sqlite3_open_v2() */ // SQLITE_OPEN_WAL = 0x00080000, /* VFS only */ } diff --git a/src/Workspaces/Core/Desktop/Workspace/SQLite/Interop/SqlConnection.cs b/src/Workspaces/Core/Desktop/Workspace/SQLite/Interop/SqlConnection.cs index 20f8ca94933703aa5c969e35bf7af152449929f1..3dd5f22710bb0d6732922ffa365c2055988052cc 100644 --- a/src/Workspaces/Core/Desktop/Workspace/SQLite/Interop/SqlConnection.cs +++ b/src/Workspaces/Core/Desktop/Workspace/SQLite/Interop/SqlConnection.cs @@ -51,7 +51,9 @@ public static SqlConnection Create(IPersistentStorageFaultInjector faultInjector { faultInjector?.OnNewConnection(); - var flags = OpenFlags.SQLITE_OPEN_CREATE | OpenFlags.SQLITE_OPEN_READWRITE; + // Enable shared cache so that multiple connections inside of same process share cache + // see https://sqlite.org/threadsafe.html for more detail + var flags = OpenFlags.SQLITE_OPEN_CREATE | OpenFlags.SQLITE_OPEN_READWRITE | OpenFlags.SQLITE_OPEN_SHAREDCACHE; var result = (Result)raw.sqlite3_open_v2(databasePath, out var handle, (int)flags, vfs: null); if (result != Result.OK) diff --git a/src/Workspaces/Core/Desktop/Workspace/SQLite/SQLitePersistentStorage.cs b/src/Workspaces/Core/Desktop/Workspace/SQLite/SQLitePersistentStorage.cs index 0b1472d5f8b0065738b8b8ac7318e83fe1211337..03f0754f5bee2b42771405d1e65e66f98d82c02e 100644 --- a/src/Workspaces/Core/Desktop/Workspace/SQLite/SQLitePersistentStorage.cs +++ b/src/Workspaces/Core/Desktop/Workspace/SQLite/SQLitePersistentStorage.cs @@ -122,6 +122,7 @@ static SQLitePersistentStorage() private readonly CancellationTokenSource _shutdownTokenSource = new CancellationTokenSource(); + private readonly IDisposable _dbOwnershipLock; private readonly IPersistentStorageFaultInjector _faultInjectorOpt; // Accessors that allow us to retrieve/store data into specific DB tables. The @@ -146,10 +147,13 @@ static SQLitePersistentStorage() string solutionFilePath, string databaseFile, Action disposer, + IDisposable dbOwnershipLock, IPersistentStorageFaultInjector faultInjectorOpt) : base(optionService, workingFolderPath, solutionFilePath, databaseFile, disposer) { + _dbOwnershipLock = dbOwnershipLock; _faultInjectorOpt = faultInjectorOpt; + _solutionAccessor = new SolutionAccessor(this); _projectAccessor = new ProjectAccessor(this); _documentAccessor = new DocumentAccessor(this); @@ -187,6 +191,21 @@ private void ReleaseConnection(SqlConnection connection) } public override void Close() + { + // Flush all pending writes so that all data our features wanted written + // are definitely persisted to the DB. + try + { + CloseWorker(); + } + finally + { + // let the lock go + _dbOwnershipLock.Dispose(); + } + } + + private void CloseWorker() { // Flush all pending writes so that all data our features wanted written // are definitely persisted to the DB. diff --git a/src/Workspaces/Core/Desktop/Workspace/SQLite/SQLitePersistentStorageService.cs b/src/Workspaces/Core/Desktop/Workspace/SQLite/SQLitePersistentStorageService.cs index eb89982577d6b8364675042d27cb86a308b1ed94..39f0ef95df647f23da10c76b77a5cbe34a0b92d3 100644 --- a/src/Workspaces/Core/Desktop/Workspace/SQLite/SQLitePersistentStorageService.cs +++ b/src/Workspaces/Core/Desktop/Workspace/SQLite/SQLitePersistentStorageService.cs @@ -4,6 +4,7 @@ using System.IO; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Options; +using Microsoft.CodeAnalysis.Shared.Utilities; using Microsoft.CodeAnalysis.SolutionSize; using Microsoft.CodeAnalysis.Storage; using Roslyn.Utilities; @@ -12,6 +13,7 @@ namespace Microsoft.CodeAnalysis.SQLite { internal partial class SQLitePersistentStorageService : AbstractPersistentStorageService { + private const string LockFile = "db.lock"; private const string StorageExtension = "sqlite3"; private const string PersistentStorageFileName = "storage.ide"; @@ -24,7 +26,7 @@ internal partial class SQLitePersistentStorageService : AbstractPersistentStorag { } - public SQLitePersistentStorageService(IOptionService optionService, IPersistentStorageFaultInjector faultInjector) + public SQLitePersistentStorageService(IOptionService optionService, IPersistentStorageFaultInjector faultInjector) : base(optionService, testing: true) { _faultInjectorOpt = faultInjector; @@ -36,9 +38,47 @@ protected override string GetDatabaseFilePath(string workingFolderPath) return Path.Combine(workingFolderPath, StorageExtension, PersistentStorageFileName); } - protected override AbstractPersistentStorage OpenDatabase(Solution solution, string workingFolderPath, string databaseFilePath) - => new SQLitePersistentStorage( - OptionService, workingFolderPath, solution.FilePath, databaseFilePath, this.Release, _faultInjectorOpt); + protected override bool TryOpenDatabase( + Solution solution, string workingFolderPath, string databaseFilePath, out AbstractPersistentStorage storage) + { + storage = null; + + // try to get db ownership lock. if someone else already has the lock. it will throw + var dbOwnershipLock = TryGetDatabaseOwnership(databaseFilePath); + if (dbOwnershipLock == null) + { + return false; + } + + storage = new SQLitePersistentStorage( + OptionService, workingFolderPath, solution.FilePath, databaseFilePath, this.Release, dbOwnershipLock, _faultInjectorOpt); + + return true; + } + + private static IDisposable TryGetDatabaseOwnership(string databaseFilePath) + { + return IOUtilities.PerformIO(() => + { + // make sure directory exist first. + EnsureDirectory(databaseFilePath); + + return File.Open( + Path.Combine(Path.GetDirectoryName(databaseFilePath), LockFile), + FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None); + }); + } + + private static void EnsureDirectory(string databaseFilePath) + { + var directory = Path.GetDirectoryName(databaseFilePath); + if (Directory.Exists(directory)) + { + return; + } + + Directory.CreateDirectory(directory); + } protected override bool ShouldDeleteDatabase(Exception exception) { diff --git a/src/Workspaces/Core/Desktop/Workspace/Storage/PersistentStorageService.cs b/src/Workspaces/Core/Desktop/Workspace/Storage/PersistentStorageService.cs index 6ed03e29f7c0bcab9161de3d128249ece89c7d79..2a9a9d5f98a5fe858ff64fa8ac6d0e99417660f5 100644 --- a/src/Workspaces/Core/Desktop/Workspace/Storage/PersistentStorageService.cs +++ b/src/Workspaces/Core/Desktop/Workspace/Storage/PersistentStorageService.cs @@ -54,7 +54,7 @@ protected AbstractPersistentStorageService(IOptionService optionService, bool te } protected abstract string GetDatabaseFilePath(string workingFolderPath); - protected abstract AbstractPersistentStorage OpenDatabase(Solution solution, string workingFolderPath, string databaseFilePath); + protected abstract bool TryOpenDatabase(Solution solution, string workingFolderPath, string databaseFilePath, out AbstractPersistentStorage storage); protected abstract bool ShouldDeleteDatabase(Exception exception); public IPersistentStorage GetStorage(Solution solution) @@ -235,7 +235,11 @@ private AbstractPersistentStorage TryCreatePersistentStorage(Solution solution, var databaseFilePath = GetDatabaseFilePath(workingFolderPath); try { - database = OpenDatabase(solution, workingFolderPath, databaseFilePath); + if (!TryOpenDatabase(solution, workingFolderPath, databaseFilePath, out database)) + { + return false; + } + database.Initialize(solution); persistentStorage = database; diff --git a/src/Workspaces/Remote/Core/Storage/RemotePersistentStorageLocationService.cs b/src/Workspaces/Remote/Core/Storage/RemotePersistentStorageLocationService.cs index 60558219e19d4e8904cb54bd12d2c378a5d203e2..c646f22d1bddfc2c9c27cb7e2f8fcb08f284f5ad 100644 --- a/src/Workspaces/Remote/Core/Storage/RemotePersistentStorageLocationService.cs +++ b/src/Workspaces/Remote/Core/Storage/RemotePersistentStorageLocationService.cs @@ -44,7 +44,8 @@ public static void UpdateStorageLocation(SolutionId id, string storageLocation) } else { - _idToStorageLocation[id] = storageLocation; + // Store the esent database in a different location for the out of proc server. + _idToStorageLocation[id] = Path.Combine(storageLocation, "Server"); } } } diff --git a/src/Workspaces/Remote/ServiceHub/Shared/RoslynJsonConverter.RoslynOnly.cs b/src/Workspaces/Remote/ServiceHub/Shared/RoslynJsonConverter.RoslynOnly.cs index 61e69a79a845436d6fd3f6d2854c5148a6a25502..d1ec1ec46946f2f04cab4305019405f843aa33fb 100644 --- a/src/Workspaces/Remote/ServiceHub/Shared/RoslynJsonConverter.RoslynOnly.cs +++ b/src/Workspaces/Remote/ServiceHub/Shared/RoslynJsonConverter.RoslynOnly.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis.AddImport; @@ -165,7 +166,7 @@ protected override PackageWithTypeResult ReadValue(JsonReader reader, JsonSerial var typeName = ReadProperty(reader); var version = ReadProperty(reader); var rank = (int)ReadProperty(reader); - var containingNamespaceNames = ReadProperty>(serializer, reader); + var containingNamespaceNames = ReadProperty>(serializer, reader); Contract.ThrowIfFalse(reader.Read()); Contract.ThrowIfFalse(reader.TokenType == JsonToken.EndObject); @@ -237,7 +238,7 @@ protected override ReferenceAssemblyWithTypeResult ReadValue(JsonReader reader, var assemblyName = ReadProperty(reader); var typeName = ReadProperty(reader); - var containingNamespaceNames = ReadProperty>(serializer, reader); + var containingNamespaceNames = ReadProperty>(serializer, reader); Contract.ThrowIfFalse(reader.Read()); Contract.ThrowIfFalse(reader.TokenType == JsonToken.EndObject); @@ -298,9 +299,9 @@ protected override AddImportFixData ReadValue(JsonReader reader, JsonSerializer Contract.ThrowIfFalse(reader.TokenType == JsonToken.StartObject); var kind = (AddImportFixKind)ReadProperty(reader); - var textChanges = ReadProperty>(serializer, reader); + var textChanges = ReadProperty>(serializer, reader).ToImmutableArrayOrEmpty(); var title = ReadProperty(reader); - var tags = ReadProperty>(serializer, reader); + var tags = ReadProperty>(serializer, reader).ToImmutableArrayOrEmpty(); var priority = (CodeActionPriority)ReadProperty(reader); var projectReferenceToAdd = ReadProperty(serializer, reader); @@ -344,13 +345,13 @@ protected override void WriteValue(JsonWriter writer, AddImportFixData source, J writer.WriteValue((int)source.Kind); writer.WritePropertyName(nameof(AddImportFixData.TextChanges)); - serializer.Serialize(writer, source.TextChanges); + serializer.Serialize(writer, source.TextChanges ?? SpecializedCollections.EmptyList()); writer.WritePropertyName(nameof(AddImportFixData.Title)); writer.WriteValue(source.Title); writer.WritePropertyName(nameof(AddImportFixData.Tags)); - serializer.Serialize(writer, source.Tags.NullToEmpty()); + serializer.Serialize(writer, source.Tags ?? SpecializedCollections.EmptyList()); writer.WritePropertyName(nameof(AddImportFixData.Priority)); writer.WriteValue((int)source.Priority);