Add LangVersion 7.2 for C# #19846

jcouv · 2017-05-29T15:44:42Z

@dotnet/roslyn-compiler for review (15.6).

jcouv · 2017-05-30T16:58:07Z

microbuild_prtest failed with a connection failure (details). I will retest.

09:37:03 GitHub pull request #19846 of commit 2eaede586e424d6bde146f0c0f44c812dcf2e4b8, no merge conflicts.
09:37:03 Notifying endpoint with id 'HTTP:helix-int-notification-url'
09:52:12 ERROR: Failed to notify endpoint with id 'HTTP:helix-int-notification-url'
09:52:12 
hudson.remoting.RequestAbortedException: java.io.IOException: Connection aborted: org.jenkinsci.remoting.nio.NioChannelHub$MonoNioTransport@6c2d6432[name=win201-76cf11]
09:52:12 	at hudson.remoting.Request.abort(Request.java:303)
09:52:12 	at hudson.remoting.Channel.terminate(Channel.java:863)
09:52:12 	at hudson.remoting.Channel$1.terminate(Channel.java:509)
09:52:12 	at hudson.remoting.AbstractByteArrayCommandTransport$1.terminate(AbstractByteArrayCommandTransport.java:71)
09:52:12 	at org.jenkinsci.remoting.nio.NioChannelHub$NioTransport.abort(NioChannelHub.java:208)
09:52:12 	at org.jenkinsci.remoting.nio.NioChannelHub.run(NioChannelHub.java:629)
09:52:12 	at jenkins.util.ContextResettingExecutorService$1.run(ContextResettingExecutorService.java:28)
09:52:12 	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
09:52:12 	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
09:52:12 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
09:52:12 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
09:52:12 	at java.lang.Thread.run(Thread.java:748)
09:52:12 	at ......remote call to win201-76cf11(Native Method)
09:52:12 	at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1433)
09:52:12 	at hudson.remoting.Request.call(Request.java:172)
09:52:12 	at hudson.remoting.Channel.call(Channel.java:796)
09:52:12 	at hudson.EnvVars.getRemote(EnvVars.java:405)
09:52:12 	at hudson.model.Computer.getEnvironment(Computer.java:1152)
09:52:12 	at jenkins.model.CoreEnvironmentContributor.buildEnvironmentFor(CoreEnvironmentContributor.java:30)
09:52:12 	at hudson.model.Run.getEnvironment(Run.java:2241)
09:52:12 	at hudson.model.AbstractBuild.getEnvironment(AbstractBuild.java:922)
09:52:12 	at com.tikal.hudson.plugins.notification.Phase.buildJobState(Phase.java:80)
09:52:12 	at com.tikal.hudson.plugins.notification.Phase.handle(Phase.java:42)
09:52:12 	at com.tikal.hudson.plugins.notification.JobListener.onStarted(JobListener.java:31)
09:52:12 	at hudson.model.listeners.RunListener.fireStarted(RunListener.java:240)
09:52:12 	at hudson.model.Run.execute(Run.java:1716)
09:52:12 	at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43)
09:52:12 	at hudson.model.ResourceController.execute(ResourceController.java:98)
09:52:12 	at hudson.model.Executor.run(Executor.java:404)
09:52:12 Caused by: java.io.IOException: Connection aborted: org.jenkinsci.remoting.nio.NioChannelHub$MonoNioTransport@6c2d6432[name=win201-76cf11]
09:52:12 	at org.jenkinsci.remoting.nio.NioChannelHub$NioTransport.abort(NioChannelHub.java:208)
09:52:12 	at org.jenkinsci.remoting.nio.NioChannelHub.run(NioChannelHub.java:629)
09:52:12 	at jenkins.util.ContextResettingExecutorService$1.run(ContextResettingExecutorService.java:28)
09:52:12 	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
09:52:12 	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
09:52:12 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
09:52:12 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
09:52:12 	at java.lang.Thread.run(Thread.java:748)
09:52:12 Caused by: java.io.IOException: Connection timed out
09:52:12 	at sun.nio.ch.FileDispatcherImpl.read0(Native Method)
09:52:12 	at sun.nio.ch.SocketDispatcher.read(SocketDispatcher.java:39)
09:52:12 	at sun.nio.ch.IOUtil.readIntoNativeBuffer(IOUtil.java:223)
09:52:12 	at sun.nio.ch.IOUtil.read(IOUtil.java:197)
09:52:12 	at sun.nio.ch.SocketChannelImpl.read(SocketChannelImpl.java:380)
09:52:12 	at org.jenkinsci.remoting.nio.FifoBuffer$Pointer.receive(FifoBuffer.java:137)
09:52:12 	at org.jenkinsci.remoting.nio.FifoBuffer.receive(FifoBuffer.java:310)
09:52:12 	at org.jenkinsci.remoting.nio.NioChannelHub.run(NioChannelHub.java:561)
09:52:12 	... 6 more
09:52:12 Failed to notify endpoint with id 'HTTP:helix-int-notification-url' - hudson.remoting.RequestAbortedException: java.io.IOException: Connection aborted: org.jenkinsci.remoting.nio.NioChannelHub$MonoNioTransport@6c2d6432[name=win201-76cf11]
09:52:12 Notifying endpoint with id 'HTTP:helix-prod-notification-url'
09:52:12 ERROR: Failed to notify endpoint with id 'HTTP:helix-prod-notification-url'
09:52:12 hudson.remoting.ChannelClosedException: channel is already closed
09:52:12 	at hudson.remoting.Channel.send(Channel.java:578)
09:52:12 	at hudson.remoting.Request.call(Request.java:130)
09:52:12 	at hudson.remoting.Channel.call(Channel.java:796)
09:52:12 	at hudson.EnvVars.getRemote(EnvVars.java:405)
09:52:12 	at hudson.model.Computer.getEnvironment(Computer.java:1152)
09:52:12 	at jenkins.model.CoreEnvironmentContributor.buildEnvironmentFor(CoreEnvironmentContributor.java:30)
09:52:12 	at hudson.model.Run.getEnvironment(Run.java:2241)
09:52:12 	at hudson.model.AbstractBuild.getEnvironment(AbstractBuild.java:922)
09:52:12 	at com.tikal.hudson.plugins.notification.Phase.buildJobState(Phase.java:80)
09:52:12 	at com.tikal.hudson.plugins.notification.Phase.handle(Phase.java:42)
09:52:12 	at com.tikal.hudson.plugins.notification.JobListener.onStarted(JobListener.java:31)
09:52:12 	at hudson.model.listeners.RunListener.fireStarted(RunListener.java:240)
09:52:12 	at hudson.model.Run.execute(Run.java:1716)
09:52:12 	at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43)
09:52:12 	at hudson.model.ResourceController.execute(ResourceController.java:98)
09:52:12 	at hudson.model.Executor.run(Executor.java:404)
09:52:12 Caused by: java.io.IOException: Connection aborted: org.jenkinsci.remoting.nio.NioChannelHub$MonoNioTransport@6c2d6432[name=win201-76cf11]
09:52:12 	at org.jenkinsci.remoting.nio.NioChannelHub$NioTransport.abort(NioChannelHub.java:208)
09:52:12 	at org.jenkinsci.remoting.nio.NioChannelHub.run(NioChannelHub.java:629)
09:52:12 	at jenkins.util.ContextResettingExecutorService$1.run(ContextResettingExecutorService.java:28)
09:52:12 	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
09:52:12 	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
09:52:12 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
09:52:12 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
09:52:12 	at java.lang.Thread.run(Thread.java:748)
09:52:12 Caused by: java.io.IOException: Connection timed out
09:52:12 	at sun.nio.ch.FileDispatcherImpl.read0(Native Method)
09:52:12 	at sun.nio.ch.SocketDispatcher.read(SocketDispatcher.java:39)
09:52:12 	at sun.nio.ch.IOUtil.readIntoNativeBuffer(IOUtil.java:223)
09:52:12 	at sun.nio.ch.IOUtil.read(IOUtil.java:197)
09:52:12 	at sun.nio.ch.SocketChannelImpl.read(SocketChannelImpl.java:380)
09:52:12 	at org.jenkinsci.remoting.nio.FifoBuffer$Pointer.receive(FifoBuffer.java:137)
09:52:12 	at org.jenkinsci.remoting.nio.FifoBuffer.receive(FifoBuffer.java:310)
09:52:12 	at org.jenkinsci.remoting.nio.NioChannelHub.run(NioChannelHub.java:561)
09:52:12 	... 6 more
09:52:12 Failed to notify endpoint with id 'HTTP:helix-prod-notification-url' - hudson.remoting.ChannelClosedException: channel is already closed
09:52:12 Notifying endpoint with id 'HTTP:legacy-notification-url'
09:52:13 Setting status of 2eaede586e424d6bde146f0c0f44c812dcf2e4b8 to PENDING with url https://ci.dot.net/job/dotnet_roslyn/job/dev15.6/job/microbuild_prtest/90/ and message: 'Started.'
09:52:13 Using context: microbuild_prtest
09:52:13 FATAL: null
09:52:13 java.lang.NullPointerException
09:52:13 [Current build status] check if current [FAILURE] is worse or equals then [ABORTED] and better or equals then [FAILURE]

jcouv · 2017-05-30T16:58:18Z

retest microbuild_prtest please

AlekseyTs · 2017-05-30T18:29:21Z

src/Compilers/CSharp/Portable/LanguageVersion.cs

@@ -120,6 +126,8 @@ internal static ErrorCode GetErrorCode(this LanguageVersion version)
                    return ErrorCode.ERR_FeatureNotAvailableInVersion7;
                case LanguageVersion.CSharp7_1:
                    return ErrorCode.ERR_FeatureNotAvailableInVersion7_1;
+                case LanguageVersion.CSharp7_2:
+                    return ErrorCode.ERR_FeatureNotAvailableInVersion7_2;


ERR_FeatureNotAvailableInVersion7_2 [](start = 37, length = 35)

What is the benefit of having an error code per version? It looks like we could easily use one parameterized error message. In fact, for some features in some situations we do this (use parameterized error message) when these errors cannot be used. #Closed

I'd raised that question before. Someone (maybe Vlad or Neal) had previously suggested this helps telemetry.
Filed follow-up issue #19871

Dug up the telemetry comment (from Cyrus): #17435 (comment)

@CyrusNajmabadi Could you expand on why separate error codes and diagnostics is preferable for LanguageVersion, rather than one parameterized set?

My telemetry comment was about you instantiating SolutionChangeAction directly. You need to create a subclass of that.

I do not care if you use a single diagnostic ID or different diagnostic IDs.

Thanks for the clarification.

I have a follow-up issue to use a generic diagnostic ID.
@AlekseyTs Any other feedback?

I think @gafter was the person who pointed out the telemetry issue to me. He can confirm if that is still an issue or not.

AlekseyTs · 2017-05-30T18:45:11Z

Done with review pass. #Closed

Conflicts: src/Compilers/CSharp/Portable/CSharpResources.resx src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs

jcouv · 2017-05-30T22:40:33Z

Rebased to resolve conflicts.
@dotnet/roslyn-compiler for a second review. Thanks

OmarTawfik · 2017-05-30T23:04:26Z

src/Compilers/CSharp/Portable/LanguageVersion.cs

@@ -120,6 +126,8 @@ internal static ErrorCode GetErrorCode(this LanguageVersion version)
                    return ErrorCode.ERR_FeatureNotAvailableInVersion7;
                case LanguageVersion.CSharp7_1:
                    return ErrorCode.ERR_FeatureNotAvailableInVersion7_1;
+                case LanguageVersion.CSharp7_2:
+                    return ErrorCode.ERR_FeatureNotAvailableInVersion7_2;


I think @gafter was the person who pointed out the telemetry issue to me. He can confirm if that is still an issue or not.

OmarTawfik · 2017-05-30T23:04:46Z

src/Compilers/CSharp/Portable/Errors/ErrorCode.cs

@@ -1488,5 +1488,9 @@ internal enum ErrorCode
        ERR_PatternWrongGenericTypeInVersion = 8314,

        #endregion diagnostics introduced for C# 7.1
+
+        #region diagnostics introduced for C# 7.2
+        ERR_FeatureNotAvailableInVersion7_2 = 8320,


Since this is going into master, and there are no concers of conflicts, we can just use 8315?

This is going into 15.6 branch. 7.1 is not closed yet, so I left some buffer. (Just today I had to use a new error code for a 7.1 fix).

OmarTawfik · 2017-05-30T23:08:13Z

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -2705,7 +2705,7 @@ A catch() block after a catch (System.Exception e) block can catch non-CLS excep
    <value>This warning occurs if the assembly attributes AssemblyKeyFileAttribute or AssemblyKeyNameAttribute found in source conflict with the /keyfile or /keycontainer command line option or key file name or key container specified in the Project Properties.</value>
  </data>


I suggest to do a string search/replace over the modified error strings in the tests project, to make sure that the error comments in tests are updated as well.

The resource strings I modified do not exist in tests. But I do have a test to extract the versions that appear on that line and verifies that it's complete: https://github.com/dotnet/roslyn/pull/19113/files#diff-492ea20f4ae7785ece041adfb13b4e7d

OmarTawfik · 2017-05-30T23:08:38Z

Approved, suggesting that this is accompanied by the project system PR as well.

jcouv · 2017-05-30T23:18:54Z

Thanks for the reminder. I'm starting on the project-system change.

jcouv · 2017-05-31T01:57:51Z

Here's the project system change to add the drop-down entry in Settings page: dotnet/project-system#2334

AlekseyTs

LGTM

jcouv · 2017-05-31T18:52:12Z

VS integration tests failed on Roslyn.VisualStudio.IntegrationTests.CSharp.CSharpCodeActions.FastDoubleInvoke (details)>

@dotnet/roslyn-infrastructure FYI. I'll re-run this leg.

jcouv · 2017-05-31T18:52:53Z

@dotnet-bot retest windows_release_vs-integration_prtest please

jcouv added the Area-Compilers label May 29, 2017

jcouv self-assigned this May 29, 2017

dnfclas added the cla-already-signed label May 29, 2017

AlekseyTs reviewed May 30, 2017

View reviewed changes

jcouv mentioned this pull request May 30, 2017

Use parameterized error message for ERR_FeatureNotAvailableInVersion #19871

Open

jcouv force-pushed the langver-72 branch from 2eaede5 to ce1fa4b Compare May 30, 2017 22:36

Add LangVersion 7.2 for C#

ff5015a

Conflicts: src/Compilers/CSharp/Portable/CSharpResources.resx src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs

jcouv force-pushed the langver-72 branch from ce1fa4b to ff5015a Compare May 30, 2017 22:40

OmarTawfik approved these changes May 30, 2017

View reviewed changes

jcouv mentioned this pull request May 31, 2017

Add option for LangVersion 7.2 dotnet/project-system#2334

Merged

jcouv mentioned this pull request May 31, 2017

Add VB LanguageVersion 15.6 #19896

Merged

AlekseyTs approved these changes May 31, 2017

View reviewed changes

jcouv mentioned this pull request May 31, 2017

[Work items] Non-trailing named arguments #19890

Closed

12 tasks

jcouv merged commit c5d9858 into dotnet:dev15.6 May 31, 2017

jcouv deleted the langver-72 branch May 31, 2017 19:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add LangVersion 7.2 for C# #19846

Add LangVersion 7.2 for C# #19846

jcouv commented May 29, 2017

jcouv commented May 30, 2017

jcouv commented May 30, 2017

AlekseyTs May 30, 2017 •

edited

Loading

jcouv May 30, 2017

jcouv May 30, 2017

CyrusNajmabadi May 30, 2017

jcouv May 30, 2017

OmarTawfik May 30, 2017

AlekseyTs commented May 30, 2017 •

edited

Loading

jcouv commented May 30, 2017

OmarTawfik May 30, 2017

OmarTawfik May 30, 2017

jcouv May 30, 2017

OmarTawfik May 30, 2017

jcouv May 30, 2017

OmarTawfik commented May 30, 2017

jcouv commented May 30, 2017

jcouv commented May 31, 2017

AlekseyTs left a comment

jcouv commented May 31, 2017

jcouv commented May 31, 2017

		@@ -2705,7 +2705,7 @@ A catch() block after a catch (System.Exception e) block can catch non-CLS excep
		<value>This warning occurs if the assembly attributes AssemblyKeyFileAttribute or AssemblyKeyNameAttribute found in source conflict with the /keyfile or /keycontainer command line option or key file name or key container specified in the Project Properties.</value>
		</data>

Add LangVersion 7.2 for C# #19846

Add LangVersion 7.2 for C# #19846

Conversation

jcouv commented May 29, 2017

jcouv commented May 30, 2017

jcouv commented May 30, 2017

AlekseyTs May 30, 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

AlekseyTs commented May 30, 2017 • edited Loading

jcouv commented May 30, 2017

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

OmarTawfik commented May 30, 2017

jcouv commented May 30, 2017

jcouv commented May 31, 2017

AlekseyTs left a comment

Choose a reason for hiding this comment

jcouv commented May 31, 2017

jcouv commented May 31, 2017

AlekseyTs May 30, 2017 •

edited

Loading

AlekseyTs commented May 30, 2017 •

edited

Loading