Skip to content

Commit

Permalink
Do honor abstract required during state initialization in nullable
Browse files Browse the repository at this point in the history
Fixes #74423. When we're getting the list of members to default initialize, we look for properties that override `abstract` properties and have nullable annotations that line up with the base ones. For these properties, we can assume that if we chain to a `SetsRequiredMembers` constructor, it took care of setting a non-null state.
  • Loading branch information
333fred committed Dec 12, 2024
1 parent 7997469 commit 848ec10
Show file tree
Hide file tree
Showing 4 changed files with 610 additions and 12 deletions.
83 changes: 71 additions & 12 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
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;
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
// 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

0 comments on commit 848ec10

Please sign in to comment.