Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create spec for lambda params with modifiers without type name #7369

Merged

Conversation

Rekkonnect
Copy link
Contributor

@Rekkonnect Rekkonnect commented Jul 22, 2023

Creates a spec for #338

This permits the simplified declaration of lambdas with parameters with modifiers, by omitting their type name.

@Rekkonnect Rekkonnect requested a review from a team as a code owner July 22, 2023 09:50
@Rekkonnect Rekkonnect changed the title Create spec for ref/in/out modifiers lambda params without type name Create spec for in/ref/out modifiers lambda params without type name Jul 22, 2023
@CyrusNajmabadi
Copy link
Member

Can you add a line saying this is only for parenthesized lambdas?

@CyrusNajmabadi
Copy link
Member

Speclet lgtm. @333fred for another pair of eyes.

@Rekkonnect Rekkonnect changed the title Create spec for in/ref/out modifiers lambda params without type name Create spec for lambda params with by-ref modifiers without type name Jul 26, 2023
@Rekkonnect
Copy link
Contributor Author

An edge case includes the addition of the scoped modifier in C# 11. Currently, lambda parameters accept the scoped modifier, therefore those could also be affected from this spec. We want to decide whether those are being supported or not.

@Rekkonnect
Copy link
Contributor Author

Also, in preview, params in lambda parameters is being supported. We also need to consider this case.

Copy link
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

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

A few small comments, overall this is looking good.

proposals/ref-out-lambda-params.md Outdated Show resolved Hide resolved
proposals/ref-out-lambda-params.md Outdated Show resolved Hide resolved
proposals/ref-out-lambda-params.md Outdated Show resolved Hide resolved
Copy link
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

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

@CyrusNajmabadi this looks good enough to go to LDM to me. Want to add it to "schedule when convenient"?

@CyrusNajmabadi
Copy link
Member

@333fred I was going to check with @MadsTorgersen @KathleenDollard first when teh right time was to bring things since we're so heads down in 12 right now. Basically, i'd bring this the next time we're open to looking for the next batch of championed constructs to take :)

@333fred
Copy link
Member

333fred commented Jul 28, 2023

I believe that's what "schedule when convenient" there for 😉.

@CyrusNajmabadi
Copy link
Member

Ah, i thought those were for meetings where we didn't have anything pressing, but we were still prior to the point that we were trying to decide on new features :)

@Rekkonnect
Copy link
Contributor Author

As for scoped parameters, since scoped is contextual, the compiler currently attempts to parse that as a type. Perhaps we will not make a move to support (scoped x) to infer the type of x and granting it the scoped modifier.

@Rekkonnect Rekkonnect changed the title Create spec for lambda params with by-ref modifiers without type name Create spec for lambda params with modifiers without type name Aug 6, 2023
@Rekkonnect
Copy link
Contributor Author

As for scoped parameters, since scoped is contextual, the compiler currently attempts to parse that as a type. Perhaps we will not make a move to support (scoped x) to infer the type of x and granting it the scoped modifier.

As per the list of breaking changes for C# 11, scoped no longer refers to a type name, therefore this restriction should not take place. scoped can be safely evaluated as a modifier in an implicit type lambda parameter list.

Therefore:

private delegate int D(scoped int x, scoped int y);
D d = (scoped x, scoped y) => x + y;

is valid and scoped reflects the modifier, not a type named scoped.

Will update the spec accordingly to reflect this change, citing the above reference.

@jjonescz
Copy link
Member

jjonescz commented Aug 8, 2023

scoped no longer refers to a type name

Is that right? Seems the breaking change only disallows declaring new types named scoped, but you could reference an existing type named scoped from a DLL and then this would be a break (not saying the break wouldn't be acceptable):

var d = (scoped x, scoped y) => x + y;

@Rekkonnect
Copy link
Contributor Author

The example:

ref scoped local; // Error

Hints that all available types named scoped are now inaccessible. And that behavior sounds very sane to me, since the type is no longer being explicitly available in other contexts, it would not be reasonable to enable accessing them in implicit lambda parameters.

@jjonescz
Copy link
Member

jjonescz commented Aug 8, 2023

ref scoped local; // Error

Maybe that's an error only when there's ref.

Hints that all available types named scoped are now inaccessible.

This test succeeds for me:

[Fact]
public void Scoped()
{
    var source1 = """
        public class scoped { }
        """;

    CreateCompilation(source1).VerifyDiagnostics(
        // (1,14): error CS9062: Types and aliases cannot be named 'scoped'.
        // public class scoped { }
        Diagnostic(ErrorCode.ERR_ScopedTypeNameDisallowed, "scoped").WithLocation(1, 14));

    var comp1 = CreateCompilation(source1, parseOptions: TestOptions.Regular10).VerifyDiagnostics(
        // (1,14): warning CS8981: The type name 'scoped' only contains lower-cased ascii characters. Such names may become reserved for the language.
        // public class scoped { }
        Diagnostic(ErrorCode.WRN_LowerCaseTypeName, "scoped").WithArguments("scoped").WithLocation(1, 14));
    var comp1Ref = comp1.EmitToImageReference();

    var source2 = """
        var d = (scoped x) => System.Console.WriteLine(x);
        d(new scoped());
        """;
    CompileAndVerify(source2, new[] { comp1Ref }, expectedOutput: "scoped").VerifyDiagnostics();
}

@Rekkonnect
Copy link
Contributor Author

Nice observation, with a little bit more testing, even the listed breaking change does not currently apply:
SharpLab link

Note that the above uses the Default branch which is C# 11 based on the fact that the error about naming a type scoped exists.

This is either an undocumented decision, or an omission in the feature. We need a definite answer to how we're going to support scoped in implicit lambda parameter lists.

@CyrusNajmabadi
Copy link
Member

if we're going to support ref and the like without an explicit type, then we need to support scoped in the same fashoin.

@Rekkonnect
Copy link
Contributor Author

Do we want to introduce a breaking change in the following example:

public class C
{
    public void M(scoped sc, scoped scoped scsc)
    {
        MSc mSc = M;
        MSc mSc1 = (scoped sc, scoped scoped scsc) => { };
    }
    
    public delegate void MSc(scoped sc, scoped scoped scsc);
}

public ref struct @scoped { }

I really hate how this types out anyway and I believe it would be best to introduce the breaking change by requiring the @ in the @scoped type.

@CyrusNajmabadi
Copy link
Member

I would put it as a case to consider and allow the ldm to make a determination on if it's ok or not.

Personally, I would take the break.

@Rekkonnect
Copy link
Contributor Author

Digging through the issues, I also discovered that #6149 is touching the subject of scoped, perhaps it would be helpful to mention it here for future reference once the spec is settled.


Parameter declarations in lambda expressions with parenthesized parameters now permit a single identifier after a modifier on the parameter. This does not apply to lambda expressions with a single parameter with omitted parentheses.

For example,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
For example,
For example, these would not be legal.

Also, just do like the first 3 examples.

| 'readonly'
| 'params'
;
```

Should the above production rule be updated to only reflect the valid combinations of modifiers, the `implicit_parenthesized_anonymous_function_parameter` rule is updated accordingly to reflect the applicable modifiers.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this note should also be removed

@CyrusNajmabadi CyrusNajmabadi merged commit c52e7f9 into dotnet:main Feb 20, 2024
1 check passed
@Rekkonnect Rekkonnect deleted the ref-out-lambda-params-without-type-name branch October 7, 2024 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants