InterpolatedStringHandlerArgumentAttribute Decoding #53932

333fred · 2021-06-08T00:00:54Z

Validate and decode InterpolatedStringHandlerArgumentAttribute from source and from metadata. This doesn't implement call-site validation, just errors on attribute usage and metadata decoding.

Test plan: #51499

…urce.

…rom metadata.

333fred · 2021-06-08T00:01:59Z

@chsienki @AlekseyTs for review.

chsienki · 2021-06-08T00:42:41Z

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerFileReference.cs

                    if (PEModule.CrackStringArrayInAttributeValue(out additionalLanguageNames, ref argsReader))
                    {
                        if (additionalLanguageNames.Length == 0)
                        {
-                            return SpecializedCollections.SingletonEnumerable(firstLanguageName);
+                            return firstLanguageName == null ? SpecializedCollections.EmptyEnumerable<string>() : SpecializedCollections.SingletonEnumerable(firstLanguageName);
                        }


Previously this would return an array of one null string in the case it read null from the language, or the additional languages with null at the end. I suspect it makes no difference, but want to check that behavior change doesn't break anything. #Resolved

Users already had to deal with the empty list as a possible return. I don't think this really introduces anything new.

chsienki · 2021-06-08T18:37:36Z

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEParameterSymbol.cs

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


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

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

chsienki · 2021-06-08T18:57:37Z

src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs

+            ArrayBuilder<ParameterSymbol?> parameters;
+            if (constructorArgument.Kind == TypedConstantKind.Primitive)
+            {
+                if (decodeName(constructorArgument, ref arguments) is (int ordinal, var parameter))


Consider using is not and inverting the conditional.

if (decodeName(constructorArgument, ref arguments) is not (int ordinal, var parameter)) { return; } parameterOrdinals = ImmutableArray.Create(ordinal); parameters = ArrayBuilder<ParameterSymbol?>.GetInstance(1); parameters.Add(parameter); ``` #Resolved

chsienki · 2021-06-08T22:13:46Z

src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs

+                    return (-1, null);
+                }
+
+                ImmutableArray<ParameterSymbol> parameters = ContainingSymbol switch


Could do this once, outside the function #Resolved

chsienki · 2021-06-08T22:23:57Z

src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs

@@ -1048,6 +1057,199 @@ bool needsReporting()
            }
        }

+#nullable enable
+        private void DecodeInterpolatedStringHandlerArgumentAttribute(ref DecodeWellKnownAttributeArguments<AttributeSyntax, CSharpAttributeData, AttributeLocation> arguments, Binder binder, BindingDiagnosticBag diagnostics)


I guess i'm suprised that we would need the binder in order to decode the attribute. I see below that we're doing overload resolution to determine if its valid or not. Would an alternative be to just decode the raw strings here? Then wherever we actually use them in the binder, do the actual resolution etc? We can always get back to them from the containing symbol I think? (they always apply to other parameters in the same list as the one the attribute is on?) #Resolved

Would an alternative be to just decode the raw strings here?

We could, yes, but then you don't get validation of the attribute at the attribute definition location. This seems suboptimal to me.

Hmm, yeah thats a bummer. Just seems like we're doing a lot of work here that as a caller I woulnd't expect to happen. Do we have anywhere else where the attribute itself can error based on semantics of other code?

In reply to: 647835991 [](ancestors = 647835991)

Hmm. Could we not just store the location along with the cracked data? So we can report on it if it fails later on?

So we can report on it if it fails later on?

This doesn't help if you never use it in this project (as I would expect many to do, since these will mostly be used in libraries). Unless Aleksey has a different opinion or suggestion for how to approach this, I believe this is the right way to go.

After discussion with Aleksey, we've decided that there's too much potential for issue with missing references, retargeting, and langversion changes to move forward with this for now. I'll remove the binder code.

chsienki · 2021-06-08T22:26:39Z

Done with review pass (iteration 4)

AlekseyTs · 2021-06-09T17:58:10Z

    private static IEnumerable<string> ReadLanguagesFromAttribute(ref BlobReader argsReader)

How changes in this function are related to the goal of the PR?

In reply to: 857917744

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerFileReference.cs:296 in b8ea696. [](commit_id = b8ea696, deletion_comment = False)

AlekseyTs · 2021-06-09T18:15:30Z

src/Compilers/Core/Portable/MetadataReader/PEModule.cs

+            }
+            else if (TryExtractStringArrayValueFromAttribute(targetAttribute.Handle, out var paramNames))
+            {
+                Debug.Assert(targetAttribute.SignatureIndex == 1);


Debug.Assert(targetAttribute.SignatureIndex == 1);

It feels like the Assert is somewhat misplaced. Should we move it before we TryExtractStringArrayValueFromAttribute? #Closed

AlekseyTs · 2021-06-09T18:21:00Z

src/Compilers/Core/Portable/MetadataReader/PEModule.cs

+            else if (TryExtractStringArrayValueFromAttribute(targetAttribute.Handle, out var paramNames))
+            {
+                Debug.Assert(targetAttribute.SignatureIndex == 1);
+                return paramNames.NullToEmpty();


paramNames.NullToEmpty()

Is an attribute with an empty array a valid form? If so, I think we should not represent invalid/broken forms with an empty array. This is applicable to other places in this function. Basically I think we should be able to judge from the result of GetInterpolatedStringHandlerArgumentAttributeValues if the attribute was missing or malformed. #Closed

Basically I think we should be able to judge from the result of GetInterpolatedStringHandlerArgumentAttributeValues if the attribute was missing or malformed.

I originally had it as default if missing/malformed, but thinking about it more I realized I didn't have a reason to need to understand the difference. Unless we want to actually error at the use site when we encounter a malformed attribute from metadata (which I didn't see the need to do), it feels like having the distinction really doesn't buy us much of anything.

Unless we want to actually error at the use site when we encounter a malformed attribute from metadata (which I didn't see the need to do), it feels like having the distinction really doesn't buy us much of anything.

My initial thinking is that we should fail to create a handler at the call site if there is a maformed InterpolatedStringHandlerArgumentAttribute attribute on the corresponding parameter.

I'll record whether the data has errors in a separate bit in the packed flags.

AlekseyTs · 2021-06-09T18:34:36Z

src/Compilers/CSharp/Portable/CSharpResources.resx

+    <value>Parameter to interpolated string handler conversion occurs after handler parameter</value>
+  </data>
+  <data name="ERR_CannotUseSelfAsInterpolatedStringHandlerArgument" xml:space="preserve">
+    <value>Interpolated string handler arguments cannot refer to themselves.</value>


Is this error about referring to the parameter in the attribute applied to the same parameter? Perhaps it would be clearer to say something like: "cannot be used to create a handler for this parameter"? #Closed

I tried to make it a bit more clear. Let me know if you think it still needs work.

AlekseyTs · 2021-06-09T18:41:01Z

src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml

@@ -2020,6 +2020,13 @@

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

+  <!-- Type expression placeholder for the arguments to the constructor call for an interpolated string handler


Type expression

"Type expression" sounds a lot like BoundTypeExpression, but I believe this node doesn't represent a type reference. Because of that the comment feels confusing to me. I suggest saying "An expression" or "A typed expression" instead. #Closed

333fred · 2021-06-09T18:44:40Z

    private static IEnumerable<string> ReadLanguagesFromAttribute(ref BlobReader argsReader)

They resulted from me nullable enabling the decoding functions (which I did because I had missed a case in the attribute decoding and was suprised I didn't get a warning about it).

In reply to: 857917744

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerFileReference.cs:296 in b8ea696. [](commit_id = b8ea696, deletion_comment = False)

AlekseyTs · 2021-06-09T18:51:43Z

    private static IEnumerable<string> ReadLanguagesFromAttribute(ref BlobReader argsReader)

They resulted from me nullable enabling the decoding functions (which I did because I had missed a case in the attribute decoding and was suprised I didn't get a warning about it).

Since the change goes beyond just adding/adjusting annotations. I think the change should be taken out of this PR and out of this feature branch. If we need to suppress nulable warnings in this function to do this, that is what we should do, I think.

In reply to: 857971951

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerFileReference.cs:296 in b8ea696. [](commit_id = b8ea696, deletion_comment = False)

AlekseyTs · 2021-06-09T19:11:15Z

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEParameterSymbol.cs

+                MethodSymbol { Parameters: var p } => p,
+                PropertySymbol { Parameters: var p } => p,
+                _ => throw ExceptionUtilities.UnexpectedValue(ContainingSymbol)
+            };


It looks like we already have internal static ImmutableArray<ParameterSymbol> GetParameters(this Symbol member) helper that can be used here. #Closed

BTW, are we going to crash on an event accessor parameter here?

AlekseyTs · 2021-06-09T19:14:57Z

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEParameterSymbol.cs

+                    case { Length: 0 } when ContainingSymbol.IsStatic:
+                        // Invalid data, bail
+                        builder.Free();
+                        return ImmutableArray<int>.Empty;


return ImmutableArray.Empty;

As I mentioned elsewhere, I think we should not silently ignore malformed applications of the attribute at the call site of the method. #Closed

AlekseyTs · 2021-06-09T19:21:25Z

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEParameterSymbol.cs

+                switch (name)
+                {
+                    case null:
+                    case { Length: 0 } when ContainingSymbol.IsStatic:


when ContainingSymbol.IsStatic:

I guess I am missing what static-ness of a container has to do with validity of the data? Is there a relevant spec? #Closed

Perhaps the Length check confused me as I thought we are dealing with an array. Consider comparing with an empty string instead, for clarity. Below as well.

AlekseyTs · 2021-06-09T19:38:12Z

src/Compilers/CSharp/Portable/Symbols/Source/SourceClonedParameterSymbol.cs

@@ -164,6 +164,8 @@ internal override ImmutableHashSet<string> NotNullIfParameterNotNull
            get { return ImmutableHashSet<string>.Empty; }
        }

+        internal override ImmutableArray<int> InterpolatedStringHandlerArgumentIndexes => _originalParam.InterpolatedStringHandlerArgumentIndexes;


_originalParam.InterpolatedStringHandlerArgumentIndexes;

Why is this needed? And why do we assume inxes won't change? #Closed

Cloned parameter symbols share attribute data, so they shouldn't change.

Cloned parameter symbols share attribute data, so they shouldn't change.

Perhaps attribute applications shouldn't change, but the meaning of attributes can. It is quite possible that for other well-known attributes it isn't an issue because they are self-contained. However for this attribute, surrounding parameters and parameter ordinals are significant, etc. So, I would say we should reinterpret the attribute application rather than take it as is. But the question is whether there is a scenario where this particular symbol is required to expose this information at all. If not, then why return anything but a value indicating - fail creating a handler at the call site. Anything other than an "error" value would require thorough testing for this code path.

I'll make these throw for now and mark with a prototype, and we'll see if the indexes are needed during lowering in the next PR. I have a hunch they might be.

AlekseyTs · 2021-06-09T19:43:26Z

src/Compilers/CSharp/Portable/Symbols/Wrapped/WrappedParameterSymbol.cs

@@ -169,6 +169,8 @@ public override string GetDocumentationCommentXml(CultureInfo preferredCulture =
            return _underlyingParameter.GetDocumentationCommentXml(preferredCulture, expandIncludes, cancellationToken);
        }

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


internal override ImmutableArray InterpolatedStringHandlerArgumentIndexes => _underlyingParameter.InterpolatedStringHandlerArgumentIndexes;

I would prefer overriding this property only in leaf classes. Otherwise, it is hard to predict whether this implementation is appropriate for all derived types now and in the future. #Closed

AlekseyTs · 2021-06-09T20:05:32Z

src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs

@@ -746,6 +751,10 @@ internal override void DecodeWellKnownAttribute(ref DecodeWellKnownAttributeArgu
                arguments.GetOrCreateData<ParameterWellKnownAttributeData>().HasEnumeratorCancellationAttribute = true;
                ValidateCancellationTokenAttribute(arguments.AttributeSyntaxOpt, (BindingDiagnosticBag)arguments.Diagnostics);
            }
+            else if (attribute.IsTargetAttribute(this, AttributeDescription.InterpolatedStringHandlerArgumentAttribute))


IsTargetAttribute

Since we need to deal with two different signatures here, I think we should use GetTargetAttributeSignatureIndex directly rather than trying to infer what signature we are dealing with based on values. #Closed

AlekseyTs · 2021-06-09T20:25:48Z

src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs

+                {
+                    if (parameter.Name.Equals(name, StringComparison.Ordinal))
+                    {
+                        if ((object)parameter == this)


if ((object)parameter == this)

Are we checking this error condition for PE? #Closed

AlekseyTs · 2021-06-09T20:28:44Z

src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs

+                    _ => throw ExceptionUtilities.UnexpectedValue(ContainingType)
+                };
+
+                foreach (var parameter in parameters)


foreach (var parameter in parameters)

It looks like for similar task for PEParameterSymbol we are using FirstOrDefault helper #Closed

AlekseyTs · 2021-06-09T21:14:49Z

src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs

+            foreach (var parameter in parameters)
+            {
+                var argumentType = parameter?.Type ?? ContainingType;
+                analyzedArguments.Arguments.Add(new BoundInterpolatedStringArgumentPlaceholder(arguments.AttributeSyntaxOpt!, argumentType, hasErrors: argumentType.IsErrorType()));


BoundInterpolatedStringArgumentPlaceholder

I am curious how BoundInterpolatedStringArgumentPlaceholder behaves as a target for a ref/out parameter. It is not a variable by default. Are we testing this? #Closed

We have tests for this side in this PR, but we're also ignoring any diagnostics that come up from this. Regardless, since I'm removing this code it'll be tested in more depth in the next PR.

AlekseyTs · 2021-06-09T21:25:43Z

src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs

+            else
+            {
+                // Could not find a constructor on '{0}' that matches the parameters specified in the InterpolatedStringHandlerArgumentAttribute.
+                diagnostics.Add(ErrorCode.ERR_CouldNotFindApplicableInterpolatedStringHandlerConstructor, errorLocation, handlerType);


diagnostics.Add(ErrorCode.ERR_CouldNotFindApplicableInterpolatedStringHandlerConstructor, errorLocation, handlerType);

I think we should worry whether there are things that are specific to this particular location that could cause a failure that might not occur at a different location (missing references, language version, etc.)
#Closed

AlekseyTs · 2021-06-09T21:29:18Z

src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs

+                errorName: handlerType.Name,
+                errorLocation,
+                suppressResultDiagnostics: true,
+                BindingDiagnosticBag.Discarded,


BindingDiagnosticBag.Discarded

When I see us binding with BindingDiagnosticBag.Discarded and then reporting a diagnostics based on the result, that is a red flag for me. Used asembly references are not tracked and it is quite possible that things break after they are removed because compiler says they are not used. #Closed

AlekseyTs · 2021-06-09T22:02:36Z

src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs

+            var vbCode = @"
+Imports System.Runtime.CompilerServices
+Public Class C
+    Public Sub M(i As Integer, <InterpolatedStringHandlerArgument({ Nothing })> c As CustomHandler)


{ Nothing }

Did you mean to use a single element array here? Or CStr()Nothing)? #Closed

Yes, this is intentionally an array with 1 element, that element being Nothing.

AlekseyTs · 2021-06-09T22:03:16Z

src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs

+        }
+
+        [Fact]
+        public void InterpolatedStringHandlerArgumentAttributeError_ReferenceSelf()


InterpolatedStringHandlerArgumentAttributeError_ReferenceSelf

Are we testing this scenario from metadata? #Closed

AlekseyTs · 2021-06-09T22:04:27Z

src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs

+            var vbCode = @"
+Imports System.Runtime.CompilerServices
+Public Class C
+    Public Shared Sub M(<InterpolatedStringHandlerArgument({ """" })> c As CustomHandler)


{ """" }

Are we testing single value overload? #Closed

AlekseyTs · 2021-06-09T22:05:37Z

src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs

+        }
+
+        [Fact]
+        public void InterpolatedStringHandlerArgumentAttributeWarn_ParameterAfterHandler()


InterpolatedStringHandlerArgumentAttributeWarn_ParameterAfterHandler

Consider adding a metadata test too. #Closed

AlekseyTs · 2021-06-09T22:09:56Z

src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs

+
+public partial struct CustomHandler
+{
+    private CustomHandler(int literalLength, int formattedCount, int i) : this() {}


private CustomHandler(int literalLength, int formattedCount, int i) : this() {}

Accessibility is another aspect that is location specific. For example, internal/protected constrcutor might be accessible from where M will be called. #Closed

I'll test location more fully in the next PR. We're removing the error for this scenario.

AlekseyTs · 2021-06-09T22:12:42Z

src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs

+            var handler = GetCustomHandlerType("CustomHandler", "partial struct", useBoolReturns: true);
+
+            var comp = CreateCompilation(new[] { code, InterpolatedStringHandlerArgumentAttribute, handler }, parseOptions: TestOptions.RegularPreview);
+            CompileAndVerify(comp, sourceSymbolValidator: verifier, symbolValidator: verifier);


CompileAndVerify

Consider VerifyDiagnostics on result of CompileAndVerify calls. This helps catch unexpected warnings, etc. #Closed

AlekseyTs · 2021-06-09T22:17:03Z

Done with review pass (commit 5).

333fred · 2021-06-10T17:28:52Z

    private static IEnumerable<string> ReadLanguagesFromAttribute(ref BlobReader argsReader)

Filed #53994 to track.

In reply to: 857980418

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerFileReference.cs:296 in b8ea696. [](commit_id = b8ea696, deletion_comment = False)

* Remove attribute constructor resolution during source binding * More tests * Small refactorings.

333fred · 2021-06-10T18:01:15Z

@AlekseyTs @chsienki addressed your feedback.

In reply to: 858138258

AlekseyTs · 2021-06-10T20:14:32Z

src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs

@@ -621,6 +621,10 @@ public override ImmutableArray<CustomModifier> RefCustomModifiers
                }
            }

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


_underlyingParameter.InterpolatedStringHandlerArgumentIndexes

Should these be adjusted? I think reduced extension methods remove the first parameter, it becomes the receiver. If we never bind against a reduced method symbol (it used to be that C# only used them for SemanticModel, this is likely still the case), consider simply throwing from both new APIs. #Closed

AlekseyTs · 2021-06-10T20:28:09Z

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEParameterSymbol.cs

        }

        private struct PackedFlags
        {
            // Layout:
-            // |...|fffffffff|n|rr|cccccccc|vvvvvvvv|
+            // |..|fffffffff|n|rr|ccccccccc|vvvvvvvvv|


// |..|fffffffff|n|rr|ccccccccc|vvvvvvvvv|

We are running out of bits here really fast. I believe CallerArgumentEx[pression is going to add at least couple of bits here. Automatic merge isn't goint to work as well.
As an alternative, can we use default value in _lazyInterpolatedStringHandlerAttributeIndexes to indicate an error and use another sentinel value to represent an unitilized state, like a shared array with a single int.MinValue in it? #Closed

Rather than creating a new structure, I think the appropriate thing to do will be to move _bits to a long at that point. It'll take a bit of effort when merging, but I'd much rather do that than try and create a separate side-structure for just this bit.

Talked with Aleksey offline, I misread his comment. I'll do this.

AlekseyTs · 2021-06-10T20:39:20Z

src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs

+            var vbCode = @"
+Imports System.Runtime.CompilerServices
+Public Class C
+    Public Sub M(i As Integer, <InterpolatedStringHandlerArgument({ Nothing })> c As CustomHandler)


{ Nothing }

Are we testing single value overload? #Closed

AlekseyTs · 2021-06-10T21:12:43Z

...CSharp/Portable/Symbols/Attributes/WellKnownAttributeData/ParameterWellKnownAttributeData.cs

+            }
+        }
+
+        private bool _hasInterpolatedStringHandlerArgumentAttributeError = false;


private bool _hasInterpolatedStringHandlerArgumentAttributeError = false;

Consider if we can get rid of this separate flag similar to the suggestion for PEParameterSymbol. #Closed

AlekseyTs · 2021-06-10T21:25:43Z

@333fred It looks like there are some related test failures.

AlekseyTs · 2021-06-10T21:31:28Z

Done with review pass (commit 7)

In reply to: 859088157

333fred · 2021-06-10T21:54:11Z

@AlekseyTs addressed feedback.

In reply to: 859088157

AlekseyTs

LGTM (commit 9), assuming CI is passing

333fred · 2021-06-10T22:10:29Z

@chsienki can you please take another look?

333fred added 3 commits June 4, 2021 11:10

Support validating InterpolatedStringHandlerArgumentAttribute from so…

83b5aa8

…urce.

Nullable enable decoding parts of PEModule.

b76fd13

Add support for decoding InterpolatedStringHandlerArgumentAttribute f…

eed18c0

…rom metadata.

333fred requested a review from a team as a code owner June 8, 2021 00:00

dotnet-issue-labeler bot added the Area-Compilers label Jun 8, 2021

333fred requested review from chsienki and AlekseyTs June 8, 2021 00:01

333fred added 2 commits June 7, 2021 17:08

Fixup annotations in DiagnosticAnalyzer and a test mistake.

fffd0f2

Update warning level test.

b8ea696

chsienki reviewed Jun 8, 2021

View reviewed changes

Minor code review refactoring.

13d6246

AlekseyTs reviewed Jun 9, 2021

View reviewed changes

333fred mentioned this pull request Jun 10, 2021

Correctly handle nullable in AnalyzerFileReference.ReadLanguagesFromAttribute #53994

Open

PR Feedback:

fba1cee

* Remove attribute constructor resolution during source binding * More tests * Small refactorings.

AlekseyTs reviewed Jun 10, 2021

View reviewed changes

333fred added 2 commits June 10, 2021 14:31

PR feedback around condensing flags.

9b1858e

Fix misnamed XML section.

a79dc1f

AlekseyTs approved these changes Jun 10, 2021

View reviewed changes

chsienki approved these changes Jun 10, 2021

View reviewed changes

333fred merged commit e0a2cd6 into dotnet:features/interpolated-string Jun 10, 2021

333fred deleted the interpolated-arguments-decode branch June 10, 2021 23:16

		@@ -2020,6 +2020,13 @@

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

		<!-- Type expression placeholder for the arguments to the constructor call for an interpolated string handler

InterpolatedStringHandlerArgumentAttribute Decoding #53932

InterpolatedStringHandlerArgumentAttribute Decoding #53932

Conversation

333fred commented Jun 8, 2021 • edited by jcouv Loading

333fred commented Jun 8, 2021

chsienki Jun 8, 2021 • edited by 333fred Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chsienki Jun 8, 2021 • edited by 333fred Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chsienki Jun 8, 2021 • edited by 333fred Loading

Choose a reason for hiding this comment

chsienki Jun 8, 2021 • edited by 333fred Loading

Choose a reason for hiding this comment

chsienki Jun 8, 2021 • edited by 333fred Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chsienki commented Jun 8, 2021

AlekseyTs commented Jun 9, 2021 • edited Loading

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

333fred commented Jun 9, 2021

AlekseyTs commented Jun 9, 2021

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

333fred commented Jun 8, 2021 •

edited by jcouv

Loading

chsienki Jun 8, 2021 •

edited by 333fred

Loading

chsienki Jun 8, 2021 •

edited by 333fred

Loading

chsienki Jun 8, 2021 •

edited by 333fred

Loading

chsienki Jun 8, 2021 •

edited by 333fred

Loading

chsienki Jun 8, 2021 •

edited by 333fred

Loading

AlekseyTs commented Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 9, 2021 •

edited

Loading

AlekseyTs Jun 10, 2021 •

edited

Loading

AlekseyTs Jun 10, 2021 •

edited

Loading

AlekseyTs Jun 10, 2021 •

edited

Loading

AlekseyTs Jun 10, 2021 •

edited

Loading

AlekseyTs commented Jun 10, 2021 •

edited by 333fred

Loading