Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ protected LocalDataFlowPass(

protected abstract int AddVariable(VariableIdentifier identifier);

/// <summary>
/// <summary>Get the slot for a variable, if the slot already exists.</summary>
/// <remarks>
/// Locals are given slots when their declarations are encountered. We only need give slots
/// to local variables, out parameters, and the "this" variable of a struct constructs.
/// Other variables are not given slots, and are therefore not tracked by the analysis. This
Expand All @@ -71,7 +72,7 @@ protected LocalDataFlowPass(
/// variables that occur before the variable is declared, as those are reported in an
/// earlier phase as "use before declaration". That allows us to avoid giving slots to local
/// variables before processing their declarations.
/// </summary>
/// </remarks>
protected int VariableSlot(Symbol symbol, int containingSlot = 0)
{
// Skip LocalStoreTracker from data flow analysis.
Expand Down
115 changes: 89 additions & 26 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ protected override OneOrMany<SyntaxList<AttributeListSyntax>> GetAttributeDeclar
public override Symbol AssociatedSymbol
=> _property;

internal SourcePropertySymbolBase AssociatedProperty
=> _property;

public override ImmutableArray<Location> Locations
=> _property.Locations;

Expand Down
805 changes: 742 additions & 63 deletions src/Compilers/CSharp/Test/Emit3/FieldKeywordTests.cs

Large diffs are not rendered by default.

194 changes: 194 additions & 0 deletions src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4845,6 +4845,200 @@ public Derived()
comp.VerifyDiagnostics(expectedDiagnostics);
}

[Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)]
[WorkItem(6754, "https://github.com/dotnet/csharplang/issues/6754")]
public void RequiredMemberSuppressesNullabilityWarnings_MemberNotNull_ChainedBaseConstructor_06()
{
// Base required property initializes a private field.
// Derived constructors have SetsRequiredMembers and demonstrate behavior with/without initializing base required property.
var @base = """
using System.Diagnostics.CodeAnalysis;
#nullable enable
public class Base
{
private string _field;
public string Field => _field;
public required string Property { get => _field; [MemberNotNull(nameof(_field))] set => _field = value; }

public Base() { }
}
""";

var derived = """
using System;
using System.Diagnostics.CodeAnalysis;
#nullable enable

var d = new Derived("property");
Console.Write(d.Field);
Console.Write(' ');

d = new Derived();
try
{
d.Field.ToString();
}
catch (NullReferenceException)
{
Console.Write("NullReferenceException");
}

class Derived : Base
{
[SetsRequiredMembers]
public Derived()
{
}

[SetsRequiredMembers]
public Derived(string value)
{
Property = value;
}
}
""";

var comp = CreateCompilationWithRequiredMembers(new[] { @base, derived, MemberNotNullAttributeDefinition });
comp.VerifyDiagnostics(
// 1.cs(22,12): warning CS8618: Non-nullable property 'Property' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable.
// public Derived()
Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "Derived").WithArguments("property", "Property").WithLocation(22, 12)
);
CompileAndVerify(comp, expectedOutput: "property NullReferenceException");

var baseComp = CreateCompilationWithRequiredMembers(new[] { @base, MemberNotNullAttributeDefinition });
comp = CreateCompilation(derived, new[] { baseComp.EmitToImageReference() });
comp.VerifyDiagnostics(
// 1.cs(22,12): warning CS8618: Non-nullable property 'Property' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable.
// public Derived()
Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "Derived").WithArguments("property", "Property").WithLocation(22, 12)
);
CompileAndVerify(comp, expectedOutput: "property NullReferenceException");
}

[Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)]
[WorkItem(6754, "https://github.com/dotnet/csharplang/issues/6754")]
public void RequiredMemberSuppressesNullabilityWarnings_MemberNotNull_ChainedBaseConstructor_07()
{
// Base required property, with nullable type, initializes a private field.
// Derived constructors have SetsRequiredMembers and demonstrate behavior with/without initializing base required property.
// Note: this is a safety hole. However, the user has to go quite off into the weeds to fall into it.
var @base = """
using System.Diagnostics.CodeAnalysis;
#nullable enable
public class Base
{
private string _field;
public string Field => _field;
public required string? Property { get => _field; [MemberNotNull(nameof(_field))] set => _field = value ?? "property"; }

public Base() { }
}
""";

var derived = """
using System;
using System.Diagnostics.CodeAnalysis;
#nullable enable

var d = new Derived(null);
Console.Write(d.Field);
Console.Write(' ');

d = new Derived();
try
{
d.Field.ToString();
}
catch (NullReferenceException)
{
Console.Write("NullReferenceException");
}

class Derived : Base
{
[SetsRequiredMembers]
public Derived()
{
}

[SetsRequiredMembers]
public Derived(string? value)
{
Property = value;
}
}
""";

var comp = CreateCompilationWithRequiredMembers(new[] { @base, derived, MemberNotNullAttributeDefinition });
comp.VerifyDiagnostics();
CompileAndVerify(comp, expectedOutput: "property NullReferenceException");

var baseComp = CreateCompilationWithRequiredMembers(new[] { @base, MemberNotNullAttributeDefinition });
comp = CreateCompilation(derived, new[] { baseComp.EmitToImageReference() });
comp.VerifyDiagnostics();
CompileAndVerify(comp, expectedOutput: "property NullReferenceException");
}

[Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)]
[WorkItem(6754, "https://github.com/dotnet/csharplang/issues/6754")]
public void RequiredMemberSuppressesNullabilityWarnings_MemberNotNull_ChainedBaseConstructor_08()
{
// Base required property initializes a private field.
// Derived is a nested type of the base and therefore can access the base private field.
// Derived constructors have SetsRequiredMembers and demonstrate behavior with/without initializing base required property.
var source = """
using System;
using System.Diagnostics.CodeAnalysis;
#nullable enable

var d = new Base.Derived("property");
Console.Write(d.Field);
Console.Write(' ');

d = new Base.Derived();
try
{
d.Field.ToString();
}
catch (NullReferenceException)
{
Console.Write("NullReferenceException");
}

public class Base
{
private string _field;
public string Field => _field;
public required string Property { get => _field; [MemberNotNull(nameof(_field))] set => _field = value; }

public Base() { }

public class Derived : Base
{
[SetsRequiredMembers]
public Derived()
{
}

[SetsRequiredMembers]
public Derived(string value)
{
Property = value;
}
}
}
""";

var comp = CreateCompilationWithRequiredMembers(new[] { source, MemberNotNullAttributeDefinition });
comp.VerifyDiagnostics(
// 0.cs(30,16): warning CS8618: Non-nullable property 'Property' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable.
// public Derived()
Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "Derived").WithArguments("property", "Property").WithLocation(30, 16)
);
CompileAndVerify(comp, expectedOutput: "property NullReferenceException");
}

[Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)]
public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_01()
{
Expand Down
13 changes: 13 additions & 0 deletions src/Dependencies/Collections/Extensions/IEnumerableExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,19 @@ public static ImmutableArray<TResult> SelectManyAsArray<TItem, TArg, TResult>(th
return builder.ToImmutableAndFree();
}

public static ImmutableArray<TResult> SelectManyAsArray<TItem, TArg, TResult>(this IReadOnlyCollection<TItem>? source, Func<TItem, TArg, OneOrMany<TResult>> selector, TArg arg)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we have an overload that takes ImmutableArray receiver too? Looks like this is currently being called on ImmutableArray receivers, causing boxing.

{
if (source is null or { Count: 0 })
return [];

// Basic heuristic. Assume each element in the source adds one item to the result.
var builder = ArrayBuilder<TResult>.GetInstance(source.Count);
foreach (var item in source)
selector(item, arg).AddRangeTo(builder);

return builder.ToImmutableAndFree();
}

public static ImmutableArray<TResult> SelectManyAsArray<TSource, TResult>(this IEnumerable<TSource>? source, Func<TSource, OneOrMany<TResult>> selector)
{
if (!TryGetBuilder<TSource, TResult>(source, useCountForBuilder: false, out var builder))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,29 @@ public static ImmutableArray<TResult> SelectManyAsArray<TItem, TResult>(this Imm
return builder.ToImmutableAndFree();
}

/// <summary>
/// Maps and flattens an immutable array to another immutable array.
/// </summary>
/// <typeparam name="TItem">Type of the source array items</typeparam>
/// <typeparam name="TArg">Type of the argument to pass to the selector.</typeparam>
/// <typeparam name="TResult">Type of the transformed array items</typeparam>
/// <param name="array">The array to transform</param>
/// <param name="selector">A transform function to apply to each element.</param>
/// <returns>If the array's length is 0, this will return an empty immutable array.</returns>
Comment thread
RikkiGibson marked this conversation as resolved.
public static ImmutableArray<TResult> SelectManyAsArray<TItem, TArg, TResult>(this ImmutableArray<TItem> array, Func<TItem, TArg, OneOrMany<TResult>> selector, TArg arg)
{
if (array.Length == 0)
return [];

var builder = ArrayBuilder<TResult>.GetInstance();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Interesting that we are not setting the capacity here like we are in the IReadOnlyCollection equivalent. But this matches the other ImmutableArray extensions around, so I guess it's fine.

foreach (var item in array)
{
selector(item, arg).AddRangeTo(builder);
}

return builder.ToImmutableAndFree();
}

/// <summary>
/// Maps and flattens a subset of immutable array to another immutable array.
/// </summary>
Expand Down