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.

Sign up for GitHub

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

Localsinit #24723

Merged
merged 9 commits into from
Feb 22, 2018
Merged

Localsinit #24723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
339bac2
SkipLocalsInitAttribute recognized as a WellKnownAttribute and proper…
t-camaia Feb 6, 2018
1712070
ILVisualizer locals init depends on LocalsAreZeroed flag
t-camaia Feb 8, 2018
cd07338
SkipLocalsInitAttribute on method tests added
t-camaia Feb 8, 2018
dc64263
LocalsAreZeroed passed as parameter instead of methodBody. Virtual Lo…
t-camaia Feb 9, 2018
665e690
LocalsAreZeroed renamed to AreLocalsZeroed. SourceMemberMethodSymbol.…
t-camaia Feb 12, 2018
ecb278d
AreLocalsZeroed in MethodSymbol changed from 'virtual' to 'abstract'
t-camaia Feb 14, 2018
bd4f01b
SerializeMethodBody cannot use body from cache if they don't have the…
t-camaia Feb 14, 2018
3c1f7d7
Packages and tests updated. HashCode issue fixed.
t-camaia Feb 21, 2018
a6d98d0
MetadataWriter and tests updated
t-camaia Feb 21, 2018
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
2 changes: 1 addition & 1 deletion build/Targets/Packages.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
<MicrosoftIdentityModelClientsActiveDirectoryVersion>3.13.8</MicrosoftIdentityModelClientsActiveDirectoryVersion>
<MicrosoftInternalPerformanceCodeMarkersDesignTimeVersion>15.0.26730-alpha</MicrosoftInternalPerformanceCodeMarkersDesignTimeVersion>
<MicrosoftInternalVisualStudioShellInterop140DesignTimeVersion>14.3.25407-alpha</MicrosoftInternalVisualStudioShellInterop140DesignTimeVersion>
<MicrosoftMetadataVisualizerVersion>1.0.0-beta1-61531-03</MicrosoftMetadataVisualizerVersion>
<MicrosoftMetadataVisualizerVersion>1.0.0-beta1-62621-03</MicrosoftMetadataVisualizerVersion>
<MicrosoftMSXMLVersion>8.0.0.0-alpha</MicrosoftMSXMLVersion>
<!-- Using a private build of Microsoft.Net.Test.SDK to work around issue https://github.com/Microsoft/vstest/issues/373 -->
<MicrosoftNETTestSdkVersion>15.6.0-dev</MicrosoftNETTestSdkVersion>
Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1346,7 +1346,7 @@ private static MethodBody GenerateMethodBody(
var localSlotManager = new LocalSlotManager(variableSlotAllocatorOpt);
var optimizations = compilation.Options.OptimizationLevel;

ILBuilder builder = new ILBuilder(moduleBuilder, localSlotManager, optimizations);
ILBuilder builder = new ILBuilder(moduleBuilder, localSlotManager, optimizations, method.AreLocalsZeroed);
DiagnosticBag diagnosticsForThisMethod = DiagnosticBag.GetInstance();
try
{
Expand Down Expand Up @@ -1460,6 +1460,7 @@ private static MethodBody GenerateMethodBody(
builder.RealizedSequencePoints,
debugDocumentProvider,
builder.RealizedExceptionHandlers,
builder.AreLocalsZeroed,
builder.GetAllScopes(),
builder.HasDynamicLocal,
importScopeOpt,
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/ErrorMethodSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,11 @@ public sealed override DllImportData GetDllImportData()
return null;
}

public override bool AreLocalsZeroed
{
get { throw ExceptionUtilities.Unreachable; }
}

internal sealed override MarshalPseudoCustomAttributeData ReturnValueMarshallingInformation
{
get { return null; }
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1153,6 +1153,11 @@ internal override void AddSynthesizedReturnTypeAttributes(PEModuleBuilder module
throw ExceptionUtilities.Unreachable;
}

public override bool AreLocalsZeroed
{
get { throw ExceptionUtilities.Unreachable; }
}

// perf, not correctness
internal override CSharpCompilation DeclaringCompilation => null;

Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1221,6 +1221,11 @@ public virtual bool IsTupleMethod
}
}

/// <summary>
/// Returns true if locals are to be initialized
/// </summary>
public abstract bool AreLocalsZeroed { get; }

/// <summary>
/// If this is a method of a tuple type, return corresponding underlying method from the
/// tuple underlying type. Otherwise, null.
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,11 @@ public override DllImportData GetDllImportData()
return _reducedFrom.GetDllImportData();
}

public override bool AreLocalsZeroed
{
get { throw ExceptionUtilities.Unreachable; }
}

internal override MarshalPseudoCustomAttributeData ReturnValueMarshallingInformation
{
get { return _reducedFrom.ReturnValueMarshallingInformation; }
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/SignatureOnlyMethodSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ public SignatureOnlyMethodSymbol(

public override bool IsExtern { get { throw ExceptionUtilities.Unreachable; } }

public override bool AreLocalsZeroed { get { throw ExceptionUtilities.Unreachable; } }

public override AssemblySymbol ContainingAssembly { get { throw ExceptionUtilities.Unreachable; } }

internal override ModuleSymbol ContainingModule { get { throw ExceptionUtilities.Unreachable; } }
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Source/LambdaSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ public override DllImportData GetDllImportData()
return null;
}

public override bool AreLocalsZeroed
{
get { throw ExceptionUtilities.Unreachable; }
}

internal override MarshalPseudoCustomAttributeData ReturnValueMarshallingInformation
{
get { return null; }
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Source/LocalFunctionSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,8 @@ internal override TypeSymbol IteratorElementType

public override DllImportData GetDllImportData() => null;

public override bool AreLocalsZeroed => throw ExceptionUtilities.Unreachable;
Copy link
Member

Choose a reason for hiding this comment

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

I think all of these can be replaced with a single override that returns true in SourceMethodSymbol

Copy link
Member

Choose a reason for hiding this comment

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

Just talked with @AlekseyTs and he thinks we should be explicit in each callsite. I think that's a fine decision as well and we can go with that.

However, I would be very surprised if this didn't throw whenever we try to emit local functions, so I think we'll have to provide a real implementation, even if it's accompanied by a prototype comment saying that the full spec will be implemented in another PR.

Copy link
Member

Choose a reason for hiding this comment

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

To answer my own question, by the time we end up at creating a MethodBody, LocalFunctions and similar are all turned into SynthesizedInstance or similar symbols, so throwing is actually the right thing to do here, I think.


internal override ImmutableArray<string> GetAppliedConditionalSymbols() => ImmutableArray<string>.Empty;

internal override bool IsMetadataNewSlot(bool ignoreInterfaceImplementationChanges = false) => false;
Expand Down
13 changes: 13 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,15 @@ public override Symbol AssociatedSymbol
}
}

public override bool AreLocalsZeroed
{
get
{
var data = this.GetDecodedWellKnownAttributeData();
return data != null ? !data.HasSkipLocalsInitAttribute : true;
}
}

#region Flags

public override bool ReturnsVoid
Expand Down Expand Up @@ -1128,6 +1137,10 @@ private void DecodeWellKnownAttributeAppliedToMethod(ref DecodeWellKnownAttribut
arguments.Diagnostics.Add(ErrorCode.ERR_SecurityCriticalOrSecuritySafeCriticalOnAsync, arguments.AttributeSyntaxOpt.Location, arguments.AttributeSyntaxOpt.GetErrorDisplayName());
}
}
else if (attribute.IsTargetAttribute(this, AttributeDescription.SkipLocalsInitAttribute))
{
arguments.GetOrCreateData<CommonMethodWellKnownAttributeData>().HasSkipLocalsInitAttribute = true;
}
else
{
var compilation = this.DeclaringCompilation;
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,11 @@ public override DllImportData GetDllImportData()
return null;
}

public override bool AreLocalsZeroed
{
get { return true; }
}

internal override MarshalPseudoCustomAttributeData ReturnValueMarshallingInformation
{
get { return null; }
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedGlobalMethodSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@ public override DllImportData GetDllImportData()
return null;
}

public override bool AreLocalsZeroed
{
get { return true; }
}

internal override MarshalPseudoCustomAttributeData ReturnValueMarshallingInformation
{
get { return null; }
Expand Down
8 changes: 8 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedInstanceMethodSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ public sealed override bool IsImplicitlyDeclared
}
}

public override bool AreLocalsZeroed
{
get
{
return true;
}
}

internal override bool TryGetThisParameter(out ParameterSymbol thisParameter)
{
Debug.Assert(!IsStatic);
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedIntrinsicOperatorSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,11 @@ public override DllImportData GetDllImportData()
return null;
}

public override bool AreLocalsZeroed
{
get { throw ExceptionUtilities.Unreachable; }
Copy link
Member

Choose a reason for hiding this comment

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

Do we not generate any code for this symbol?

Copy link
Member

Choose a reason for hiding this comment

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

Looks like it's only used in the semantic model, so I suspect not.

}

internal override IEnumerable<Cci.SecurityAttribute> GetSecurityInformation()
{
return SpecializedCollections.EmptyEnumerable<Cci.SecurityAttribute>();
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedStaticConstructor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,11 @@ public override DllImportData GetDllImportData()
return null;
}

public override bool AreLocalsZeroed
{
get { return true; }
}

internal override MarshalPseudoCustomAttributeData ReturnValueMarshallingInformation
{
get { return null; }
Expand Down
8 changes: 8 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Wrapped/WrappedMethodSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@ public override bool HidesBaseMethodsByName
}
}

public override bool AreLocalsZeroed
{
get
{
return UnderlyingMethod.AreLocalsZeroed;
}
}

public override ImmutableArray<Location> Locations
{
get
Expand Down
Loading