-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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 module initializers in VB #44227
Disallow module initializers in VB #44227
Conversation
@@ -198,6 +198,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic | |||
If arguments.Attribute.IsTargetAttribute(Me, AttributeDescription.SkipLocalsInitAttribute) Then | |||
arguments.Diagnostics.Add(ERRID.WRN_AttributeNotSupportedInVB, arguments.AttributeSyntaxOpt.Location, AttributeDescription.SkipLocalsInitAttribute.FullName) | |||
End If | |||
|
|||
If arguments.Attribute.IsTargetAttribute(Me, AttributeDescription.ModuleInitializerAttribute) Then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arguments.Attribute.IsTargetAttribute(Me, AttributeDescription.ModuleInitializerAttribute) [](start = 15, length = 90)
Should we limit this check to MethodSymbols? If I remember correctly C# ignores the attribute on anything, but a method. #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think that would be more consistent.
Done with review pass (iteration 2) #Closed |
@@ -1572,6 +1572,10 @@ lReportErrorOnTwoTokens: | |||
arguments.Diagnostics.Add(ERRID.ERR_ExplicitTupleElementNamesAttribute, arguments.AttributeSyntaxOpt.Location) | |||
End If | |||
|
|||
If arguments.Attribute.IsTargetAttribute(Me, AttributeDescription.ModuleInitializerAttribute) Then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If arguments.Attribute.IsTargetAttribute(Me, AttributeDescription.ModuleInitializerAttribute) [](start = 12, length = 93)
It looks like this check should be moved into DecodeWellKnownAttributeAppliedToMethod. Please add a test with the attribute applied to a return. #Closed
|
||
Dim comp = CreateCompilationWithMscorlib40(source) | ||
|
||
CompilationUtils.AssertTheseDiagnostics(comp, <expected></expected>) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AssertTheseDiagnostics [](start = 29, length = 22)
AssertNoDiagnostics
? To avoid creating dummy XML elements. #Closed
Done with review pass (iteration 3) #Closed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (iteration 4)
Related to #40500
Module initializers are not supported in VB. See #43281 (comment)
Tests are heavily based on the SkipLocalsInit tests in VB.