Skip to content

Allow fully oblivious types to coexist with nullable-aware base types in partial classes#55861

Merged
RikkiGibson merged 7 commits intodotnet:mainfrom
RikkiGibson:partial-base-nullability
Aug 31, 2021
Merged

Allow fully oblivious types to coexist with nullable-aware base types in partial classes#55861
RikkiGibson merged 7 commits intodotnet:mainfrom
RikkiGibson:partial-base-nullability

Conversation

@RikkiGibson
Copy link
Copy Markdown
Member

@RikkiGibson RikkiGibson commented Aug 24, 2021

Fixes #45960

Some of the changes in the implementation are a bit weird here because we're adding some awareness of the difference between a top-level "non-annotated" base type and a top-level "oblivious" base type. This affects the test PartialBaseTypeDifference_02. If it is found that this doesn't make sense, we could simplify the change.

@RikkiGibson RikkiGibson requested a review from a team as a code owner August 24, 2021 19:35
@ghost ghost added the Area-Compilers label Aug 24, 2021
@AlekseyTs

This comment has been minimized.

Sign in to view
@AlekseyTs

This comment has been minimized.

Sign in to view
@jcouv jcouv self-assigned this Aug 27, 2021
@AlekseyTs

This comment has been minimized.

Sign in to view
AlekseyTs
AlekseyTs approved these changes Aug 30, 2021
View reviewed changes
Copy link
Copy Markdown
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 7)

jcouv
jcouv reviewed Aug 30, 2021
View reviewed changes
src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol_Bases.cs
baseTypeLocation = decl.NameLocation;
reportedPartialConflict = true;

static bool containsOnlyOblivious(TypeSymbol type)
Copy link
Copy Markdown
Member

@jcouv jcouv Aug 30, 2021

Choose a reason for hiding this comment

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

nit: we tend to put local functions at the end of methods. Consider moving to keep main logic flow

jcouv
jcouv approved these changes Aug 30, 2021
View reviewed changes
Copy link
Copy Markdown
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 7) with a nit/suggestion

@jcouv jcouv added the Feature - Nullable Reference Types Nullable Reference Types label Aug 30, 2021
@RikkiGibson RikkiGibson merged commit ddd7482 into dotnet:main Aug 31, 2021
@ghost ghost added this to the Next milestone Aug 31, 2021
@dibarbet dibarbet modified the milestones: Next, 17.0.P4 Aug 31, 2021
@RikkiGibson RikkiGibson deleted the partial-base-nullability branch September 8, 2021 00:16
@RikkiGibson RikkiGibson mentioned this pull request Sep 2, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlekseyTs AlekseyTs AlekseyTs approved these changes

@jcouv jcouv jcouv approved these changes

Assignees

@jcouv jcouv

Labels

Area-Compilers Feature - Nullable Reference Types Nullable Reference Types

Projects

None yet

Milestone

17.0.P4

Development

Successfully merging this pull request may close these issues.

Build fails with CS0263 when a partial class derived from a generic type is defined across different nullable contexts

4 participants

@RikkiGibson @AlekseyTs @jcouv @dibarbet