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

Support Requires on type in NativeAot #68978

Merged
merged 7 commits into from
May 13, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
26 changes: 25 additions & 1 deletion src/coreclr/tools/Common/Compiler/Logging/MessageOrigin.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;
using System.Text;

using Internal.IL;
using Internal.TypeSystem;

namespace ILCompiler.Logging
Expand Down Expand Up @@ -35,6 +36,29 @@ public MessageOrigin(TypeSystemEntity memberDefinition, string fileName = null,
SourceColumn = sourceColumn;
}

public MessageOrigin(MethodIL origin, int ilOffset)
{
string document = null;
int? lineNumber = null;

IEnumerable<ILSequencePoint> sequencePoints = origin.GetDebugInfo()?.GetSequencePoints();
if (sequencePoints != null)
{
foreach (var sequencePoint in sequencePoints)
{
if (sequencePoint.Offset <= ilOffset)
{
document = sequencePoint.Document;
lineNumber = sequencePoint.LineNumber;
}
}
}
FileName = document;
MemberDefinition = origin.OwningMethod;
SourceLine = lineNumber;
SourceColumn = null;
}

public override string ToString()
{
if (FileName == null)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Reflection.Metadata;
using ILCompiler.Logging;
using ILLink.Shared;
using Internal.TypeSystem;
using Internal.TypeSystem.Ecma;

Expand Down Expand Up @@ -44,40 +50,110 @@ internal static string GetGenericParameterDeclaringMemberDisplayName(GenericPara
return ((TypeDesc)parent).GetDisplayName();
}

internal static string GetRequiresAttributeMessage(MethodDesc method, string requiresAttributeName)
internal static bool TryGetRequiresAttribute(TypeSystemEntity member, string requiresAttributeName, [NotNullWhen(returnValue: true)] out CustomAttributeValue<TypeDesc>? attribute)
{
var ecmaMethod = method.GetTypicalMethodDefinition() as EcmaMethod;
if (ecmaMethod == null)
return null;
attribute = default;
CustomAttributeValue<TypeDesc>? decoded = default;
switch (member)
{
case MethodDesc method:
var ecmaMethod = method.GetTypicalMethodDefinition() as EcmaMethod;
if (ecmaMethod == null)
return false;
decoded = ecmaMethod.GetDecodedCustomAttribute("System.Diagnostics.CodeAnalysis", requiresAttributeName);
break;
case MetadataType type:
var ecmaType = type as EcmaType;
if (ecmaType == null)
return false;
decoded = ecmaType.GetDecodedCustomAttribute("System.Diagnostics.CodeAnalysis", requiresAttributeName);
break;
default:
Debug.Fail(member.GetType().ToString());
break;
}
if (!decoded.HasValue)
return false;

attribute = decoded.Value;
return true;
}

var decoded = ecmaMethod.GetDecodedCustomAttribute("System.Diagnostics.CodeAnalysis", requiresAttributeName);
if (decoded == null)
return null;
internal static string GetRequiresAttributeMessage(CustomAttributeValue<TypeDesc> attribute)
{
if (attribute.FixedArguments.Length != 0)
return (string)attribute.FixedArguments[0].Value;

var decodedValue = decoded.Value;
return null;
}

if (decodedValue.FixedArguments.Length != 0)
return (string)decodedValue.FixedArguments[0].Value;
internal static string GetRequiresAttributeUrl(CustomAttributeValue<TypeDesc> attribute)
{
if (attribute.NamedArguments.Length != 0 && attribute.NamedArguments[0].Name == "Url")
return (string)attribute.NamedArguments[0].Value;

return null;
}

internal static string GetRequiresAttributeUrl(MethodDesc method, string requiresAttributeName)
/// <summary>
/// Determines if method is within a declared Requires scope - this typically means that trim analysis
/// warnings should be suppressed in such a method.
/// </summary>
/// <remarks>Unlike <see cref="DoesMethodRequires(MethodDesc, string, out CustomAttributeValue?)"/>
/// if a declaring type has Requires, all methods in that type are considered "in scope" of that Requires. So this includes also
/// instance methods (not just statics and .ctors).</remarks>
internal static bool IsMethodInRequiresScope(this MethodDesc method, string requiresAttribute)
Copy link
Member

Choose a reason for hiding this comment

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

I would call this IsInRequiresScope - that's the name we use in the analyzer - so it will be easier to lookup the matching functionality there. (The body does match the behavior, so the name might as well)

Copy link
Member

Choose a reason for hiding this comment

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

I see the linker calls this IsMethodInRequiresUnreferencedCodeScope...
In that case I don't mind the current name, but we should then rename the one in the analyzer to match.
And please add a comment which methods this is a counterpart for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opened dotnet/linker#2788 to track the changes between linker/analyzer

{
var ecmaMethod = method.GetTypicalMethodDefinition() as EcmaMethod;
if (ecmaMethod == null)
return null;
if (method.HasCustomAttribute("System.Diagnostics.CodeAnalysis", requiresAttribute) && !method.IsStaticConstructor)
return true;

if (method.OwningType is TypeDesc type && TryGetRequiresAttribute(type, requiresAttribute, out _))
return true;

var decoded = ecmaMethod.GetDecodedCustomAttribute("System.Diagnostics.CodeAnalysis", requiresAttributeName);
if (decoded == null)
return null;
return false;
}

var decodedValue = decoded.Value;
/// <summary>
/// Determines if method requires (and thus any usage of such method should be warned about).
/// </summary>
/// <remarks>Unlike <see cref="IsMethodInRequiresScope(MethodDesc, string)"/> only static methods
/// and .ctors are reported as requires when the declaring type has Requires on it.</remarks>
internal static bool DoesMethodRequires(this MethodDesc method, string requiresAttribute, [NotNullWhen(returnValue: true)] out CustomAttributeValue<TypeDesc>? attribute)
tlakollo marked this conversation as resolved.
Show resolved Hide resolved
{
attribute = null;
if (method.IsStaticConstructor)
return false;

if (decodedValue.NamedArguments.Length != 0 && decodedValue.NamedArguments[0].Name == "Url")
return (string)decodedValue.NamedArguments[0].Value;
if (TryGetRequiresAttribute(method, requiresAttribute, out attribute))
return true;

return null;
if ((method.Signature.IsStatic || method.IsConstructor) && method.OwningType is not null &&
TryGetRequiresAttribute(method.OwningType, requiresAttribute, out attribute))
return true;

return false;
}

internal static bool DoesFieldRequires(this FieldDesc field, string requiresAttribute, [NotNullWhen(returnValue: true)] out CustomAttributeValue<TypeDesc>? attribute)
{
if (!field.IsStatic || field.OwningType is null)
{
attribute = null;
return false;
}

return TryGetRequiresAttribute(field.OwningType, requiresAttribute, out attribute);
}

internal static bool DoesMemberRequires(this TypeSystemEntity member, string requiresAttribute, [NotNullWhen(returnValue: true)] out CustomAttributeValue<TypeDesc>? attribute)
{
attribute = null;
return member switch
{
MethodDesc method => DoesMethodRequires(method, requiresAttribute, out attribute),
FieldDesc field => DoesFieldRequires(field, requiresAttribute, out attribute),
_ => false
};
}
}
}
Loading