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

Avoid assuming containing assembly is from source in DefiniteAssignment #59069

Merged
merged 7 commits into from
Jan 27, 2022
Merged
Show file tree
Hide file tree
Changes from 6 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 @@ -187,6 +187,7 @@ protected AbstractFlowPass(
bool trackRegions = false,
bool nonMonotonicTransferFunction = false)
{
Debug.Assert(compilation != null);
Debug.Assert(node != null);

if (firstInRegion != null && lastInRegion != null)
Expand Down
33 changes: 21 additions & 12 deletions src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

#if DEBUG
// We use a struct rather than a class to represent the state for efficiency
// for data flow analysis, with 32 bits of data inline. Merely copying the state
Expand Down Expand Up @@ -59,20 +57,18 @@ internal partial class DefiniteAssignmentPass : LocalDataFlowPass<
/// <summary>
/// Some variables that should be considered initially assigned. Used for region analysis.
/// </summary>
private readonly HashSet<Symbol> initiallyAssignedVariables;
private readonly HashSet<Symbol>? initiallyAssignedVariables;

/// <summary>
/// Variables that were used anywhere, in the sense required to suppress warnings about
/// unused variables.
/// </summary>
private readonly PooledHashSet<LocalSymbol> _usedVariables = PooledHashSet<LocalSymbol>.GetInstance();

#nullable enable
/// <summary>
/// Parameters of record primary constructors that were read anywhere.
/// </summary>
private PooledHashSet<ParameterSymbol>? _readParameters;
#nullable disable

/// <summary>
/// Variables that were used anywhere, in the sense required to suppress warnings about
Expand Down Expand Up @@ -105,12 +101,12 @@ internal partial class DefiniteAssignmentPass : LocalDataFlowPass<
/// <summary>
/// The current source assembly.
/// </summary>
private readonly SourceAssemblySymbol _sourceAssembly;
private readonly SourceAssemblySymbol? _sourceAssembly;

/// <summary>
/// A set of address-of expressions for which the operand is not definitely assigned.
/// </summary>
private readonly HashSet<PrefixUnaryExpressionSyntax> _unassignedVariableAddressOfSyntaxes;
private readonly HashSet<PrefixUnaryExpressionSyntax>? _unassignedVariableAddressOfSyntaxes;

/// <summary>
/// Tracks variables for which we have already reported a definite assignment error. This
Expand All @@ -136,7 +132,7 @@ internal partial class DefiniteAssignmentPass : LocalDataFlowPass<
/// <summary>
/// The topmost method of this analysis.
/// </summary>
protected MethodSymbol topLevelMethod;
protected MethodSymbol? topLevelMethod;

protected bool _convertInsufficientExecutionStackExceptionToCancelledByStackGuardException = false; // By default, just let the original exception to bubble up.

Expand All @@ -151,7 +147,7 @@ internal DefiniteAssignmentPass(
BoundNode node,
bool strictAnalysis,
bool trackUnassignments = false,
HashSet<PrefixUnaryExpressionSyntax> unassignedVariableAddressOfSyntaxes = null,
HashSet<PrefixUnaryExpressionSyntax>? unassignedVariableAddressOfSyntaxes = null,
bool requireOutParamsAssigned = true,
bool trackClassFields = false,
bool trackStaticMembers = false)
Expand All @@ -160,7 +156,7 @@ internal DefiniteAssignmentPass(
trackUnassignments)
{
this.initiallyAssignedVariables = null;
_sourceAssembly = ((object)member == null) ? null : (SourceAssemblySymbol)member.ContainingAssembly;
_sourceAssembly = GetSourceAssembly(compilation, member, node);
_unassignedVariableAddressOfSyntaxes = unassignedVariableAddressOfSyntaxes;
_requireOutParamsAssigned = requireOutParamsAssigned;
_trackClassFields = trackClassFields;
Expand All @@ -175,11 +171,11 @@ internal DefiniteAssignmentPass(
BoundNode node,
EmptyStructTypeCache emptyStructs,
bool trackUnassignments = false,
HashSet<Symbol> initiallyAssignedVariables = null)
HashSet<Symbol>? initiallyAssignedVariables = null)
: base(compilation, member, node, emptyStructs, trackUnassignments)
{
this.initiallyAssignedVariables = initiallyAssignedVariables;
_sourceAssembly = ((object)member == null) ? null : (SourceAssemblySymbol)member.ContainingAssembly;
_sourceAssembly = GetSourceAssembly(compilation, member, node);
this.CurrentSymbol = member;
_unassignedVariableAddressOfSyntaxes = null;
_requireOutParamsAssigned = true;
Expand Down Expand Up @@ -208,6 +204,18 @@ internal DefiniteAssignmentPass(
_shouldCheckConverted = this.GetType() == typeof(DefiniteAssignmentPass);
}

private static SourceAssemblySymbol GetSourceAssembly(
CSharpCompilation compilation,
Symbol member,
BoundNode node)
{
Debug.Assert(member is null ||
node.Kind == BoundKind.Attribute ||
(object)member.ContainingAssembly == compilation.SourceAssembly);

return compilation.SourceAssembly;
}

protected override void Free()
{
variableBySlot.Free();
Expand Down Expand Up @@ -237,6 +245,7 @@ protected override int AddVariable(VariableIdentifier identifier)
return slot;
}

#nullable disable
protected Symbol GetNonMemberSymbol(int slot)
{
VariableIdentifier variableId = variableBySlot[slot];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8032,6 +8032,59 @@ static void Main()
Assert.Equal("px", GetSymbolNamesJoined(analysis.WrittenOutside));
}

[WorkItem(57428, "https://github.com/dotnet/roslyn/issues/57428")]
[Fact]
public void AttributeArgumentWithLambdaBody_01()
{
var source =
@"using System.Runtime.InteropServices;
class Program
{
static void F([DefaultParameterValue(() => { return 0; })] object obj)
{
}
}";
var compilation = CreateCompilation(source);
compilation.VerifyDiagnostics(
// (4,42): error CS0182: An attribute argument must be a constant expression, typeof expression or array creation expression of an attribute parameter type
// static void F([DefaultParameterValue(() => { return 0; })] object obj)
Diagnostic(ErrorCode.ERR_BadAttributeArgument, "() => { return 0; }").WithLocation(4, 42));

var tree = compilation.SyntaxTrees[0];
var model = compilation.GetSemanticModel(tree);
var expr = tree.GetRoot().DescendantNodes().OfType<LiteralExpressionSyntax>().Single();
var analysis = model.AnalyzeDataFlow(expr);
Assert.False(analysis.Succeeded);
}

[Fact]
public void AttributeArgumentWithLambdaBody_02()
{
var source =
@"using System;
class A : Attribute
{
internal A(object o) { }
}
class Program
{
static void F([A(() => { return 0; })] object obj)
{
}
}";
var compilation = CreateCompilation(source);
compilation.VerifyDiagnostics(
// (8,22): error CS0182: An attribute argument must be a constant expression, typeof expression or array creation expression of an attribute parameter type
// static void F([A(() => { return 0; })] object obj)
Diagnostic(ErrorCode.ERR_BadAttributeArgument, "() => { return 0; }").WithLocation(8, 22));

var tree = compilation.SyntaxTrees[0];
var model = compilation.GetSemanticModel(tree);
var expr = tree.GetRoot().DescendantNodes().OfType<LiteralExpressionSyntax>().Single();
var analysis = model.AnalyzeDataFlow(expr);
Assert.False(analysis.Succeeded);
}

#endregion

#region "Used Local Functions"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4617,5 +4617,31 @@ class Ignored2 { }
CodeActionEquivalenceKey = nameof(FeaturesResources.Extract_method),
}.RunAsync();
}

[WorkItem(57428, "https://github.com/dotnet/roslyn/issues/57428")]
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod)]
public async Task AttributeArgumentWithLambdaBody()
{
await TestInRegularAndScript1Async(
@"using System.Runtime.InteropServices;
class Program
{
static void F([DefaultParameterValue(() => { return [|null|]; })] object obj)
{
}
}",
@"using System.Runtime.InteropServices;
class Program
{
static void F([DefaultParameterValue(() => { return {|Rename:NewMethod|}(); })] object obj)
{
}

private static object NewMethod()
{
return null;
}
}");
}
}
}