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

Implement Required Members for VB #66084

Merged
merged 22 commits into from
May 10, 2023
Merged

Implement Required Members for VB #66084

merged 22 commits into from
May 10, 2023

Conversation

333fred
Copy link
Member

@333fred 333fred commented Dec 21, 2022 •
edited by jcouv
Loading

Implements support for required members in VB. We support creating new instances of types with required members and ensuring that all required members are set in an object initializer. We do not support inheriting from a type with required members, as that would require emitting new metadata for these cases. Closes #61435.

Relates to test plan #57046

@333fred 333fred added the Feature - Required Members Required properties and fields label Dec 21, 2022
@333fred 333fred force-pushed the vb-required branch 5 times, most recently from 20e700b to f936bbc Compare December 21, 2022 20:31
@333fred
Implement remaining errors:
98167b1 
* Block inheriting from types with required members
* Block calling constructors with invalid required member lists
* Block substituting types with required members in As New, unless the parameterless ctor has SetsRequiredMembers.
@333fred
Fill out all tests
7890731
@333fred 333fred marked this pull request as ready for review December 21, 2022 22:07
@333fred 333fred requested a review from a team as a code owner December 21, 2022 22:07
@333fred
Copy link
Member Author

333fred commented Dec 21, 2022

@dotnet/roslyn-compiler @AlekseyTs this should be ready for review.

@333fred 333fred requested a review from AlekseyTs December 21, 2022 22:09
@333fred
Copy link
Member Author

333fred commented Dec 28, 2022

@dotnet/roslyn-compiler @AlekseyTs for review please.

@333fred
PR feedback
8c9cff4 
* Remove Nothing handling to align with C#.
* Remove suppressions of cascading errors.
* Expand unbound test coverage.
* Expand tuple test coverage.
@333fred
Copy link
Member Author

333fred commented Mar 22, 2023

@AlekseyTs addressed feedback.

In reply to: 1470886843

333fred added 3 commits March 22, 2023 17:11
@333fred
Pass correct containing type in local rewriting of tuples.
58072b4
@333fred
Merge remote-tracking branch 'upstream/main' into vb-required
610d5f4 
* upstream/main: (2060 commits)
  implement code folding for anonymous objects
  Cleanup/perf in creating member maps. (dotnet#67997)
  Address feedback, clean the use of ILegacyGlobalOptionsWorkspaceService
  Update Publish.json
  Semantic snippets: Add inline statement snippets (dotnet#67819)
  Update VSSDK Build tools version
  Update doc comment
  Address feedback
  Move declaration near reference
  Address feedback
  Avoid stack overflow due to deep recursion on long chain of calls. (dotnet#67913)
  Check ILegacyGlobalOptionsWorkspaceService is null
  test scout queue
  add test
  update editor
  "Where" clause typo fix (dotnet#68002)
  Use `Keyword` helper method instead of hardcoding keyword help terms
  Move the SyntaxTree and SemanticModel action based analyzers to respect context.FilterSpan
  EnC refactoring: Align wrappers with contracts better (dotnet#67967)
  Include EA.RazorCompiler in source build (dotnet#67996)
  ...
@333fred
Fix test baseline
e1e3866
@AlekseyTs
Copy link
Contributor

Done with review pass (commit 17)

AlekseyTs
AlekseyTs approved these changes May 7, 2023
View reviewed changes
Copy link
Contributor

@AlekseyTs AlekseyTs 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 19)

jcouv
jcouv reviewed May 10, 2023
View reviewed changes
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

Done with review pass (iteration 19)

@jcouv jcouv self-assigned this May 10, 2023
jcouv
jcouv approved these changes May 10, 2023
View reviewed changes
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 22)

AlekseyTs
AlekseyTs approved these changes May 10, 2023
View reviewed changes
Copy link
Contributor

@AlekseyTs AlekseyTs 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 22)

@333fred 333fred merged commit 974bb4e into dotnet:main May 10, 2023
@333fred 333fred deleted the vb-required branch May 10, 2023 20:13
@allisonchou allisonchou mentioned this pull request May 12, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcouv jcouv jcouv approved these changes

@AlekseyTs AlekseyTs AlekseyTs approved these changes

Assignees

@jcouv jcouv

Labels
Area-Compilers Feature - Required Members Required properties and fields
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Required Members VB Support
3 participants
@333fred @AlekseyTs @jcouv