Skip to content

Handle interaction of VB with Static Abstract Members in interfaces#56375

Merged
AlekseyTs merged 2 commits intodotnet:mainfrom
AlekseyTs:VBStaticAbstract
Sep 17, 2021
Merged

Handle interaction of VB with Static Abstract Members in interfaces#56375
AlekseyTs merged 2 commits intodotnet:mainfrom
AlekseyTs:VBStaticAbstract

Conversation

@AlekseyTs
Copy link
Copy Markdown
Contributor

No description provided.

<value>The CallerArgumentExpressionAttribute applied to parameter will have no effect. It is applied with an invalid parameter name.</value>
</data>
<data name="ERR_BadAbstractStaticMemberAccess" xml:space="preserve">
<value>A static abstract interface member cannot be accessed.</value>
Copy link
Copy Markdown
Member

@333fred 333fred Sep 14, 2021

Choose a reason for hiding this comment

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

static abstract

Should we use Shared MustInherit here? And same for the abstract in the message below? #Resolved

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should we use Shared MustInherit here? And same for the abstract in the message below?

I don't think using MustInherit term is appropriate here because the language doesn't have such concept for interface members. I could replace static with shared for consistency though. Would that address the concern?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Alright, as long as we're consistent between the error messages I'm ok.

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Copy Markdown
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

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

LGTM (commit 1), with one minor wording comment.

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-compiler For the second review.

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-compiler For the second review.

2 similar comments
@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-compiler For the second review.

@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-compiler For the second review.

@jaredpar
Copy link
Copy Markdown
Member

@RikkiGibson PTAL

System.Console.WriteLine(NameOf(x.E1))
~~~~
</expected>)
AssertNoDiagnostics(comp)
Copy link
Copy Markdown
Member

@RikkiGibson RikkiGibson Sep 17, 2021

Choose a reason for hiding this comment

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

Was this just changed incidentally in order to make events within NameOf more consistent with other kinds of member accesses? #Resolved

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Was this just changed incidentally in order to make events within NameOf more consistent with other kinds of member accesses?

This was changed intentionally for the mentioned reason.

@AlekseyTs AlekseyTs merged commit e3756cc into dotnet:main Sep 17, 2021
@ghost ghost added this to the Next milestone Sep 17, 2021
@Cosifne Cosifne modified the milestones: Next, 17.0.P5 Sep 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants