Skip to content

Enable updated escape rules if System.Runtime.CompilerServices.RuntimeFeature.ByRefFields exists#62304

Merged
cston merged 1 commit intodotnet:mainfrom
cston:62131
Jul 26, 2022
Merged

Enable updated escape rules if System.Runtime.CompilerServices.RuntimeFeature.ByRefFields exists#62304
cston merged 1 commit intodotnet:mainfrom
cston:62131

Conversation

@cston
Copy link
Contributor

@cston cston commented Jul 1, 2022

Fixes #62131

@cston cston requested a review from a team as a code owner July 1, 2022 05:00
@ghost ghost added the Area-Compilers label Jul 1, 2022
@cston
Copy link
Contributor Author

cston commented Jul 25, 2022

@dotnet/roslyn-compiler, please review.

// (10,20): error CS8167: Cannot return by reference a member of parameter 'r' because it is not a ref or out parameter
// return ref r.F;
Diagnostic(ErrorCode.ERR_RefReturnParameter2, "r").WithArguments("r").WithLocation(10, 20));
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion10, "ref T").WithArguments("ref fields", "11.0").WithLocation(3, 12));
Copy link
Member

@jcouv jcouv Jul 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're missing an error on line 854 below. If you try to use ref fields in a C# 11 compilation but the runtime feature flag doesn't exist, the compiler should complain.
This can be handled in a separate PR, since I understand this PR is meant for whether the updated escape rules kick in.

This is described in the spec:
"This feature [ref fields] requires runtime support and changes to the ECMA spec. As such these will only be enabled when the corresponding feature flag is set in corelib. The issue tracking the exact API is tracked here dotnet/runtime#64165"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. #62919.

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Thanks (iteration 1) with follow-up issue for potentially missing runtime feature flag issue.

@cston
Copy link
Contributor Author

cston commented Jul 25, 2022

@dotnet/roslyn-compiler, for a second review of small change, thanks.

@cston cston merged commit 4260c6e into dotnet:main Jul 26, 2022
@cston cston deleted the 62131 branch July 26, 2022 00:37
@ghost ghost added this to the Next milestone Jul 26, 2022
@allisonchou allisonchou modified the milestones: Next, 17.4 P1 Jul 26, 2022
cston added a commit to cston/roslyn that referenced this pull request Jul 29, 2022
…s.RuntimeFeature.ByRefFields exists (dotnet#62304)"

This reverts commit 4260c6e.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable updated escape rules if System.Runtime.CompilerServices.RuntimeFeature.ByRefFields exists

4 participants