Skip to content

Commit

Permalink
Don't use DefaultInterpolatedStringHandler in expression trees
Browse files Browse the repository at this point in the history
Introduces a binder flag to track whether we are currently in an expression tree or not. Fixes dotnet#55114.
  • Loading branch information
333fred committed Jul 26, 2021
1 parent fc4a667 commit 9d0e52e
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 21 deletions.
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,8 @@ internal virtual bool IsLabelsScopeBinder
}
}

internal bool InExpressionTree => (Flags & BinderFlags.InExpressionTree) == BinderFlags.InExpressionTree;

/// <summary>
/// True if this is the top-level binder for a local function or lambda
/// (including implicit lambdas from query expressions).
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/BinderFlags.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@ internal enum BinderFlags : uint
/// </summary>
SuppressTypeArgumentBinding = 1 << 29,

/// <summary>
/// The current context is an expression tree
/// </summary>
InExpressionTree = 1 << 30,

// Groups

AllClearedAtExecutableCodeBoundary = InLockBody | InCatchBlock | InCatchFilter | InFinallyBlock | InTryBlockOfTryCatch | InNestedFinallyBlock,
Expand Down
7 changes: 4 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -565,12 +565,13 @@ private BoundExpression CreateAnonymousFunctionConversion(SyntaxNode syntax, Bou
BoundLambda boundLambda;
if (delegateType is { })
{
if (destination.IsNonGenericExpressionType())
bool isExpression = destination.IsNonGenericExpressionType();
if (isExpression)
{
delegateType = Compilation.GetWellKnownType(WellKnownType.System_Linq_Expressions_Expression_T).Construct(delegateType);
delegateType.AddUseSiteInfo(ref useSiteInfo);
}
boundLambda = unboundLambda.Bind(delegateType);
boundLambda = unboundLambda.Bind(delegateType, isExpression);
}
else
{
Expand All @@ -591,7 +592,7 @@ private BoundExpression CreateAnonymousFunctionConversion(SyntaxNode syntax, Bou
var discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;
_ = unboundLambda.InferDelegateType(ref discardedUseSiteInfo);
#endif
var boundLambda = unboundLambda.Bind((NamedTypeSymbol)destination);
var boundLambda = unboundLambda.Bind((NamedTypeSymbol)destination, isExpressionTree: destination.IsGenericOrNonGenericExpressionType(out _));
diagnostics.AddRange(boundLambda.Diagnostics);
return createAnonymousFunctionConversion(syntax, source, boundLambda, conversion, isCast, conversionGroup, destination);
}
Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4286,7 +4286,8 @@ private BoundExpression BindDelegateCreationExpression(SyntaxNode node, NamedTyp
diagnostics.Add(node, useSiteInfo);
// Attempting to make the conversion caches the diagnostics and the bound state inside
// the unbound lambda. Fetch the result from the cache.
BoundLambda boundLambda = unboundLambda.Bind(type);
Debug.Assert(!type.IsGenericOrNonGenericExpressionType(out _));
BoundLambda boundLambda = unboundLambda.Bind(type, isExpressionTree: false);

if (!conversion.IsImplicit || !conversion.IsValid)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,8 @@ BoundInterpolatedString constructWithData(ImmutableArray<BoundExpression> parts,
bool tryBindAsHandlerType([NotNullWhen(true)] out BoundInterpolatedString? result)
{
result = null;
if (unconvertedInterpolatedString.Parts.ContainsAwaitExpression())

if (InExpressionTree || unconvertedInterpolatedString.Parts.ContainsAwaitExpression())
{
return false;
}
Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1671,7 +1671,8 @@ private ImmutableArray<BoundExpression> BuildArgumentsForErrorRecovery(AnalyzedA
if (parameterType?.Kind == SymbolKind.NamedType &&
(object)parameterType.GetDelegateType() != null)
{
var discarded = unboundArgument.Bind((NamedTypeSymbol)parameterType);
// Just assume we're not in an expression tree for the purposes of error recovery.
var discarded = unboundArgument.Bind((NamedTypeSymbol)parameterType, isExpressionTree: false);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2010,7 +2010,7 @@ internal void GenerateAnonymousFunctionConversionError(BindingDiagnosticBag diag

if (reason == LambdaConversionResult.BindingFailed)
{
var bindingResult = anonymousFunction.Bind(delegateType);
var bindingResult = anonymousFunction.Bind(delegateType, isExpressionTree: false);
Debug.Assert(ErrorFacts.PreventsSuccessfulDelegateConversion(bindingResult.Diagnostics.Diagnostics));
diagnostics.AddRange(bindingResult.Diagnostics);
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1246,7 +1246,7 @@ source.Type is object &&
IsConstantNumericZero(sourceConstantValue);
}

private static LambdaConversionResult IsAnonymousFunctionCompatibleWithDelegate(UnboundLambda anonymousFunction, TypeSymbol type)
private static LambdaConversionResult IsAnonymousFunctionCompatibleWithDelegate(UnboundLambda anonymousFunction, TypeSymbol type, bool isTargetExpressionTree)
{
Debug.Assert((object)anonymousFunction != null);
Debug.Assert((object)type != null);
Expand Down Expand Up @@ -1355,7 +1355,7 @@ private static LambdaConversionResult IsAnonymousFunctionCompatibleWithDelegate(
}

// Ensure the body can be converted to that delegate type
var bound = anonymousFunction.Bind(delegateType);
var bound = anonymousFunction.Bind(delegateType, isTargetExpressionTree);
if (ErrorFacts.PreventsSuccessfulDelegateConversion(bound.Diagnostics.Diagnostics))
{
return LambdaConversionResult.BindingFailed;
Expand Down Expand Up @@ -1397,7 +1397,7 @@ private static LambdaConversionResult IsAnonymousFunctionCompatibleWithExpressio
return LambdaConversionResult.Success;
}

return IsAnonymousFunctionCompatibleWithDelegate(anonymousFunction, delegateType);
return IsAnonymousFunctionCompatibleWithDelegate(anonymousFunction, delegateType, isTargetExpressionTree: true);
}

public static LambdaConversionResult IsAnonymousFunctionCompatibleWithType(UnboundLambda anonymousFunction, TypeSymbol type)
Expand All @@ -1411,7 +1411,7 @@ public static LambdaConversionResult IsAnonymousFunctionCompatibleWithType(Unbou
}
else if (type.IsDelegateType())
{
return IsAnonymousFunctionCompatibleWithDelegate(anonymousFunction, type);
return IsAnonymousFunctionCompatibleWithDelegate(anonymousFunction, type, isTargetExpressionTree: false);
}
else if (type.IsGenericOrNonGenericExpressionType(out bool _))
{
Expand Down
26 changes: 16 additions & 10 deletions src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs
Original file line number Diff line number Diff line change
Expand Up @@ -399,8 +399,8 @@ internal UnboundLambda WithNoCache()
public NamedTypeSymbol? InferDelegateType(ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
=> Data.InferDelegateType(ref useSiteInfo);

public BoundLambda Bind(NamedTypeSymbol delegateType)
=> SuppressIfNeeded(Data.Bind(delegateType));
public BoundLambda Bind(NamedTypeSymbol delegateType, bool isExpressionTree)
=> SuppressIfNeeded(Data.Bind(delegateType, isExpressionTree));

public BoundLambda BindForErrorRecovery()
=> SuppressIfNeeded(Data.BindForErrorRecovery());
Expand Down Expand Up @@ -519,13 +519,18 @@ public virtual void GenerateAnonymousFunctionConversionError(BindingDiagnosticBa
}

// Returns the inferred return type, or null if none can be inferred.
public BoundLambda Bind(NamedTypeSymbol delegateType)
public BoundLambda Bind(NamedTypeSymbol delegateType, bool isTargetExpressionTree)
{
BoundLambda? result;
if (!_bindingCache!.TryGetValue(delegateType, out result))
bool inExpressionTree = Binder.InExpressionTree || isTargetExpressionTree;

if (!_bindingCache!.TryGetValue(delegateType, out BoundLambda? result))
{
result = ReallyBind(delegateType);
result = ImmutableInterlocked.GetOrAdd(ref _bindingCache, delegateType, result);
result = ReallyBind(delegateType, inExpressionTree);
// For simplicity, we don't cache expression tree results
if (!inExpressionTree)
{
result = ImmutableInterlocked.GetOrAdd(ref _bindingCache, delegateType, result);
}
}

return result;
Expand Down Expand Up @@ -629,7 +634,7 @@ private static TypeWithAnnotations DelegateReturnTypeWithAnnotations(MethodSymbo
ref useSiteInfo);
}

private BoundLambda ReallyBind(NamedTypeSymbol delegateType)
private BoundLambda ReallyBind(NamedTypeSymbol delegateType, bool inExpressionTree)
{
Debug.Assert(Binder.ContainingMemberOrLambda is { });

Expand All @@ -649,7 +654,8 @@ private BoundLambda ReallyBind(NamedTypeSymbol delegateType)
// For simplicity, reuse is limited to expression-bodied lambdas. In those cases,
// we reuse the bound expression and apply any conversion to the return value
// since the inferred return type was not used when binding for return inference.
if (refKind == CodeAnalysis.RefKind.None &&
if (!inExpressionTree &&
refKind == CodeAnalysis.RefKind.None &&
_returnInferenceCache!.TryGetValue(cacheKey, out BoundLambda? returnInferenceLambda) &&
GetLambdaExpressionBody(returnInferenceLambda.Body) is BoundExpression expression &&
(lambdaSymbol = returnInferenceLambda.Symbol).RefKind == refKind &&
Expand All @@ -663,7 +669,7 @@ private BoundLambda ReallyBind(NamedTypeSymbol delegateType)
else
{
lambdaSymbol = CreateLambdaSymbol(Binder.ContainingMemberOrLambda, returnType, cacheKey.ParameterTypes, cacheKey.ParameterRefKinds, refKind);
lambdaBodyBinder = new ExecutableCodeBinder(_unboundLambda.Syntax, lambdaSymbol, ParameterBinder(lambdaSymbol, Binder));
lambdaBodyBinder = new ExecutableCodeBinder(_unboundLambda.Syntax, lambdaSymbol, ParameterBinder(lambdaSymbol, Binder), inExpressionTree ? BinderFlags.InExpressionTree : BinderFlags.None);
block = BindLambdaBody(lambdaSymbol, lambdaBodyBinder, diagnostics);
}

Expand Down
28 changes: 28 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10316,6 +10316,32 @@ public void DisallowedInExpressionTrees()
);
}

[Fact, WorkItem(55114, "https://github.com/dotnet/roslyn/issues/55114")]
public void AsStringInExpressionTrees_01()
{
var code = @"
using System;
using System.Linq.Expressions;
Expression<Func<string, string>> e = o => $""{o.Length}"";";

var comp = CreateCompilation(new[] { code, GetInterpolatedStringHandlerDefinition(includeSpanOverloads: false, useDefaultParameters: false, useBoolReturns: false) });
comp.VerifyEmitDiagnostics();
}

[Fact, WorkItem(55114, "https://github.com/dotnet/roslyn/issues/55114")]
public void AsStringInExpressionTrees_02()
{
var code = @"
using System;
using System.Linq.Expressions;
Expression<Func<Func<string, string>>> e = () => o => $""{o.Length}"";";

var comp = CreateCompilation(new[] { code, GetInterpolatedStringHandlerDefinition(includeSpanOverloads: false, useDefaultParameters: false, useBoolReturns: false) });
comp.VerifyEmitDiagnostics();
}

[Theory]
[CombinatorialData]
public void CustomHandlerUsedAsArgumentToCustomHandler(bool useBoolReturns, bool validityParameter)
Expand Down Expand Up @@ -11335,5 +11361,7 @@ .locals init (int V_0, //i
}
");
}


}
}

0 comments on commit 9d0e52e

Please sign in to comment.