Skip to content

VBOverloadResolutionPriority: break attribute binding cycles, clone a bunch of C# tests#76008

Merged
AlekseyTs merged 2 commits intodotnet:features/VBOverloadResolutionPriorityfrom
AlekseyTs:OverloadPriority_04
Nov 23, 2024
Merged

VBOverloadResolutionPriority: break attribute binding cycles, clone a bunch of C# tests#76008
AlekseyTs merged 2 commits intodotnet:features/VBOverloadResolutionPriorityfrom
AlekseyTs:OverloadPriority_04

Conversation

@AlekseyTs
Copy link
Contributor

No description provided.

@AlekseyTs AlekseyTs requested review from 333fred and cston November 21, 2024 15:29
@AlekseyTs AlekseyTs requested a review from a team as a code owner November 21, 2024 15:29
@ghost ghost added the untriaged Issues and PRs which have not yet been triaged by a lead label Nov 21, 2024
@AlekseyTs
Copy link
Contributor Author

@cston, @333fred, @dotnet/roslyn-compiler Please review

@AlekseyTs
Copy link
Contributor Author

@cston, @dotnet/roslyn-compiler For the second review

1 similar comment
@AlekseyTs
Copy link
Contributor Author

@cston, @dotnet/roslyn-compiler For the second review

throw DirectCast(Nothing, System.Exception)
End Sub
"

Copy link
Contributor

@cston cston Nov 23, 2024

Choose a reason for hiding this comment

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

It looks like these are not used. #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like these are not used.

Looks this way. Will remove.


Dim compilation = CreateCompilation({source, OverloadResolutionPriorityAttributeDefinitionVB}, options:=TestOptions.DebugExe)

' Note, VB silently ignores attributes at wrong location
Copy link
Contributor

@cston cston Nov 23, 2024

Choose a reason for hiding this comment

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

Note, VB silently ignores attributes at wrong location

Do we need a test for C# consuming such cases? #Closed

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like there is such a test already: Overrides_ChangePriorityInMetadata_Methods.

"

Dim compilation = CreateCompilation(source, options:=TestOptions.DebugExe)
compilation.AssertTheseEmitDiagnostics(
Copy link
Contributor

@cston cston Nov 23, 2024

Choose a reason for hiding this comment

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

compilation.AssertTheseEmitDiagnostics

The equivalent C# test avoids verifying diagnostics, presumably to increase the chance of a cycle in attribute binding when using the semantic model. Is that not useful here? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is that not useful here?

It is done below. The compilation is recreated after we check diagnostics

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Compilers Feature - Overload Resolution Priority 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.

3 participants