Async main codegen #18472

TyOverby · 2017-04-05T18:02:45Z

No description provided.

TyOverby · 2017-04-06T02:28:52Z

@dotnet/roslyn-compiler

This bit should be ready for a general review.

I've added some pretty minimal tests that I plan on fleshing out in another PR. #Resolved

jaredpar · 2017-04-06T06:20:50Z

src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs

@@ -221,7 +241,10 @@ private static MethodSymbol GetEntryPoint(CSharpCompilation compilation, PEModul
                moduleBeingBuilt.SetMethodBody(synthesizedEntryPoint, emittedBody);
            }

-            Debug.Assert((object)entryPoint != null || entryPointAndDiagnostics.Diagnostics.HasAnyErrors() || !compilation.Options.Errors.IsDefaultOrEmpty);
+            if (addedDefinition && moduleBeingBuilt != null) {


Braces on new lines

jaredpar · 2017-04-06T06:21:33Z

src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs

@@ -196,8 +197,27 @@ private static MethodSymbol GetEntryPoint(CSharpCompilation compilation, PEModul
            diagnostics.AddRange(entryPointAndDiagnostics.Diagnostics);

            var entryPoint = entryPointAndDiagnostics.MethodSymbol;
-            var synthesizedEntryPoint = entryPoint as SynthesizedEntryPointSymbol;
-            if (((object)synthesizedEntryPoint != null) &&
+


We should add a comment to the top of the function noting that this returns the real entry point vs. the user defined one. The name of the function is ambiguous with this feature now.

jaredpar · 2017-04-06T06:22:23Z

src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs

+            bool addedDefinition = false;
+            if (entryPoint is SynthesizedEntryPointSymbol s)
+            {
+                synthesizedEntryPoint = s;


What case hits this branch of the if?

The other two other subclasses of SynthesizedEntryPointSymbol are ScriptEntryPoint and SubmissionEntryPoint both of which (I think) are for scripting support.

jaredpar · 2017-04-06T06:23:55Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+
+            // Task -> Void
+            // Task<_> -> Int32
+            private static TypeSymbol TranslateReturnType(CSharpCompilation compilation, TypeSymbol returnType) {


Braces on new lines. Other occurences in this review that should be fixed.

jaredpar · 2017-04-06T06:24:06Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+                }
+            }
+
+


Double blank line.

jaredpar · 2017-04-06T06:24:52Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+                _paramKind = userMain.ParameterCount == 0 ? ParamKind.NoArgs : ParamKind.StringArrayArgs;
+                _returnKind = GetReturnKind(compilation, userMain.ReturnType);
+
+


Double blank line.

jaredpar · 2017-04-06T06:25:19Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+                        type: _userMain.ReturnType)
+                { WasCompilerGenerated = true };
+
+


Double blank line

Still see this in latest source.

Still see this in the latest source.

@TyOverby still present

Actually Done. I think I fixed another instance of this problem earlier...

Clearly I need to update the auto-formatter in order to accommodate this

jaredpar · 2017-04-06T17:24:03Z

src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs

+                addedDefinition = true;
+            }
+
+            if ((synthesizedEntryPoint != null) &&


Need to use the ((object)synthesizedEntryPoint) != null style of comparison here.

jaredpar · 2017-04-06T17:28:39Z

src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs

+            }
+            else if (entryPoint.HasAsyncMainReturnType(compilation) && compilation.LanguageVersion >= LanguageVersion.CSharp7_1)
+            {
+                synthesizedEntryPoint = new AsyncForwardEntryPoint(compilation, diagnostics, entryPoint.ContainingType, entryPoint);


It seems odd that we are attaching the diagnostics to only the async sythesized method symbol. What is the rationale for this?

In the case the diagnostics parameter had existing diagnostics in it, possible for a bag, it would be attaching those errors to the generated main as well which seems incorrect. As say using entryPointAndDiagnostics.Diagnostics. Even that though feels like we are double tracking diagnostics here.

Right now the only place that the diagnosticsBag is used is during the GetRequiredMethod lookup for GetAwaiter and GetResult.

jaredpar · 2017-04-06T17:29:34Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+            // Task<_> -> Int32
+            private static TypeSymbol TranslateReturnType(CSharpCompilation compilation, TypeSymbol returnType)
+            {
+                if (returnType.IsGenericTaskType(compilation)) {


Nit: more code formatting issues all through this file. Please review the entire PR and correct these.

jaredpar · 2017-04-06T17:30:09Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+            // The user-defined asyncrhonous main method.
+            private MethodSymbol _userMain;
+            private DiagnosticBag _diagnosticBag;
+            private CSharpCompilation _compilation;


Why is this stored as a field? It seems to only be used in the constructor of the type.

You're right, this reference must have survived some refactors.

jaredpar · 2017-04-06T17:30:30Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+        internal sealed class AsyncForwardEntryPoint : SynthesizedEntryPointSymbol
+        {
+            // The user-defined asyncrhonous main method.
+            private MethodSymbol _userMain;


jaredpar · 2017-04-06T17:30:49Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+            private CSharpCompilation _compilation;
+
+            private ReturnKind _returnKind;
+            private ParamKind _paramKind;


both of these can be readonly

jaredpar · 2017-04-06T17:31:04Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+            private ReturnKind _returnKind;
+            private ParamKind _paramKind;
+
+            private ImmutableArray<ParameterSymbol> _parameters;


jaredpar · 2017-04-06T17:32:23Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+                var syntax = this.GetSyntax();
+
+                var refKinds = Parameters.Length == 0  ? ImmutableArray<RefKind>.Empty : ImmutableArray.Create(RefKind.None);
+                var arguments = new List<BoundExpression>();


This can be optimized by using new List<BoundExpression>(capacity: Parameters.Length)

jaredpar · 2017-04-06T17:33:31Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+                        receiverOpt: null,
+                        method: _userMain,
+                        arguments: arguments.ToImmutableArray(),
+                        argumentNamesOpt: ImmutableArray<string>.Empty,


Is there a reason we aren't replicated the names on the user authored Main method? True it's legal to generate source without parameter names but some tools still don't handle it gracefully.

Ugh fine :P

I think that you meant for this comment to be on another line though. Our main method parameter names is defined in the call to SynthesizedParameterSymbol.Create inside the constructor.

Ah you're correct. The use of ImmutableArray<string>.Empty is correct in this location.

jaredpar · 2017-04-06T17:34:28Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+                        type: _userMain.ReturnType)
+                { WasCompilerGenerated = true };
+
+                // PROTOTYPE(async-main): Errors should likely be reported here instead of bailing.


Agree. Likely we can just move this up into the creation of the type. This would also avoid the need to store the bag on the type.

cston · 2017-04-06T17:42:06Z

src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs

+
+            SynthesizedEntryPointSymbol synthesizedEntryPoint = null;
+            bool addedDefinition = false;
+            if (entryPoint is SynthesizedEntryPointSymbol s)


is SynthesizedEntryPointSymbol s followed by synthesizedEntryPoint = s; seems more convoluted (to me at least) than:

var synthesizedEntryPoint = entryPoint as SynthesizedEntryPointSymbol; if ((object)synthesizedEntryPoint == null) { // other cases }

cston · 2017-04-06T17:44:19Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+                else
+                {
+                    return compilation.GetSpecialType(SpecialType.System_Void);
+                }


Consider using conditional operator here and in method below:

return returnType.IsGenericTaskType(compilation) ? ... : ...;

What's the style formatting for breaking this across multiple lines?

I'm partial to

return returnType.IsGenericTaskType(compilation) ? compilation.GetSpecialType(SpecialType.System_Int32) : compilation.GetSpecialType(SpecialType.System_Void);

cston · 2017-04-06T17:46:35Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+                foreach (var parameterSymbol in Parameters)
+                {
+                    arguments.Add(new BoundParameter(syntax, parameterSymbol, parameterSymbol.Type));
+                }


var arguments = Parameters.SelectAsArray((p, s) => new BoundParameter(s, p, p.Type), syntax);

I actually had this code originally but removed it because I thought that LINQ was more-or-less banned in the compiler

cston · 2017-04-06T17:54:46Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+
+            internal AsyncForwardEntryPoint(CSharpCompilation compilation, DiagnosticBag diagnosticBag, NamedTypeSymbol containingType, MethodSymbol userMain): 
+                base(containingType, TranslateReturnType(compilation, userMain.ReturnType)) {
+                Debug.Assert(userMain != null);


Debug.Assert seems unnecessary since userMain was dereferenced in base call, and also below. (NullReferenceException seems sufficient.)

Makes sense

cston · 2017-04-06T18:09:31Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+            {
+                var syntax = this.GetSyntax();
+
+                var refKinds = Parameters.Length == 0  ? ImmutableArray<RefKind>.Empty : ImmutableArray.Create(RefKind.None);


It seems strange to assume there is exactly zero or one parameter for refKinds but not for arguments below. Consider generalizing this. Perhaps: var refKinds = Parameters.SelectAsArray(p => RefKind.None);

Better yet, argumentRefKindsOpt is an optional parameter so we can pass in default(ImmutableArray<RefKind>) instead.

cston · 2017-04-06T18:11:18Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+                        isDelegateCall: false,
+                        expanded: false,
+                        invokedAsExtensionMethod: false,
+                        argsToParamsOpt: ImmutableArray<int>.Empty,


I believe argumentNamesOpt and argsToParamsOpt should be default(ImmutableArray<T>) rather than Empty if not set.

jaredpar · 2017-04-07T18:24:45Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

@@ -334,14 +334,13 @@ private static BoundCall CreateParameterlessCall(CSharpSyntaxNode syntax, BoundE
        internal sealed class AsyncForwardEntryPoint : SynthesizedEntryPointSymbol
        {
            // The user-defined asyncrhonous main method.
-            private MethodSymbol _userMain;
+            private readonly MethodSymbol _userMain;
            private DiagnosticBag _diagnosticBag;


This can be readonly as well.

Have we figured out my earlier questionts about this bag in general? It seems like it's opening the door for duplicate / unnecessary errors here. Feel like they should be resolved at the point of creating this symbol, not later.

I've moved this bag to the constructor and provide the userMain location as the location for the diagnostic to go.

jaredpar · 2017-04-07T18:27:09Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

-                }
+                return returnType.IsGenericTaskType(compilation)
+                    ? compilation.GetSpecialType(SpecialType.System_Int32)
+                    : compilation.GetSpecialType(SpecialType.System_Void);


Is it possible for GetSpecialType to return null here or are we sufficiently protected by this point? If it can return null then we need to account for that as the return here is used in places where null isn't acceptable.

We should be protected at this point, as this code is only executed if an async-main entrypoint was found, which can't happen if GetSpecialType is broken.

jaredpar · 2017-04-07T22:44:41Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

        {
+            if ((object)location == null) {


Looks like your formatting is off here. I heard there was this neat tool that formatted the code you changed in a git commit 😉

Unfortunately, the user still needs to remember to use the tool...

jaredpar · 2017-04-07T22:46:21Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+                var userMainLocation = userMain.DeclaringSyntaxReferences.SingleOrDefault()?.GetLocation() ?? NoLocation.Singleton;
+
+                _getAwaiterMethod = GetRequiredMethod(_userMain.ReturnType, WellKnownMemberNames.GetAwaiter, diagnosticBag, userMainLocation);
+                _getResultMethod = GetRequiredMethod(_getAwaiterMethod.ReturnType, WellKnownMemberNames.GetResult, diagnosticBag);


When GetRequiredMethod fails does it return null or does it fake a symbol? Curious to know what happens if this fails and then CreateBody is called (if that is possible).

It returns null.

CreateBody is not called if there are any error diagnostics in the bag. (GetRequiredMethod adds error diagnostics if it can't find anything).

jaredpar · 2017-04-08T00:08:54Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

+                BoundCall getAwaiterGetResult =
+                    CreateParameterlessCall(
+                            syntax: syntax,
+                            method: _getResultMethod,


Can CreateBody be called if the constructor for AsyncForwardEntryPoint added an error to the diagnostic bag? If so this would end up passing a null value for _getResultMethod.

CreateBody is called from MethodCompiler.GetEntryPoint only if there are no error diagnostics.

TyOverby · 2017-04-08T00:13:33Z

@jaredpar: I think I've addressed everything that you've mentioned. #Resolved

cston · 2017-04-26T14:43:15Z

src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs

@@ -2059,28 +2059,31 @@ private AssemblySymbol GetForwardedToAssembly(string fullName, int arity, Diagno
            return null;
        }

-        internal static void CheckFeatureAvailability(SyntaxNode syntax, MessageID feature, DiagnosticBag diagnostics, Location locationOpt = null)
+        internal static bool CheckFeatureAvailability(SyntaxNode syntax, MessageID feature, DiagnosticBag diagnostics, Location locationOpt = null)


It doesn't look like the return value is used.

Oh yep, not used anymore.

Fixed

…lity

AlekseyTs · 2017-04-26T16:50:55Z

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncMainTests.cs

+        }
+        [Fact]
+
+        public void MainCanReturnTaskAndGenericOnInt_NoAsync()


public void MainCanReturnTaskAndGenericOnInt_NoAsync() [](start = 8, length = 54)

There is an empty line after the [Fact] attribute.

AlekseyTs · 2017-04-26T16:52:21Z

src/Test/Utilities/CoreClr/CoreCLRRuntimeEnvironment.cs


-            if (expectedOutput.Trim() != actualOutput.Trim())
+            if (expectedOutput.Trim() != Output.Trim())


if (expectedOutput.Trim() != Output.Trim()) [](start = 12, length = 43)

Is this line safe for expectedOutput == null?

AlekseyTs · 2017-04-26T17:02:58Z

src/Compilers/CSharp/Portable/Symbols/MethodSymbolExtensions.cs

+
+        internal static CSharpSyntaxNode ExtractReturnTypeSyntax(this MethodSymbol method)
+        {
+            method = method.PartialDefinitionPart ?? method;


Please make sure there are tests for partial methods:

there is only definition

there is only implementation

definition is syntactically first

implementation is syntactically first

Result produced by this method should be observable in the tests, probably through location of an error.

Partial methods must be void returning, so they won't hit any of these new code paths.

As we discussed offline, let's add direct tests for this function then.

@AlekseyTs: I've added tests for this API on non-main methods.

AlekseyTs · 2017-04-26T17:05:11Z

Done with review pass (iteration 52)

cston · 2017-04-26T17:06:44Z

src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs

+
+
+            var syntax = method.ExtractReturnTypeSyntax();
+            var dumbInstance = new BoundLiteral(syntax, ConstantValue.Null, method.ReturnType);


BoundDefaultExpression(syntax, method.ReturnType)

Hmm, swore I fixed this earlier.

Done.

cston · 2017-04-26T17:07:35Z

src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs

+            if (method.RefKind != RefKind.None)
+            {
+                return false;
+            }


Duplicated case.

cston · 2017-04-26T17:19:14Z

LGTM

AlekseyTs · 2017-04-27T17:23:23Z

src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs

+            }
+
+            var syntax = method.ExtractReturnTypeSyntax();
+            var dumbInstance = new BoundLiteral(syntax, ConstantValue.Null, method.ReturnType);


var dumbInstance = new BoundLiteral(syntax, ConstantValue.Null, method.ReturnType); [](start = 12, length = 83)

Were you going to change this to BoundDefaultExpression?

I was, but the synthesized expression was emitting warnings about using "dot" after default, so I reverted it.

AlekseyTs · 2017-04-27T17:25:35Z

Done with review pass (iteration 54)

AlekseyTs · 2017-04-27T18:53:18Z

src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs

+            Assert.Equal(1, m.Length);
+            var returnSyntax = ((MethodSymbol) m.First()).ExtractReturnTypeSyntax();
+            var newLineCount = Environment.NewLine.Length * 1;
+            Assert.Equal(returnSyntax.Location.SourceSpan, TextSpan.FromBounds(34 + newLineCount, 38 + newLineCount));


Assert.Equal(returnSyntax.Location.SourceSpan, TextSpan.FromBounds(34 + newLineCount, 38 + newLineCount)); [](start = 12, length = 106)

I think it would be better to find the node that you expect directly from the syntax tree. This way I cannot really tell if your expectations are correct. Also, I believe expected value should be first, otherwise failure message is confusing.

Once you find the node, simply assert that the nodes are the same.

In reply to: 113774955 [](ancestors = 113774955)

This applies to other new tests

Done, and Done

AlekseyTs · 2017-04-27T18:56:03Z

src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs

+            var comp = CreateStandardCompilation(text);
+            var global = comp.GlobalNamespace;
+            var a = global.GetTypeMembers("A", 0).Single();
+            var m = a.GetMembers("M");


var m = a.GetMembers("M"); [](start = 12, length = 26)

I assume this is definition, please assert that for clarity.

AlekseyTs · 2017-04-27T18:56:10Z

src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs

@@ -203,6 +204,84 @@ public partial class A {
        }

        [Fact]
+        public void PartialExtractSyntaxLocation_DeclBeforeDef()


PartialExtractSyntaxLocation_DeclBeforeDef [](start = 20, length = 42)

Please also test what the API returns when called off of implementation.

AlekseyTs · 2017-04-27T18:56:23Z

src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs

+        }
+
+        [Fact]
+        public void PartialExtractSyntaxLocation_DefBeforeDecl()


PartialExtractSyntaxLocation_DefBeforeDecl [](start = 20, length = 42)

Please also test what the API returns when called off of implementation.

How can I use the API to get the non-declaration method symbol?

AlekseyTs · 2017-04-27T18:58:05Z

Done with review pass (iteration 56)

agocke · 2017-04-27T19:41:52Z

src/Test/Utilities/CoreClr/CoreCLRRuntimeEnvironment.cs

@@ -72,23 +72,18 @@ public CoreCLRRuntimeEnvironment(IEnumerable<ModuleData> additionalDependencies
            }
        }

-        public (int ExitCode, string Output) Execute(string moduleName, int expectedOutputLength)
+        public int Execute(string moduleName, string[] args, string expectedOutput)


It looks like args is just being dropped.

agocke · 2017-04-27T19:43:55Z

src/Test/Utilities/CoreClr/SharedConsoleOutWriter.cs

+                errorOutputWriter = new CappedStringWriter(exLength);
+                outputWriter = new CappedStringWriter(exLength);
+            }
+            else


Just because there's no expected output doesn't mean we shouldn't capture the incorrect output. I would keep always using CappedStringWriter.

CappedStringWriter fails if the capacity is exceeded, if you don't fill out expectedOutput, then it would be like saying "this program can not produce output.

This whole file has been reverted.

agocke · 2017-04-27T19:44:18Z

src/Test/Utilities/CoreClr/SharedConsoleOutWriter.cs

@@ -11,25 +11,35 @@ internal static class SharedConsole
        private static TextWriter s_savedConsoleOut;
        private static TextWriter s_savedConsoleError;

-        private static AsyncLocal<StringWriter> s_currentOut;
-        private static AsyncLocal<StringWriter> s_currentError;
+        private static AsyncLocal<TextWriter> s_currentOut;


Given my comment below this can stay as StringWriter.

This whole file has been reverted.

agocke · 2017-04-27T19:45:11Z

src/Test/Utilities/CoreClr/TestExecutionLoadContext.cs

@@ -95,7 +95,7 @@ private Assembly LoadImageAsAssembly(ImmutableArray<byte> mainImage)
            }
        }

-        internal (int ExitCode, string Output) Execute(ImmutableArray<byte> mainImage, int expectedOutputLength)
+        internal (int ExitCode, string Output) Execute(ImmutableArray<byte> mainImage, int? expectedOutputLength)


I don't see a reason for the int?. If they don't expect output, just put in 0.

0 would mean "this program must produce no output"

agocke · 2017-04-27T19:50:47Z

src/Test/Utilities/Desktop/CodeRuntime/DesktopRuntimeEnvironment.cs

        {
            try
            {
                var emitData = GetEmitData();
                emitData.RuntimeData.ExecuteRequested = true;
-                return emitData.Manager.Execute(moduleName, expectedOutputLength, out processOutput);
+                var resultCode = emitData.Manager.Execute(moduleName, args, expectedOutput?.Length, out var output);


I would just pass expectedOutput?.Length ?? 0 and keep the arg as a non-nullable int.

See above comment.

This whole file has been reverted.

agocke · 2017-04-27T19:51:28Z

src/Test/Utilities/Desktop/CodeRuntime/DesktopRuntimeEnvironment.cs

@@ -377,10 +371,20 @@ CompilationTestData IInternalRuntimeEnvironment.GetCompilationTestData()

        private static readonly object s_consoleGuard = new object();

-        internal static void Capture(Action action, int expectedLength, out string output, out string errorOutput)
+        internal static void Capture(Action action, int? expectedLength, out string output, out string errorOutput)


I don't think we need the int?. The comparison doesn't happen until the top level and that helper can use expectedOutput == null as a sentinel.

removed and defaulted with 0.

agocke · 2017-04-27T19:53:09Z

src/Test/Utilities/Portable/CommonTestBase.cs

@@ -136,6 +142,8 @@ public abstract partial class CommonTestBase : TestBase
                                manifestResources,
                                expectedSignatures,
                                expectedOutput,
+                                expectedReturnCode,
+                                args ?? new string[] { },


Array.Empty<string>()

AlekseyTs · 2017-04-28T17:41:42Z

src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs

+            var node = tree.GetRoot().FindNode(token.Span);
+
+            var mLocations = tree.GetRoot().DescendantTokens().Where(t => t.ValueText == "M").ToList();
+            var otherSymbol = (MethodSymbol)model.GetDeclaredSymbolForNode(tree.GetRoot().FindNode(mLocations[0].Span));


var otherSymbol = (MethodSymbol)model.GetDeclaredSymbolForNode(tree.GetRoot().FindNode(mLocations[0].Span)); [](start = 12, length = 108)

This is way too complicated than it should be.

AlekseyTs · 2017-04-28T17:41:54Z

src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs

+            var tree = comp.SyntaxTrees.Single();
+            var model = comp.GetSemanticModel(tree);
+            var token = tree.GetRoot().DescendantTokens().Where(t => t.Kind() == SyntaxKind.VoidKeyword).Skip(1).First();
+            var node = tree.GetRoot().FindNode(token.Span);


var node = tree.GetRoot().FindNode(token.Span); [](start = 12, length = 47)

This is way too complicated than it should be.

TyOverby added 2 commits April 3, 2017 11:01

attempt to get main entrypoints working

e27fb34

async main codegen

b4cee90

TyOverby added the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Apr 5, 2017

TyOverby requested a review from jaredpar April 5, 2017 18:02

dnfclas added the cla-already-signed label Apr 5, 2017

TyOverby removed the request for review from jaredpar April 5, 2017 18:02

TyOverby added 4 commits April 5, 2017 11:04

remove garbage file

61e4705

fix vb tests

39a8600

add some comments

0cb90e3

moduleBeingBuilt can sometimes be null apparently

4af405a

TyOverby removed the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Apr 6, 2017

jaredpar reviewed Apr 6, 2017

View reviewed changes

TyOverby added 2 commits April 6, 2017 10:18

fix style nits

5a3fbcf

more whitespace fixes

b16987f

jaredpar requested changes Apr 6, 2017

View reviewed changes

cston reviewed Apr 6, 2017

View reviewed changes

TyOverby added 2 commits April 7, 2017 09:04

address feedback

8f0e223

automated style fixer

ed6271d

jaredpar requested changes Apr 7, 2017

View reviewed changes

TyOverby added 2 commits April 7, 2017 13:39

move most work out into the constructor

578e811

protect against null in error condition

92d3bfc

jaredpar reviewed Apr 8, 2017

View reviewed changes

cston reviewed Apr 26, 2017

View reviewed changes

TyOverby added 3 commits April 26, 2017 08:46

move some methods around, split some tests up

0a84b1d

fix non-returning async tests, remove bool return from check availabi…

e4e355e

…lity

add params test

b290268

AlekseyTs reviewed Apr 26, 2017

View reviewed changes

cston reviewed Apr 26, 2017

View reviewed changes

TyOverby added 2 commits April 26, 2017 11:09

style and expected output fix

eb68cea

switch back to null because default emits warnings

247e05d

AlekseyTs reviewed Apr 27, 2017

View reviewed changes

TyOverby added 2 commits April 27, 2017 10:37

add partial method tests

8e60326

fix locations for unix

881dea0

AlekseyTs reviewed Apr 27, 2017

View reviewed changes

agocke reviewed Apr 27, 2017

View reviewed changes

TyOverby added 4 commits April 27, 2017 13:56

better partial syntax tests

1d2b4db

use expected length of 0

3e8237a

use default length of 0 for expected output

2047833

check implementation symbol

7187917

TyOverby merged commit 0539155 into dotnet:features/async-main Apr 28, 2017

AlekseyTs reviewed Apr 28, 2017

View reviewed changes

		_paramKind = userMain.ParameterCount == 0 ? ParamKind.NoArgs : ParamKind.StringArrayArgs;
		_returnKind = GetReturnKind(compilation, userMain.ReturnType);


		if (expectedOutput.Trim() != actualOutput.Trim())
		if (expectedOutput.Trim() != Output.Trim())



		var syntax = method.ExtractReturnTypeSyntax();
		var dumbInstance = new BoundLiteral(syntax, ConstantValue.Null, method.ReturnType);

Async main codegen #18472

Async main codegen #18472

Conversation

TyOverby commented Apr 5, 2017

TyOverby commented Apr 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TyOverby Apr 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TyOverby Apr 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TyOverby commented Apr 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TyOverby commented Apr 6, 2017 •

edited

Loading

TyOverby Apr 6, 2017 •

edited

Loading

TyOverby Apr 6, 2017 •

edited

Loading

TyOverby commented Apr 8, 2017 •

edited

Loading