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.

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

SynthesizedReadOnlyListTypeSymbol.Create - Check for MissingMetadataTypeSymbol #71330

Merged
merged 4 commits into from
Dec 19, 2023

Conversation

alrz
Copy link
Contributor

@alrz alrz commented Dec 18, 2023

Fix #71297

@alrz alrz requested a review from a team as a code owner December 18, 2023 21:01
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 18, 2023
@ghost ghost added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Dec 18, 2023
@@ -102,8 +102,8 @@ internal static NamedTypeSymbol Create(SourceModuleSymbol containingModule, stri
DiagnosticInfo? diagnosticInfo = null;

var hasReadOnlyInterfaces =
!compilation.IsTypeMissing(SpecialType.System_Collections_Generic_IReadOnlyCollection_T)
&& !compilation.IsTypeMissing(SpecialType.System_Collections_Generic_IReadOnlyList_T);
compilation.GetSpecialType(SpecialType.System_Collections_Generic_IReadOnlyCollection_T) is not MissingMetadataTypeSymbol &&
Copy link
Contributor

@AlekseyTs AlekseyTs Dec 18, 2023

Choose a reason for hiding this comment

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

compilation.GetSpecialType(SpecialType.System_Collections_Generic_IReadOnlyCollection_T) is not MissingMetadataTypeSymbol &&

It looks like we are fixing a real bug, but not adding any unit-tests. I think we should add a unit-test that makes the problem observable. I.e. a test that fails without the fix. #Closed

@alrz
Copy link
Contributor Author

alrz commented Dec 19, 2023

@AlekseyTs @dotnet/roslyn-compiler this is ready for review. thanks.

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 4)

@AlekseyTs
Copy link
Contributor

@dotnet/roslyn-compiler For the second review

@cston cston merged commit 00d7d7e into dotnet:main Dec 19, 2023
25 checks passed
@ghost ghost added this to the Next milestone Dec 19, 2023
@cston
Copy link
Member

cston commented Dec 19, 2023

Thanks @alrz for the contribution.

333fred added a commit to 333fred/roslyn that referenced this pull request Dec 19, 2023
* upstream/main: (79 commits)
  SynthesizedReadOnlyListTypeSymbol.Create - Check for MissingMetadataTypeSymbol (dotnet#71330)
  Add comment
  Make obsolete
  Update src/Features/CSharp/Portable/SignatureHelp/LightweightOverloadResolution.cs
  Update tests
  Update tests
  Update tests
  Raw strings
  File scoped namespaces
  Add tests
  Add index check
  Add tests
  Docs
  Simplify
  Move to top level type
  Finalize
  Simplify
  In progress
  In progress
  Do not call Unsubst if it is not available due to argument processing (dotnet#71282)
  ...
@Cosifne Cosifne modified the milestones: Next, 17.9 P3 Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee. 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.

SynthesizedReadOnlyListTypeSymbol.Create should check for MissingMetadataTypeSymbol for missing types
4 participants