Propagate marked attributes to synthesized fields. #76567
Conversation
- From a parameter to a state machine field - From a parameter to a closure field - From a VB property/event to a backing field Related to dotnet#73920.
dotnet-issue-labeler
bot
added
the
untriaged
src/Compilers/CSharp/Portable/Lowering/ClosureConversion/LambdaCapturedVariable.cs
Outdated
Show resolved
Hide resolved
|
@dotnet/roslyn-compiler Please review |
| bool isThis = IsThis(captured, out ParameterSymbol? parameter); | ||
| return parameter is not null && !isThis ? | ||
| new LambdaCapturedVariableForRegularParameter(frame, TypeWithAnnotations.Create(type), fieldName, parameter) : | ||
| new LambdaCapturedVariable(frame, TypeWithAnnotations.Create(type), fieldName, isThis); | ||
| } | ||
|
|
||
| private static bool IsThis(Symbol captured) |
There was a problem hiding this comment.
I prefer to not adjust existing consumers of this helper.
| { | ||
| AddSynthesizedAttribute(ref attributes, attr); | ||
| } | ||
| } |
There was a problem hiding this comment.
This block to copy attributes is repeated in several locations. Would it make sense to extract a helper method? #WontFix
There was a problem hiding this comment.
This block to copy attributes is repeated in several locations. Would it make sense to extract a helper method?
I do not think this is worth doing.
| @@ -2487,5 +2487,283 @@ class Test | |||
| } | |||
|
|
|||
| #endregion | |||
|
|
|||
| [Fact] | |||
| public void CompilerLoweringPreserveAttribute_01() | |||
There was a problem hiding this comment.
Did the behavior of these two tests change with this PR?
No, the behavior did not change.
| @@ -3026,5 +3026,74 @@ class C | |||
| var property = compilation.GetMember<PropertySymbol>("C.P"); | |||
| Assert.True(property.RequiresInstanceReceiver); | |||
| } | |||
|
|
|||
| [Fact] | |||
| public void CompilerLoweringPreserveAttribute_01() | |||
There was a problem hiding this comment.
Did the behavior of these two tests change with this PR?
No, it didn't.
| using System; | ||
|
|
||
| [AttributeUsage(AttributeTargets.Field)] | ||
| public class Preserve1Attribute : Attribute { } |
There was a problem hiding this comment.
What are we testing with no
[CompilerLoweringPreserve]attribute?
We are testing that the field: target allows to apply any attribute to the field explicitly. Therefore, no special treatment around CompilerLoweringPreserve is necessary.
| (attributeType.GetAttributeUsageInfo().ValidTargets And System.AttributeTargets.Field) <> 0 Then | ||
| AddSynthesizedAttribute(attributes, attr) | ||
| End If | ||
| Next |
There was a problem hiding this comment.
This block is used in a couple of locations. Would it make sense to extract a helper method? #WontFix
There was a problem hiding this comment.
This block is used in a couple of locations. Would it make sense to extract a helper method?
I do not think this is worth doing.
AlekseyTs
merged commit bfa4f1a
into
dotnet:features/GeneratedCodeAttributes
Related to #73920.