Skip to content

Sync linker to NativeAOT#77894

Merged
vitek-karas merged 9 commits intodotnet:mainfrom
vitek-karas:SyncLinkerToAot
Nov 9, 2022
Merged

Sync linker to NativeAOT#77894
vitek-karas merged 9 commits intodotnet:mainfrom
vitek-karas:SyncLinkerToAot

Conversation

@vitek-karas
Copy link
Copy Markdown
Member

Sync latest changes from dotnet/linker to the shared code in NativeAOT and also to the code which is similar in NativeAOT.

Syncs NativeAOT to https://github.com/dotnet/linker/tree/e502e7255030dde029e984e397924338485d1079/

The only notable change is the switch to use ParameterProxy and related abstractions when operating with method parameters. This was introduced in linker to avoid bugs when using a simple integer to refer to parameters by index, since different parts of the system use different indexing (some have index 0 as the implicit this parameter, while others don't).

The places which are similar to code in linker I converted to use ParameterProxy and helpers. Other places which interact directly with the type system in NativeAOT codebase I kept using the underlying APIs as it seems to make more sense.

One test change - this modifies the infra for running linker tests to define NATIVEAOT when compiling any of the tests sources. This is needed to ifdef out a test targeting vararg which is not supported by NativeAOT.

This tries to avoid using Signature.Length and the array directly - to make the code more explicit.

There are still places left as-is intentionally since they interact with the rest of the type system like ParameterHandles and so on, where it makes sense to use the lower level abstractions.
One notable thing - this modifies the test infra to define NATIVEAOT when it compiles any test code. This was needed in one case since NativeAOT doesn't support varargs and fails to process the affected test - so this is used to ifdef the vararg related code.
@vitek-karas
Copy link
Copy Markdown
Member Author

dotnet/linker counterpart to this change is dotnet/linker#3101
which syncs the changes from runtime repo to the shared code back into the linker repo.

// We have to issue a warning, otherwise we could break the app without a warning.
// This is not the ideal warning, but it's good enough for now.
_diagnosticContext.AddDiagnostic(DiagnosticId.UnrecognizedParameterInMethodCreateInstance, calledMethod.GetParameterDisplayName(1), calledMethod.GetDisplayName());
_diagnosticContext.AddDiagnostic(DiagnosticId.UnrecognizedParameterInMethodCreateInstance, calledMethod.GetParameter((ParameterIndex)(1 + (calledMethod.HasImplicitThis() ? 1 : 0))).GetDisplayName(), calledMethod.GetDisplayName());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would this be equivalent? The intrinsic changing from static to non-static is about as likely as the parameter position changing in the first place. We'll probably have to implement this TODO eventually anyway for compat with linker once we have 1,000,000 trimming customer and the inevitable "one in a million" customer hits it.

Suggested change
_diagnosticContext.AddDiagnostic(DiagnosticId.UnrecognizedParameterInMethodCreateInstance, calledMethod.GetParameter((ParameterIndex)(1 + (calledMethod.HasImplicitThis() ? 1 : 0))).GetDisplayName(), calledMethod.GetDisplayName());
_diagnosticContext.AddDiagnostic(DiagnosticId.UnrecognizedParameterInMethodCreateInstance, calledMethod.GetParameter((ParameterIndex)1)).GetDisplayName(), calledMethod.GetDisplayName());

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Actually it will not be equivalent. This method is called for both Activator.CreateInstance(string assemblyName, string typeName) which is static, as well as AppDomain.CreateInstance(string assemblyName, string typeName) which is instance method. So we need this "offset" thing to point to the right parameter (typeName in this case).

vitek-karas and others added 2 commits November 7, 2022 05:50
…owAnnotations.cs

Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
Copy link
Copy Markdown
Member

@jtschuster jtschuster left a comment

Choose a reason for hiding this comment

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

Thank you!

@vitek-karas vitek-karas merged commit 2934824 into dotnet:main Nov 9, 2022
@vitek-karas vitek-karas deleted the SyncLinkerToAot branch November 9, 2022 09:25
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants