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

InterpolatedStringHandlerArgumentAttribute Decoding #53932

2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5607,7 +5607,7 @@ private BoundExpression BindTypeParameterCreationExpression(SyntaxNode node, Typ
/// of this method (i.e. not populating a pre-existing <see cref="OverloadResolutionResult{MethodSymbol}"/>).
/// Presently, rationalizing this behavior is not worthwhile.
/// </remarks>
private bool TryPerformConstructorOverloadResolution(
internal bool TryPerformConstructorOverloadResolution(
NamedTypeSymbol typeContainingConstructors,
AnalyzedArguments analyzedArguments,
string errorName,
Expand Down
7 changes: 7 additions & 0 deletions src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2020,6 +2020,13 @@

<Node Name="BoundInterpolatedStringHandlerPlaceholder" Base="BoundValuePlaceholderBase"/>

<!-- A typed expression placeholder for the arguments to the constructor call for an interpolated string handler
conversion. We intentionally use a placeholder for overload resolution here to ensure that no conversion
from expression can occur. -->
<Node Name="BoundInterpolatedStringArgumentPlaceholder" Base="BoundValuePlaceholderBase">
<Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow" />
</Node>

<Node Name="BoundStringInsert" Base="BoundExpression">
<Field Name="Type" Type="TypeSymbol?" Override="true" Null="always"/>
<Field Name="Value" Type="BoundExpression" Null="disallow"/>
Expand Down
25 changes: 25 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -6656,4 +6656,29 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_GlobalUsingOutOfOrder" xml:space="preserve">
<value>A global using directive must precede all non-global using directives.</value>
</data>
<data name="ERR_NullInvalidInterpolatedStringHandlerArgumentName" xml:space="preserve">
<value>null is not a valid parameter name. To get access to the receiver of an instance method, use the empty string as the parameter name.</value>
</data>
<data name="ERR_NotInstanceInvalidInterpolatedStringHandlerArgumentName" xml:space="preserve">
<value>'{0}' is not an instance method, the receiver cannot be an interpolated string handler argument.</value>
</data>
<data name="ERR_InvalidInterpolatedStringHandlerArgumentName" xml:space="preserve">
<value>'{0}' is not a valid parameter name from '{1}'.</value>
</data>
<data name="ERR_TypeIsNotAnInterpolatedStringHandlerType" xml:space="preserve">
<value>'{0}' is not an interpolated string handler type.</value>
</data>
<data name="ERR_InterpolatedStringHandlerIncorrectNumberOfConstructorArguments" xml:space="preserve">
<value>'{0}' does not have a constructor that takes {1} arguments.</value>
</data>
<data name="WRN_ParameterOccursAfterInterpolatedStringHandlerParameter" xml:space="preserve">
<value>Parameter {0} occurs after {1} in the parameter list, but is used as an argument for interpolated string handler conversions. This will require the caller to reorder parameters with named arguments at the call site. Consider putting the interpolated string handler parameter after all arguments involved.</value>
</data>
<data name="WRN_ParameterOccursAfterInterpolatedStringHandlerParameter_Title" xml:space="preserve">
<value>Parameter to interpolated string handler conversion occurs after handler parameter</value>
</data>
<data name="ERR_CannotUseSelfAsInterpolatedStringHandlerArgument" xml:space="preserve">
<value>InterpolatedStringHandlerArgumentAttribute arguments cannot refer to the parameter the attribute is used on.</value>
<comment>InterpolatedStringHandlerArgumentAttribute is a type name and should not be translated.</comment>
</data>
</root>
7 changes: 7 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1942,6 +1942,13 @@ internal enum ErrorCode
ERR_InterpolatedStringHandlerMethodReturnMalformed = 9001,
ERR_InterpolatedStringHandlerMethodReturnInconsistent = 9002,
ERR_InterpolatedStringHandlerInvalidCreateMethod = 9003,
ERR_NullInvalidInterpolatedStringHandlerArgumentName = 9004,
ERR_NotInstanceInvalidInterpolatedStringHandlerArgumentName = 9005,
ERR_InvalidInterpolatedStringHandlerArgumentName = 9006,
ERR_TypeIsNotAnInterpolatedStringHandlerType = 9007,
ERR_InterpolatedStringHandlerIncorrectNumberOfConstructorArguments = 9008,
WRN_ParameterOccursAfterInterpolatedStringHandlerParameter = 9009,
ERR_CannotUseSelfAsInterpolatedStringHandlerArgument = 9010,

#endregion diagnostics introduced for C# 10.0

Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,7 @@ internal static int GetWarningLevel(ErrorCode code)
case ErrorCode.WRN_AnalyzerReferencesFramework:
case ErrorCode.WRN_UnreadRecordParameter:
case ErrorCode.WRN_DoNotCompareFunctionPointers:
case ErrorCode.WRN_ParameterOccursAfterInterpolatedStringHandlerParameter:
return 1;
default:
return 0;
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -158,5 +158,21 @@ public void AddNotNullIfParameterNotNull(string parameterName)
_notNullIfParameterNotNull = _notNullIfParameterNotNull.Add(parameterName);
SetDataStored();
}

private ImmutableArray<int> _interpolatedStringHandlerArguments = ImmutableArray<int>.Empty;
public ImmutableArray<int> InterpolatedStringHandlerArguments
{
get
{
VerifySealed(expected: true);
return _interpolatedStringHandlerArguments;
}
set
{
VerifySealed(expected: false);
_interpolatedStringHandlerArguments = value;
SetDataStored();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,7 @@ internal int MethodHashCode()
internal override bool IsCallerMemberName => false;
internal override FlowAnalysisAnnotations FlowAnalysisAnnotations => FlowAnalysisAnnotations.None;
internal override ImmutableHashSet<string> NotNullIfParameterNotNull => ImmutableHashSet<string>.Empty;
internal override ImmutableArray<int> InterpolatedStringHandlerArgumentIndexes => ImmutableArray<int>.Empty;
internal override bool HasInterpolatedStringHandlerArgumentError => false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using System.Reflection;
using System.Reflection.Metadata;
using System.Runtime.InteropServices;
Expand Down Expand Up @@ -141,6 +142,9 @@ public bool TryGetFlowAnalysisAnnotations(out FlowAnalysisAnnotations value)
private ConstantValue _lazyDefaultValue = ConstantValue.Unset;
private ThreeState _lazyIsParams;

private static readonly ImmutableArray<int> s_defaultStringHandlerAttributeIndexes = ImmutableArray.Create(int.MinValue);
private ImmutableArray<int> _lazyInterpolatedStringHandlerAttributeIndexes = s_defaultStringHandlerAttributeIndexes;

/// <summary>
/// Attributes filtered out from m_lazyCustomAttributes, ParamArray, etc.
/// </summary>
Expand Down Expand Up @@ -702,6 +706,95 @@ private static FlowAnalysisAnnotations DecodeFlowAnalysisAttributes(PEModule mod
return annotations;
}

#nullable enable
internal override ImmutableArray<int> InterpolatedStringHandlerArgumentIndexes
{
get
{
EnsureInterpolatedStringHandlerArgumentAttributeDecoded();
ImmutableArray<int> indexes = _lazyInterpolatedStringHandlerAttributeIndexes;
Debug.Assert(indexes != s_defaultStringHandlerAttributeIndexes);
return indexes.NullToEmpty();
}
}

internal override bool HasInterpolatedStringHandlerArgumentError
{
get
{
EnsureInterpolatedStringHandlerArgumentAttributeDecoded();
ImmutableArray<int> indexes = _lazyInterpolatedStringHandlerAttributeIndexes;
Debug.Assert(indexes != s_defaultStringHandlerAttributeIndexes);
return indexes.IsDefault;
}
}

private void EnsureInterpolatedStringHandlerArgumentAttributeDecoded()
{
ImmutableArray<int> indexes = _lazyInterpolatedStringHandlerAttributeIndexes;
if (indexes == s_defaultStringHandlerAttributeIndexes)
{
indexes = DecodeInterpolatedStringHandlerArgumentAttribute();
Debug.Assert(indexes != s_defaultStringHandlerAttributeIndexes);
var initialized = ImmutableInterlocked.InterlockedCompareExchange(ref _lazyInterpolatedStringHandlerAttributeIndexes, value: indexes, comparand: s_defaultStringHandlerAttributeIndexes);
Debug.Assert(initialized == s_defaultStringHandlerAttributeIndexes || indexes == initialized || indexes.SequenceEqual(initialized));
}
}

private ImmutableArray<int> DecodeInterpolatedStringHandlerArgumentAttribute()
{
var (paramNames, hasAttribute) = _moduleSymbol.Module.GetInterpolatedStringHandlerArgumentAttributeValues(_handle);

if (!hasAttribute)
{
return ImmutableArray<int>.Empty;
}
else if (paramNames.IsDefault || Type is not NamedTypeSymbol { IsInterpolatedStringHandlerType: true })
{
return default;
}

if (paramNames.IsEmpty)
{
return ImmutableArray<int>.Empty;
}

var builder = ArrayBuilder<int>.GetInstance(paramNames.Length);
var parameters = ContainingSymbol.GetParameters();

foreach (var name in paramNames)
{
switch (name)
{
case null:
case "" when ContainingSymbol.IsStatic:
// Invalid data, bail
builder.Free();
return default;

case "":
builder.Add(-1);
break;

default:
var param = parameters.FirstOrDefault(static (p, name) => string.Equals(p.Name, name, StringComparison.Ordinal), name);
Copy link
Contributor

@chsienki chsienki Jun 8, 2021

Choose a reason for hiding this comment

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

Given that this is the only place we're using FirstOrDefault, is it worth just putting the implementation here, avoiding the lambda, and probably isnt any more lines of code as you don't need the below checks? #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so. I prefer it the way I wrote it.

if (param is not null && (object)param != this)
{
builder.Add(param.Ordinal);
break;
}
else
{
builder.Free();
return default;
}
}
}

return builder.ToImmutableAndFree();
}
#nullable disable

internal override ImmutableHashSet<string> NotNullIfParameterNotNull
{
get
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,10 @@ internal NativeIntegerParameterSymbol(NativeIntegerTypeSymbol containingType, Na

public override ImmutableArray<CustomModifier> RefCustomModifiers => _underlyingParameter.RefCustomModifiers;

internal override ImmutableArray<int> InterpolatedStringHandlerArgumentIndexes => _underlyingParameter.InterpolatedStringHandlerArgumentIndexes;

internal override bool HasInterpolatedStringHandlerArgumentError => _underlyingParameter.HasInterpolatedStringHandlerArgumentError;

public override bool Equals(Symbol? other, TypeCompareKind comparison) => NativeIntegerTypeSymbol.EqualsHelper(this, other, comparison, symbol => symbol._underlyingParameter);

public override int GetHashCode() => _underlyingParameter.GetHashCode();
Expand Down
Loading