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

Disallow modifiers in local_using_declaration #72589

Merged
merged 9 commits into from
Mar 22, 2024
Merged

Conversation

jcouv
Copy link
Member

@jcouv jcouv commented Mar 19, 2024

Fixes #72496

@jcouv jcouv self-assigned this Mar 19, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Mar 19, 2024
@jcouv jcouv marked this pull request as ready for review March 20, 2024 21:18
@jcouv jcouv requested a review from a team as a code owner March 20, 2024 21:18
@jcouv jcouv requested a review from 333fred March 21, 2024 17:08
@jcouv
Copy link
Member Author

jcouv commented Mar 21, 2024

@dotnet/roslyn-compiler for a second review. Thanks

@@ -7902,4 +7902,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_ParamsCollectionMissingConstructor" xml:space="preserve">
<value>Non-array params collection type must have an applicable constructor that can be called with no arguments.</value>
</data>
<data name="ERR_NoModifiersOnUsing" xml:space="preserve">
<value>Modifiers cannot be placed on resource declarations</value>
Copy link
Member

@cston cston Mar 21, 2024

Choose a reason for hiding this comment

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

resource declarations

Do we use the term "resource declarations" elsewhere for these declarations? Consider "using declarations" instead. #Resolved

// (1,39): error CS1002: ; expected
// using (const var obj2 = new object()) { }
Diagnostic(ErrorCode.ERR_SemicolonExpected, "{").WithLocation(1, 39));
}
Copy link
Member

@cston cston Mar 21, 2024

Choose a reason for hiding this comment

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

}

Consider testing using const System.IDisposable d = null; which currently binds without any errors. #Resolved

Copy link
Member Author

@jcouv jcouv Mar 21, 2024

Choose a reason for hiding this comment

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

const cases are tested in ModifiersInUsingLocalDeclarations_Const and ModifiersInUsingLocalDeclarations_Const_Async
Never mind, you meant with an explicit type... Sorry for the confusion

// (1,44): error CS1002: ; expected
// await using (const var obj = new object()) { }
Diagnostic(ErrorCode.ERR_SemicolonExpected, "{").WithLocation(1, 44));
}
Copy link
Member

@cston cston Mar 21, 2024

Choose a reason for hiding this comment

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

}

Consider testing await using const System.IAsyncDisposable d = null; which currently binds without errors. #Resolved

@jcouv jcouv requested a review from cston March 21, 2024 18:41
{
void M()
{
using const System.IAsyncDisposable obj = null;
Copy link
Member

Choose a reason for hiding this comment

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

It looks like the cases in this test should be IDisposable rather than IAsyncDisposable.

@jcouv jcouv enabled auto-merge (squash) March 21, 2024 18:59
@jcouv jcouv merged commit 7ecf61f into dotnet:main Mar 22, 2024
24 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Mar 22, 2024
@RikkiGibson RikkiGibson removed this from the Next milestone Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The compiler doesn't flag errors in simple 'using' statements.
4 participants