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.

Sign up for GitHub

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

Jump to bottom

VB support for CallerArgumentExpression #54132

Merged
merged 47 commits into from
Jun 29, 2021
Merged

VB support for CallerArgumentExpression #54132

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
c776485
Partial VB support for CallerArgumentExpression
Youssef1313 Jun 16, 2021
b75f8bc
Little more work
Youssef1313 Jun 16, 2021
83a2715
Small fix
Youssef1313 Jun 16, 2021
8468010
PROTOTYPEs from feedback to address later
Youssef1313 Jun 16, 2021
914e2cd
Attempt to handle VB extension methods
Youssef1313 Jun 16, 2021
55722d5
Delete prototype
Youssef1313 Jun 16, 2021
d8bdb68
Test
Youssef1313 Jun 16, 2021
85f6d49
Small fixes
Youssef1313 Jun 17, 2021
7d4aa11
Fixes and more tests
Youssef1313 Jun 17, 2021
8257cc4
Chain VerifyDiagnostics on CompileAndVerify
Youssef1313 Jun 18, 2021
2c45646
Merge remote-tracking branch 'upstream/features/caller-argument-expre…
Youssef1313 Jun 18, 2021
19a9502
Address more of the feedback
Youssef1313 Jun 18, 2021
2589254
Add unintentionally deleted test
Youssef1313 Jun 18, 2021
28d4885
Fix
Youssef1313 Jun 18, 2021
07dd639
Add prototype
Youssef1313 Jun 18, 2021
6846749
Restore blank line
Youssef1313 Jun 18, 2021
e95117c
Restore blank line
Youssef1313 Jun 18, 2021
b61e9aa
Few more tests
Youssef1313 Jun 18, 2021
93f127d
Com parameter
Youssef1313 Jun 19, 2021
fb0ff4b
Move down and add test
Youssef1313 Jun 19, 2021
b6205dd
Fix tests
Youssef1313 Jun 19, 2021
40394c1
Fix test
Youssef1313 Jun 19, 2021
2e28133
Restore indentation
Youssef1313 Jun 19, 2021
0808b08
remove outdated prototype
Youssef1313 Jun 19, 2021
003ae5f
Check feature availability
Youssef1313 Jun 19, 2021
5e99aa7
Move from wrapped parameter symbol to derived types
Youssef1313 Jun 19, 2021
28631e8
Fix implementation and add test for extension error case
Youssef1313 Jun 19, 2021
dd6ddf6
Fix tests
Youssef1313 Jun 19, 2021
2b15627
Update comment and fix test
Youssef1313 Jun 19, 2021
bd8170e
Warn for self-referential
Youssef1313 Jun 19, 2021
ad1a6ca
Warn for incorrect parameter name
Youssef1313 Jun 19, 2021
db8e72c
More tests
Youssef1313 Jun 20, 2021
f3052c4
Fix test
Youssef1313 Jun 20, 2021
a4769eb
abstract SourceClonedParameterSymbol, tests, and some of the other fe…
Youssef1313 Jun 22, 2021
8893361
Fix build
Youssef1313 Jun 22, 2021
35e9813
Address feedback
Youssef1313 Jun 23, 2021
d25d7ac
Update tests
Youssef1313 Jun 23, 2021
e663cf3
Revert bad change
Youssef1313 Jun 23, 2021
13ddcbd
Fix assertion to ignore DummySyntaxTree
Youssef1313 Jun 23, 2021
6b9426c
Metadata case insensitive test, unreachable path, use AssertTheseDiag…
Youssef1313 Jun 23, 2021
b57907e
Add more tests as requested
Youssef1313 Jun 23, 2021
ccc7c2d
Address feedback
Youssef1313 Jun 24, 2021
4281bff
Address feedback on tests
Youssef1313 Jun 25, 2021
b935e01
Fix test
Youssef1313 Jun 25, 2021
d28d029
Address feedback
Youssef1313 Jun 26, 2021
f9a4bdd
Fix test
Youssef1313 Jun 26, 2021
5e96858
Back to case-insensitive behavior
Youssef1313 Jun 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/Compilers/Core/Portable/Symbols/Attributes/CommonParameterEarlyWellKnownAttributeData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,6 @@ public bool HasCallerMemberNameAttribute
}
}

// PROTOTYPE(caller-expr): If VB won't be supported, this should be moved into a C#-specific type.

private int _argumentExpressionParameterIndex = -1;
public int CallerArgumentExpressionParameterIndex
{
Expand Down
26 changes: 21 additions & 5 deletions src/Compilers/VisualBasic/Portable/Binding/Binder_Invocation.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2424,7 +2424,7 @@ ProduceBoundNode:
' Deal with Optional arguments
' Need to handle optional arguments here, there could be conversion errors, etc.

argument = GetArgumentForParameterDefaultValue(param, node, diagnostics, callerInfoOpt)
argument = GetArgumentForParameterDefaultValue(param, node, diagnostics, callerInfoOpt, parameterToArgumentMap, arguments)
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved

If argument Is Nothing Then
If Not includeMethodNameInErrorMessages Then
Expand Down Expand Up @@ -3098,7 +3098,12 @@ ProduceBoundNode:

End Sub

Friend Function GetArgumentForParameterDefaultValue(param As ParameterSymbol, syntax As SyntaxNode, diagnostics As BindingDiagnosticBag, callerInfoOpt As SyntaxNode) As BoundExpression
Friend Function GetArgumentForParameterDefaultValue(param As ParameterSymbol,
syntax As SyntaxNode,
diagnostics As BindingDiagnosticBag,
callerInfoOpt As SyntaxNode,
parameterToArgumentMap As ArrayBuilder(Of Integer),
arguments As ImmutableArray(Of BoundExpression)) As BoundExpression
Dim defaultArgument As BoundExpression = Nothing

' See Section 3 of §11.8.2 Applicable Methods
Expand All @@ -3114,8 +3119,9 @@ ProduceBoundNode:
Dim isCallerLineNumber As Boolean = param.IsCallerLineNumber
Dim isCallerMemberName As Boolean = param.IsCallerMemberName
Dim isCallerFilePath As Boolean = param.IsCallerFilePath
Dim callerArgumentExpressionParameterIndex As Integer = param.CallerArgumentExpressionParameterIndex

If isCallerLineNumber OrElse isCallerMemberName OrElse isCallerFilePath Then
If isCallerLineNumber OrElse isCallerMemberName OrElse isCallerFilePath OrElse callerArgumentExpressionParameterIndex > -1 Then
Dim callerInfoValue As ConstantValue = Nothing

If isCallerLineNumber Then
Expand Down Expand Up @@ -3149,9 +3155,19 @@ ProduceBoundNode:
If container IsNot Nothing AndAlso container.Name IsNot Nothing Then
callerInfoValue = ConstantValue.Create(container.Name)
End If
Else
Debug.Assert(isCallerFilePath)
ElseIf isCallerFilePath Then
callerInfoValue = ConstantValue.Create(callerInfoOpt.SyntaxTree.GetDisplayPath(callerInfoOpt.Span, Me.Compilation.Options.SourceReferenceResolver))
Else
Debug.Assert(callerArgumentExpressionParameterIndex > -1)
Dim argumentIndex = parameterToArgumentMap(callerArgumentExpressionParameterIndex)

' PROTOTYPE(caller-expr): It's unlikely this assumption is correct. Keep for now until a test hits it.
Debug.Assert(argumentIndex > -1 AndAlso argumentIndex < arguments.Length)

' PROTOTYPE(caller-expr): ToStringWithoutTrivia
' PROTOTYPE(caller-expr): Check feature availability
' PROTOTYPE(caller-expr): Tests
callerInfoValue = ConstantValue.Create(arguments(argumentIndex).Syntax.ToString())
End If

If callerInfoValue IsNot Nothing Then
Expand Down
31 changes: 31 additions & 0 deletions src/Compilers/VisualBasic/Portable/Symbols/Metadata/PE/PEParameterSymbol.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols.Metadata.PE
Private _lazyHasCallerMemberNameAttribute As ThreeState = ThreeState.Unknown
Private _lazyHasCallerFilePathAttribute As ThreeState = ThreeState.Unknown

''' <summary>
''' The index of a CallerArgumentExpression. The value -2 means uninitialized, -1 means
''' Not found. Otherwise, the index of the CallerArgumentExpression.
''' </summary>
Private _lazyCallerArgumentExpressionParameterIndex As Integer = -2
Copy link
Member

@333fred 333fred Jun 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

-2

Consider making this a constant, and not -2 since we already use -2 for reduced extension method receivers. #Closed


Private _lazyIsParamArray As ThreeState

''' <summary>
Expand Down Expand Up @@ -174,6 +180,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols.Metadata.PE
_lazyHasCallerLineNumberAttribute = ThreeState.False
_lazyHasCallerMemberNameAttribute = ThreeState.False
_lazyHasCallerFilePathAttribute = ThreeState.False
_lazyCallerArgumentExpressionParameterIndex = -1
_lazyIsParamArray = ThreeState.False
Else
Try
Expand Down Expand Up @@ -605,6 +612,30 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols.Metadata.PE
End Get
End Property

Friend Overrides ReadOnly Property CallerArgumentExpressionParameterIndex As Integer
Get
If _lazyCallerArgumentExpressionParameterIndex = -2 Then
Debug.Assert(Not _handle.IsNil)

Dim attribute = PEModule.FindTargetAttribute(_handle, AttributeDescription.CallerArgumentExpressionAttribute)
Dim parameterName As String = Nothing
If attribute.HasValue AndAlso PEModule.TryExtractStringValueFromAttribute(attribute.Handle, parameterName) Then
Dim parameters = ContainingSymbol.GetParameters()
For i = 0 To parameters.Length - 1
If parameters(i).Name = parameterName Then
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
_lazyCallerArgumentExpressionParameterIndex = i
Exit For
End If
Next
Else
_lazyCallerArgumentExpressionParameterIndex = -1
End If
End If

Return _lazyCallerArgumentExpressionParameterIndex
End Get
End Property

''' <remarks>
''' This is for perf, not for correctness.
''' </remarks>
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/VisualBasic/Portable/Symbols/ParameterSymbol.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols

Friend MustOverride ReadOnly Property IsCallerFilePath As Boolean

Friend MustOverride ReadOnly Property CallerArgumentExpressionParameterIndex As Integer
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved

Protected Overrides ReadOnly Property HighestPriorityUseSiteError As Integer
Get
Return ERRID.ERR_UnsupportedType1
Expand Down
6 changes: 6 additions & 0 deletions src/Compilers/VisualBasic/Portable/Symbols/ReducedExtensionMethodSymbol.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
End Get
End Property

Friend Overrides ReadOnly Property CallerArgumentExpressionParameterIndex As Integer
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
Get
Return m_CurriedFromParameter.CallerArgumentExpressionParameterIndex
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
End Get
End Property

Public Overrides ReadOnly Property HasExplicitDefaultValue As Boolean
Get
Return m_CurriedFromParameter.HasExplicitDefaultValue
Expand Down
6 changes: 6 additions & 0 deletions src/Compilers/VisualBasic/Portable/Symbols/Retargeting/RetargetingParameterSymbol.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols.Retargeting
End Get
End Property

Friend Overrides ReadOnly Property CallerArgumentExpressionParameterIndex As Integer
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
Get
Return _underlyingParameter.CallerArgumentExpressionParameterIndex
End Get
End Property

Public Overrides ReadOnly Property ContainingSymbol As Symbol
Get
Return RetargetingTranslator.Retarget(_underlyingParameter.ContainingSymbol)
Expand Down
6 changes: 6 additions & 0 deletions src/Compilers/VisualBasic/Portable/Symbols/SignatureOnlyParameterSymbol.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
Throw ExceptionUtilities.Unreachable
End Get
End Property

Friend Overrides ReadOnly Property CallerArgumentExpressionParameterIndex As Integer
Get
Throw ExceptionUtilities.Unreachable
End Get
End Property
#End Region
End Class
End Namespace
6 changes: 6 additions & 0 deletions src/Compilers/VisualBasic/Portable/Symbols/Source/LambdaParameterSymbol.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
End Get
End Property

Friend Overrides ReadOnly Property CallerArgumentExpressionParameterIndex As Integer
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
Get
Return -1
End Get
End Property

Public NotOverridable Overrides ReadOnly Property IsParamArray As Boolean
Get
Return False
Expand Down
6 changes: 6 additions & 0 deletions src/Compilers/VisualBasic/Portable/Symbols/Source/MeParameterSymbol.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
End Get
End Property

Friend Overrides ReadOnly Property CallerArgumentExpressionParameterIndex As Integer
Get
Return -1
End Get
End Property

Public Overrides ReadOnly Property IsMe As Boolean
Get
Return True
Expand Down
6 changes: 6 additions & 0 deletions src/Compilers/VisualBasic/Portable/Symbols/Source/SourceClonedParameterSymbol.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
End Get
End Property

Friend Overrides ReadOnly Property CallerArgumentExpressionParameterIndex As Integer
Get
Return _originalParam.CallerArgumentExpressionParameterIndex
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

CallerArgumentExpressionParameterIndex

This looks fragile and probably incorrect, at least for delegate EndInvoke method. This implementation assumes that the target parameter is still going to be present and will remain at the same position, which probably can be wrong. The more robust approach is to reinterpret the attribute in the new signature. But before we do this, let's create some tests and figure out if this code path is even reachable in interesting scenarios. Perhaps simply returningh -1 is going to be good enough. #Closed

Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, I bet C# has similar class, we should check it as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

@AlekseyTs EndInvoke should only be cloning parameters that are ref or out:

roslyn/src/Compilers/CSharp/Portable/Symbols/Source/SourceDelegateMethodSymbol.cs

Lines 402 to 409 in 22a7203

foreach (SourceParameterSymbol p in invoke.Parameters)
{
if (p.RefKind != RefKind.None)
{
var synthesizedParam = new SourceClonedParameterSymbol(originalParam: p, newOwner: this, newOrdinal: ordinal++, suppressOptional: true);
parameters.Add(synthesizedParam);
}
}

So it is not going to ever clone a caller argument expression parameter. Hence, this path is unreachable for an EndInvoke. However, the implementation is needed for indexers and delegate invoke methods.

Copy link
Contributor

Choose a reason for hiding this comment

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

So it is not going to ever clone a caller argument expression parameter.

Why do you think so? Is there a test that demonstrates that?

Copy link
Contributor

@AlekseyTs AlekseyTs Jun 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

However, the implementation is needed for indexers and delegate invoke methods.

I see nothing in this type that limits its application to these scenarios. Therefore, the implementation is not robust in the long term in my opinion.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why do you think so?

Per the code above that creates SourceClonedParameterSymbol for EndInvoke, a parameter is only created if its RefKind == RefKind.None.

CallerArgumentExpressionAttribute is only allowed for optional parameters (which should have RefKind.None.

Applying the attribute to a ref or out parameter generates a compile-time error:

error CS9006: The CallerArgumentExpressionAttribute may only be applied to parameters with default values

These are just my thoughts, I may be missing some interesting cases that you may have.

Copy link
Contributor

Choose a reason for hiding this comment

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

CallerArgumentExpressionAttribute is only allowed for optional parameters (which should have RefKind.None.

Is this accurate? One cannot do this in IL or VB?

Applying the attribute to a ref or out parameter generates a compile-time error:

error CS9006: The CallerArgumentExpressionAttribute may only be applied to parameters with default values

Why are we talking about C# error, when we are working in VB? What prevents applying this attribute in IL for example?

This is perfectly valid code:

class Test
    Shared Sub M(Optional ByRef x as String = "ddd")
    End Sub
    
    Shared Sub Test()
        M()
    End SUb
End Class

Copy link
Contributor

Choose a reason for hiding this comment

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

I see nothing in this type that limits its application to these scenarios. Therefore, the implementation is not robust in the long term in my opinion.

One way out could be making this type MustInherit, creating specialized NotInheritable derived types for specific scenarios, implementing the API in the derived types only.

Copy link
Member Author

Choose a reason for hiding this comment

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

@AlekseyTs

Behavioral-wise:

  • VB delegates doesn't allow optional parameter, hence, calling CallerArgumentExpressionIndex should never be done for EndInvoke invocations (the binder calls this property only if we're binding an optional parameter).

  • For IL, shouldn't we be dealing with metadata symbols, not source symbols? or I'm misunderstanding the concept here?

Implementation-wise:

I'm okay with making the type abstract and introducing derived type.

Copy link
Contributor

@AlekseyTs AlekseyTs Jun 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I'm misunderstanding the concept here?

The discussion like that can go forever and will never end because reviewer's concerns can almost never be addressed by gueses, speculations or asumptions. The best way to alleviate reviewer's concerns is to create a test for the scenario in question and demonstrate the behavior, in this case the behavior of the API. Specifically here we are discussing an API behavior, whether there is an error reported or not has very little impact to the question what API's behavior is and what it should be. We are not discussing an issue with lowering/code generation. Since the type is not specific to any single particular scenario (a unique reason for creating a clone), each scenario should be tested individually and the tests should demonstrate that the implementation is in fact necessary and correct. Neccessity can be demonstrated by observing a break for a successful scenario when the implementation is removed/changed. For example, you are saying: "the implementation is needed for indexers and delegate invoke methods." Please demonstrate the fact with unit tests.

I'm okay with making the type abstract and introducing derived type.

I would prefer that we will create more specialized derived types. This way one can reason when instances are created and how they are used, can confirm that the code path is tested. Today anyone can create an instance of SourceClonedParameterSymbol for whatever reason and "take" an implementation of this API, whether appropriate or not, with it without even be aware of the possible impact. I think we should prevent this from happening.

End Get
End Property

Public Overrides ReadOnly Property IsByRef As Boolean
Get
Return _originalParam.IsByRef
Expand Down
13 changes: 13 additions & 0 deletions src/Compilers/VisualBasic/Portable/Symbols/Source/SourceComplexParameterSymbol.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,19 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
End Get
End Property

Friend Overrides ReadOnly Property CallerArgumentExpressionParameterIndex As Integer
Get
Dim attributeSource As SourceParameterSymbol = If(Me.BoundAttributesSource, Me)

Dim data = attributeSource.GetEarlyDecodedWellKnownAttributeData()
If data Is Nothing Then
Return -1
End If

Return data.CallerArgumentExpressionParameterIndex
End Get
End Property

''' <summary>
''' Is parameter explicitly declared ByRef. Can be different from IsByRef only for
''' String parameters of Declare methods.
Expand Down
10 changes: 10 additions & 0 deletions src/Compilers/VisualBasic/Portable/Symbols/Source/SourceNamedTypeSymbol_ComClass.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1576,6 +1576,16 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
End Get
End Property

Friend Overrides ReadOnly Property CallerArgumentExpressionParameterIndex As Integer
Get
If IsComEventParameter Then
Return -1
End If

Return _clonedFrom.CallerArgumentExpressionParameterIndex
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

CallerArgumentExpressionParameterIndex

Why is this the right value? Ae we testing this code path? #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

@AlekseyTs I don't have enough info about COM, is it expected that order of parameters can change, etc?

Trying to test this code path locally with the following test, but there are few errors:

        <Fact>
        Public Sub ComClass()
            Dim source As String = "
Imports Microsoft.VisualBasic

<ComClass(ComClass1.ClassId, ComClass1.InterfaceId, ComClass1.EventsId)>
Public Class ComClass1
    ' Use the Region directive to define a section named COM Guids. 
#Region ""COM GUIDs""
    ' These  GUIDs provide the COM identity for this class 
    ' and its COM interfaces. You can generate 
    ' these guids using guidgen.exe
    Public Const ClassId As String = ""7666AC25-855F-4534-BC55-27BF09D49D46""
    Public Const InterfaceId As String = ""54388137-8A76-491e-AA3A-853E23AC1217""
    Public Const EventsId As String = ""EA329A13-16A0-478d-B41F-47583A761FF2""
#End Region

    Public Sub New()
        MyBase.New()
    End Sub

    Public Sub M(x As Integer, Optional y As String = """")
    End Sub
End Class

Module Program
    Sub Main()
        Dim comObj As New ComClass1()
        comObj.M(1+  0)
    End Sub
End Module
"
            Dim compilation = CreateCompilation(source, targetFramework:=TargetFramework.NetCoreApp, references:={Net451.MicrosoftVisualBasic}, options:=TestOptions.ReleaseExe, parseOptions:=TestOptions.RegularLatest)
            CompileAndVerify(compilation, expectedOutput:="").VerifyDiagnostics() ' PROTOTYPE(caller-expr): Figure out how to fix these:
            ' (5) : error BC35000: Requested operation is not available because the runtime library function 'System.Runtime.InteropServices.GuidAttribute..ctor' is not defined.
            ' (5) : error BC35000: Requested operation is not available because the runtime library function 'System.Runtime.InteropServices.ClassInterfaceAttribute..ctor' is not defined.
            ' (5) : error BC35000: Requested operation is not available because the runtime library function 'System.Runtime.InteropServices.DispIdAttribute..ctor' is not defined.
        End Sub

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have enough info about COM, is it expected that order of parameters can change, etc?

I do not remember these details. Since you are the PR author, it is on you to "defend" the changes. I would start with reviewing parts of the code that create symbols like this and analyze how and in what fashion this is happening. For tests, I would try to locate tests that are targeting this feature and would try to clone/adjust them as appropriate.

Copy link
Member Author

Choose a reason for hiding this comment

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

@AlekseyTs I experimented with this. As far as I can tell, the SynthesizedComParameters are only exposed to emitter. It's not exposed to binder at all. It feels safe to just throw ExceptionUtilities.Unreachable if my conclusions are correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

I experimented with this. As far as I can tell, the SynthesizedComParameters are only exposed to emitter. It's not exposed to binder at all. It feels safe to just throw ExceptionUtilities.Unreachable if my conclusions are correct.

Looks this way to me too.

End Get
End Property

Public Overrides ReadOnly Property IsParamArray As Boolean
Get
If IsComEventParameter Then
Expand Down
27 changes: 20 additions & 7 deletions src/Compilers/VisualBasic/Portable/Symbols/Source/SourceParameterSymbol.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,10 @@
' The .NET Foundation licenses this file to you under the MIT license.
' See the LICENSE file in the project root for more information.

Imports System.Collections.Generic
Imports System.Collections.Immutable
Imports System.Collections.ObjectModel
Imports System.Runtime.InteropServices
Imports System.Threading
Imports Microsoft.CodeAnalysis.CodeGen
Imports Microsoft.CodeAnalysis.Text
Imports Microsoft.CodeAnalysis.VisualBasic.Binder
Imports Microsoft.CodeAnalysis.VisualBasic.Symbols
Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
Imports TypeKind = Microsoft.CodeAnalysis.TypeKind

Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
Friend MustInherit Class SourceParameterSymbol
Expand Down Expand Up @@ -249,6 +242,24 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
arguments.GetOrCreateData(Of ParameterEarlyWellKnownAttributeData).HasCallerFilePathAttribute = True
ElseIf VisualBasicAttributeData.IsTargetEarlyAttribute(arguments.AttributeType, arguments.AttributeSyntax, AttributeDescription.CallerMemberNameAttribute) Then
arguments.GetOrCreateData(Of ParameterEarlyWellKnownAttributeData).HasCallerMemberNameAttribute = True
ElseIf VisualBasicAttributeData.IsTargetEarlyAttribute(arguments.AttributeType, arguments.AttributeSyntax, AttributeDescription.CallerArgumentExpressionAttribute) Then
Dim hasDiagnostics As Boolean = False
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
Dim index = -1
Dim attribute = arguments.Binder.GetAttribute(arguments.AttributeSyntax, arguments.AttributeType, hasDiagnostics)
If Not attribute.HasErrors Then
Dim parameterName As String = Nothing
If attribute.ConstructorArguments.Single().TryDecodeValue(SpecialType.System_String, parameterName) Then
Dim parameters = containingSymbol.GetParameters()
For i = 0 To parameters.Length - 1
If parameters(i).Name = parameterName Then
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
index = i
Exit For
End If
Next
End If
End If

arguments.GetOrCreateData(Of ParameterEarlyWellKnownAttributeData).CallerArgumentExpressionParameterIndex = index
End If

Return MyBase.EarlyDecodeWellKnownAttribute(arguments)
Expand Down Expand Up @@ -286,6 +297,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
Debug.Assert(arguments.SymbolPart = AttributeLocation.None)
Debug.Assert(TypeOf arguments.Diagnostics Is BindingDiagnosticBag)

' PROTOTYPE(caller-expr): Warn for self-referential and invalid parameter name to CallerArgumentExpressionAttribute.

' Differences from C#:
'
' DefaultParameterValueAttribute
Expand Down
11 changes: 11 additions & 0 deletions src/Compilers/VisualBasic/Portable/Symbols/Source/SourceSimpleParameterSymbol.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,17 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
End Get
End Property

Friend Overrides ReadOnly Property CallerArgumentExpressionParameterIndex As Integer
Get
Dim data = GetEarlyDecodedWellKnownAttributeData()
If data Is Nothing Then
Return -1
End If

Return data.CallerArgumentExpressionParameterIndex
End Get
End Property

Friend Overrides ReadOnly Property IsExplicitByRef As Boolean
Get
' SourceSimpleParameterSymbol is never created for a parameter with ByRef modifier.
Expand Down
6 changes: 6 additions & 0 deletions src/Compilers/VisualBasic/Portable/Symbols/SubstitutedParameterSymbol.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
End Get
End Property

Friend Overrides ReadOnly Property CallerArgumentExpressionParameterIndex As Integer
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
Get
Return _originalDefinition.CallerArgumentExpressionParameterIndex
End Get
End Property

Public Overrides ReadOnly Property RefCustomModifiers As ImmutableArray(Of CustomModifier)
Get
Return TypeSubstitution.SubstituteCustomModifiers(_originalDefinition.RefCustomModifiers)
Expand Down
15 changes: 15 additions & 0 deletions src/Compilers/VisualBasic/Portable/Symbols/SymbolExtensions.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,21 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
End Select
End Function

''' <summary>
''' Returns the parameters of a given method or property.
''' </summary>
<Extension()>
Friend Function GetParameters(sym As Symbol) As ImmutableArray(Of ParameterSymbol)
Select Case sym.Kind
Case SymbolKind.Method
Return DirectCast(sym, MethodSymbol).Parameters
Case SymbolKind.Property
Return DirectCast(sym, PropertySymbol).Parameters
Case Else
Return ImmutableArray(Of ParameterSymbol).Empty
End Select
End Function

<Extension()>
Friend Function OfMinimalArity(symbols As IEnumerable(Of NamespaceOrTypeSymbol)) As NamespaceOrTypeSymbol
Dim minAritySymbol As NamespaceOrTypeSymbol = Nothing
Expand Down
6 changes: 6 additions & 0 deletions src/Compilers/VisualBasic/Portable/Symbols/SynthesizedSymbols/SynthesizedParameterSymbol.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
End Get
End Property

Friend Overrides ReadOnly Property CallerArgumentExpressionParameterIndex As Integer
Get
Return -1
End Get
End Property

''' <summary>
''' True if the argument value must be included in the marshalled arguments passed to a remote callee only if it is different from the default value (if there is one).
''' </summary>
Expand Down
6 changes: 6 additions & 0 deletions src/Compilers/VisualBasic/Portable/Symbols/Wrapped/WrappedParameterSymbol.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
End Get
End Property

Friend Overrides ReadOnly Property CallerArgumentExpressionParameterIndex As Integer
Get
Return Me._underlyingParameter.CallerArgumentExpressionParameterIndex
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
End Get
End Property

Friend Overrides ReadOnly Property IsExplicitByRef As Boolean
Get
Return Me._underlyingParameter.IsExplicitByRef
Expand Down