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

Honor abstract required during state initialization in nullable #76363

Merged
merged 4 commits into from
Dec 16, 2024
Merged

Honor abstract required during state initialization in nullable #76363

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
848ec10
Do honor abstract `required` during state initialization in nullable
333fred Dec 12, 2024
6777ba9
Add additionally requested tests
333fred Dec 13, 2024
66794f8
Add one more test
333fred Dec 13, 2024
81d2864
Expand comment.
333fred Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 71 additions & 12 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -910,13 +910,28 @@ static ImmutableArray<Symbol> membersToBeInitialized(NamedTypeSymbol containingT
=> ImmutableArray<Symbol>.Empty,

(includeAllMembers: false, includeCurrentTypeRequiredMembers: true, includeBaseRequiredMembers: false)
=> containingType.GetMembersUnordered().SelectManyAsArray(predicate: SymbolExtensions.IsRequired, selector: getAllMembersToBeDefaulted),
=> containingType.GetMembersUnordered().SelectManyAsArray(
predicate: SymbolExtensions.IsRequired,
selector: symbol => getAllMembersToBeDefaulted(symbol, filterOverridingProperties: true)),

(includeAllMembers: false, includeCurrentTypeRequiredMembers: true, includeBaseRequiredMembers: true)
=> containingType.AllRequiredMembers.SelectManyAsArray(static kvp => getAllMembersToBeDefaulted(kvp.Value)),
=> containingType.AllRequiredMembers.SelectManyAsArray(static kvp => getAllMembersToBeDefaulted(kvp.Value, filterOverridingProperties: true)),

(includeAllMembers: true, includeCurrentTypeRequiredMembers: _, includeBaseRequiredMembers: false)
=> containingType.GetMembersUnordered().SelectAsArray(getFieldSymbolToBeInitialized),
=> containingType.GetMembersUnordered().SelectManyAsArray(
selector: symbol =>
{
var symbolToInitialize = getFieldSymbolToBeInitialized(symbol);
var prop = symbolToInitialize as PropertySymbol ?? (symbolToInitialize as FieldSymbol)?.AssociatedSymbol as PropertySymbol;
333fred marked this conversation as resolved.
Show resolved Hide resolved
if (prop is not null && isFilterableOverrideOfAbstractProperty(prop))
{
return OneOrMany<Symbol>.Empty;
}
else
{
return OneOrMany.Create(symbolToInitialize);
}
}),

(includeAllMembers: true, includeCurrentTypeRequiredMembers: true, includeBaseRequiredMembers: true)
=> getAllTypeAndRequiredMembers(containingType),
Expand All @@ -928,50 +943,94 @@ static ImmutableArray<Symbol> membersToBeInitialized(NamedTypeSymbol containingT
static ImmutableArray<Symbol> getAllTypeAndRequiredMembers(TypeSymbol containingType)
{
var members = containingType.GetMembersUnordered();
var requiredMembers = containingType.BaseTypeNoUseSiteDiagnostics?.AllRequiredMembers ?? ImmutableSegmentedDictionary<string, Symbol>.Empty;
var baseRequiredMembers = containingType.BaseTypeNoUseSiteDiagnostics?.AllRequiredMembers ?? ImmutableSegmentedDictionary<string, Symbol>.Empty;

if (requiredMembers.IsEmpty)
if (baseRequiredMembers.IsEmpty)
{
return members;
}

var builder = ArrayBuilder<Symbol>.GetInstance(members.Length + requiredMembers.Count);
var builder = ArrayBuilder<Symbol>.GetInstance(members.Length + baseRequiredMembers.Count);
builder.AddRange(members);
foreach (var (_, requiredMember) in requiredMembers)
foreach (var (_, requiredMember) in baseRequiredMembers)
{
// We want to assume that all required members were _not_ set by the chained constructor
builder.AddRange(getAllMembersToBeDefaulted(requiredMember));

// We exclude any members that are abstract and have an implementation in the current type, when that implementation passes the same heuristics
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
// we use in the other other cases (annotations match up across overrides)
if (requiredMember is PropertySymbol { IsAbstract: true } abstractProperty)
{
if (members.FirstOrDefault(static (thisMember, baseMember) => thisMember.IsOverride && (object)thisMember.GetOverriddenMember() == baseMember, requiredMember) is { } overridingMember
&& isFilterableOverrideOfAbstractProperty((PropertySymbol)overridingMember))
{
continue;
}
}

builder.AddRange(getAllMembersToBeDefaulted(requiredMember, filterOverridingProperties: false));
}

return builder.ToImmutableAndFree();
}

static IEnumerable<Symbol> getAllMembersToBeDefaulted(Symbol requiredMember)
static OneOrMany<Symbol> getAllMembersToBeDefaulted(Symbol requiredMember, bool filterOverridingProperties)
{
Debug.Assert(requiredMember.IsRequired());

if (requiredMember is FieldSymbol)
{
yield return requiredMember;
return OneOrMany.Create(requiredMember);
}
else
{
var property = (PropertySymbol)requiredMember;
yield return getFieldSymbolToBeInitialized(property);

if (filterOverridingProperties && isFilterableOverrideOfAbstractProperty(property))
{
return OneOrMany<Symbol>.Empty;
}

var @return = OneOrMany.Create(getFieldSymbolToBeInitialized(property));

// If the set method is null (ie missing), that's an error, but we'll recover as best we can
foreach (var notNullMemberName in (property.SetMethod?.NotNullMembers ?? property.NotNullMembers))
{
foreach (var member in property.ContainingType.GetMembers(notNullMemberName))
{
yield return getFieldSymbolToBeInitialized(member);
@return = @return.Add(getFieldSymbolToBeInitialized(member));
}
}

return @return;
}
}

static Symbol getFieldSymbolToBeInitialized(Symbol requiredMember)
=> requiredMember is SourcePropertySymbolBase { BackingField: { } backingField } ? backingField : requiredMember;

static bool isFilterableOverrideOfAbstractProperty(PropertySymbol property)
{
// If this is an override of an abstract property, and the overridden property has the same nullable
// annotation as us, we can skip default-initializing the property because the chained constructor
// will have done so
if (property.OverriddenProperty is not { IsAbstract: true } overriddenProperty)
{
return false;
}

var symbolAnnotations = property is SourcePropertySymbolBase { UsesFieldKeyword: true, BackingField: { } field }
? field!.FlowAnalysisAnnotations
: property.GetFlowAnalysisAnnotations();
var symbolType = ApplyUnconditionalAnnotations(property.TypeWithAnnotations, symbolAnnotations);
if (!symbolType.NullableAnnotation.IsNotAnnotated())
{
return false;
}

var overriddenAnnotations = overriddenProperty.GetFlowAnalysisAnnotations();
var overriddenType = ApplyUnconditionalAnnotations(overriddenProperty.TypeWithAnnotations, overriddenAnnotations);
return overriddenType.NullableAnnotation == symbolType.NullableAnnotation;
}
}
}
}
Expand Down
Loading
Loading