Filter out CompilerFeatureRequired and Obsolete attribute from method symbols #78313
Conversation
… symbols In C#, we filter out CompilerFeatureRequired and Obsolete attributes from GetAttribute calls when we understand their meaning, instead of surfacing them in the public API. This can have downstream effects in the IDE, such as appearing as though these constructors are deprecated. We now do the same in VB. Closes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2436215.
ghost
added
Area-Compilers
untriaged
| Dim attributeData As ImmutableArray(Of VisualBasicAttributeData) = Nothing | ||
| Dim containingPEModuleSymbol = DirectCast(ContainingModule(), PEModuleSymbol) | ||
| containingPEModuleSymbol.LoadCustomAttributes(Me.Handle, attributeData) | ||
| Dim checkForRequiredMembers = MethodKind = MethodKind.Constructor AndAlso |
There was a problem hiding this comment.
See https://github.com/dotnet/roslyn/blob/main/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs#L1012-L1030 for the equivalent C# version of this. I based this logic on the current C# code.
|
|
||
| Dim comp = CreateCompilation(vbCode, {cComp.EmitToImageReference()}) | ||
| comp.AssertNoDiagnostics | ||
| Dim comp = CreateCompilation(vbCode, {cComp.EmitToImageReference()}, targetFramework:=TargetFramework.Net70) |
There was a problem hiding this comment.
Turns out some of these compilations were not targeting a version of .NET with SetsRequiredMembers in it, which was then giving missing symbol text in ToTestDisplayString().
| attributeData = containingPEModuleSymbol.GetCustomAttributesForToken( | ||
| Me.Handle, | ||
| filteredOutAttribute1:=discard1, | ||
| filterOut1:=If(compilerFeatureRequiredDiagnostic Is Nothing, AttributeDescription.CompilerFeatureRequiredAttribute, Nothing), |
There was a problem hiding this comment.
No, I didn't cook up some invalid IL test here. If you'd prefer that I do so, I can.
There was a problem hiding this comment.
I assume we have a test for C#, it feels like it shouldn't be very hard to port it.
There was a problem hiding this comment.
We didn't, but now we do.
|
@dotnet/roslyn-compiler for a second review |
|
@dotnet/roslyn-compiler for a second review. |
| filteredOutAttribute1:=discard1, | ||
| filterOut1:=If(compilerFeatureRequiredDiagnostic Is Nothing, AttributeDescription.CompilerFeatureRequiredAttribute, Nothing), | ||
| filteredOutAttribute2:=discard2, | ||
| filterOut2:=If(ObsoleteAttributeData Is Nothing, AttributeDescription.ObsoleteAttribute, Nothing)) |
There was a problem hiding this comment.
4 participants
In C#, we filter out CompilerFeatureRequired and Obsolete attributes from GetAttribute calls when we understand their meaning, instead of surfacing them in the public API. This can have downstream effects in the IDE, such as appearing as though these constructors are deprecated. We now do the same in VB. Closes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2436215.