Skip to content

Conversation

@jkoritzinsky
Copy link
Member

Add some annotations for ILC as discovered in #75793

@ghost ghost added the area-System.Xml label Sep 30, 2022
@ghost ghost assigned jkoritzinsky Sep 30, 2022
@jkoritzinsky jkoritzinsky added the linkable-framework Issues associated with delivering a linker friendly framework label Sep 30, 2022
@ghost
Copy link

ghost commented Sep 30, 2022

Tagging subscribers to this area: @dotnet/area-system-xml
See info in area-owners.md if you want to be subscribed.

Issue Details

Add some annotations for ILC as discovered in #75793

Author: jkoritzinsky
Assignees: -
Labels:

area-System.Xml

Milestone: -

@ghost
Copy link

ghost commented Sep 30, 2022

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr
See info in area-owners.md if you want to be subscribed.

Issue Details

Add some annotations for ILC as discovered in #75793

Author: jkoritzinsky
Assignees: jkoritzinsky
Labels:

area-System.Xml, linkable-framework

Milestone: -

}

[UnconditionalSuppressMessage("AotAnalysis", "IL3050:AotUnfriendlyApi",
Justification = "All types that are instantiated with this method are used elsewhere in this file in the implementations of the DatatypeImplementation abstract class.")]
Copy link
Member

Choose a reason for hiding this comment

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

I assume that this warning is about values.ToArray(_itemType.ValueType). I think it works, but it fragile and hard to see that it is correct.

I would suggest factoring out the code that produces the Aot warning into a local method, apply the warning onto the local method only, and use the array type to create the array to guarantee that it is kept. Something like:

array = CreateArray(ListValueType);
values.CopyTo(array, 0);

...
[UnconditionalSuppressMessage("AotAnalysis", "Array type is passed as an argument to this method, so it is guaranteed to exist.")]
static Array CreateArray(Type arrayType, int length)
{
    Array array = Array.CreateInstance(arrayType.GetElementType()!, length);
    Debug.Assert(array.GetType() == arrayType);
    return array;
}

Copy link
Member

Choose a reason for hiding this comment

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

Opened #76478 to make this better.

@jkoritzinsky jkoritzinsky requested a review from eerhardt October 3, 2022 21:23
@radical
Copy link
Member

radical commented Oct 3, 2022

/azp run runtime-wasm-libtests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

LGTM. It would be good to get sign off from a System.Xml owner.

@jkotas jkotas merged commit 6294ab1 into dotnet:main Oct 4, 2022
@jkoritzinsky jkoritzinsky deleted the linker-fixes-xml branch October 4, 2022 02:46
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Xml linkable-framework Issues associated with delivering a linker friendly framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants