-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Move linker to runtime #77149
Move linker to runtime #77149
Conversation
Commit migrated from dotnet/linker@982e315
Add EnableAotAnalyzer as compiler visible Commit migrated from dotnet/linker@24373af
Commit migrated from dotnet/linker@0cb8b9d
Changes "ExpectedWarning" to "ExpectedNoWarnings" Commit migrated from dotnet/linker@45b481f
…#2639) Other than sharing more code and adapting it so that it works on both linker and analyzer, this change brings simple analysis of constant integer values in the analyzer. This is necessary to make most reflection API calls recognize binding flags. For example `GetMethods(BindingFlags.Public | BindingFlags.Static)`. So this change adds analysis of constant values (as recognized by the Roslyn's operation tree) and the OR binary operator for integers and enums. Added some new tests for the binding flags handling. Reenabled some disabled tests for analyzer. Moved the main affected tests from the generated source files to the hardcoded one and force them to exact match of warnings for both linker and analyzer. Commit migrated from dotnet/linker@f0bd2ae
…224.4 (dotnet/linker#2663) [main] Update dependencies from dotnet/arcade Commit migrated from dotnet/linker@54c94cb
…0227.1 (dotnet/linker#2664) [main] Update dependencies from dotnet/runtime Commit migrated from dotnet/linker@396c37d
* Fix behavior of intrinsics with null or empty inputs Add tests for intrinsics receiving null or empty values, and tweak a few intrinsics to avoid warnings for these cases. This fixes some unnecessary warnings, and also fixes a crash in the linker. Some of the new shared intrinsics that don't produce type values need to have some tracked value. Presumably this should really be Unknown, but currently they fall back on the shared logic to track a value with annotations (which for these intrinsics will be None). A few unnecessary warnings are left as-is to avoid changing the linker behavior. Commit migrated from dotnet/linker@6aa9837
Shares two more intrinsics, with supporting infra. Fixed a bug in the intrinsics - passing null to the name of a property/field will throw at runtime, so no need to validate anything. Modifies the existing tests to add warnings, since that is the only verifyable behavior for the analyzer. Commit migrated from dotnet/linker@8c0df91
Commit migrated from dotnet/linker@75235a5
…304.3 (dotnet/linker#2676) [main] Update dependencies from dotnet/arcade Commit migrated from dotnet/linker@14b3d62
…0307.1 (dotnet/linker#2677) [main] Update dependencies from dotnet/runtime Commit migrated from dotnet/linker@26e0c5c
Commit migrated from dotnet/linker@5baeb5e
* Use attributes on lambdas in tests * Fix formatting * Unindent attributes Commit migrated from dotnet/linker@93eac59
Commit migrated from dotnet/linker@98c81d5
* Warn on DAM mismatch between overrides Commit migrated from dotnet/linker@ac6cfb3
…307.6 (dotnet/linker#2684) [main] Update dependencies from dotnet/arcade Commit migrated from dotnet/linker@cbcfcfc
…0313.2 (dotnet/linker#2685) [main] Update dependencies from dotnet/runtime Commit migrated from dotnet/linker@ae1f280
* Remove SuppressionContextMember Instead of computing the suppression context member when we push to the scope stack, it is now computed on demand when we need to know whether a warning is suppressed. The suppression context should be entirely determined by the static scopes, so there isn't any need to track it dynamically. Commit migrated from dotnet/linker@2303da0
…tnet/linker#2675) Add intrinsic support for Nullable.GetUnderlyingType and support for MakeGenericType with Nullables Adds ArrayCreationOperation visitors to create ArrayValue's in the analyzer, and adds start of dataflow analysis for array values. Adds tests to validate dataflow in Arrays. Co-authored-by: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Commit migrated from dotnet/linker@ed8b22a
Commit migrated from dotnet/linker@1d77412
* Add test which mimics what is done in Type.ImplementInterface helper in runtime There's nothing new in this test, but it's better to have coverage for the pattern as it's used in runtime. * Add tests for properties * Simplify Co-authored-by: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Commit migrated from dotnet/linker@5929598
…318.2 (dotnet/linker#2697) [main] Update dependencies from dotnet/arcade Commit migrated from dotnet/linker@9f890c6
…0321.1 (dotnet/linker#2698) [main] Update dependencies from dotnet/runtime Commit migrated from dotnet/linker@64ad0ff
tagging @ViktorHofer and @dotnet/area-infrastructure-libraries so that somebody on the infrastructure side can take a peek at some of the infra changes to make sure they are all right. @tlakollo is this code ready for review? |
This is draft so I guess you don't want a review yet? If you want one, which commits would you want to be reviewed? Presumably, not the ones that are cherry-picked from dotnet/linker. |
At this moment I'm just experimenting with how the infrastructure behaves by inserting the linker repo. Basically, make sure that none of the changes breaks the runtime in any way even if the repo is not running in CI. Then how it will behave after the testing subset is enabled, and so on. |
I'm so sorry, I missed that it was a draft PR. I have pinged @tlakollo and indeed I don't think a review here is required as this is mostly to iterate on CI for testing. |
I added a motivation section on #75278 to give more info about why this is being prototyped |
Change source lines since now we are using spaces instead of tabs (pending issue with pdbs and symbols on assemblies) Workaround the deletion of RefSafetyRulesAttribute and their associated types injected by the compiler For now add extra ExpectedWarnings on warnings being duplicated by the analyzer (pending investigation)
…related tests will fail Upgrade to MicrosoftCodeAnalysisVersion 4.5+ generates a different type of analysis callbacks, the CheckAttributeInstantiation method is no longer needed. And a bug in which a warning was printed twice by the analyzer is fixed.
…e vs the microsoft internal package
Typo on Cecil.Pdb package Update names in csproj's Add warning for file header mismatch/missing
</ItemGroup> | ||
|
||
<ItemGroup Condition="$(_subset.Contains('+tools.linkertests+'))"> | ||
<ProjectToBuild Include="$(ToolsProjectRoot)linker\test\Mono.Linker.Tests\Mono.Linker.Tests.csproj" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should be built even in source build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, never mind, I guess we don't need tests for source build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is meant by "source build" please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a nutshell source-build is a requirement from some Linux distributions which require source code to build. So basically when they get dotnet they actually get all the source code (one of those pieces is the runtime, but they are more pieces) and build it. The source-build repository is in charge of taking all of these pieces and glue them together, it also has an explanation of what source build is -> /~https://github.com/dotnet/source-build#source-build-goals
We want all the source code to be included, but the tests are not going to be executed once it's an app like dotnet so we exclude them using the DotNetBuildFromSource variable.
Add condition for test group to any change in src/tools/linker Remove extra tool in global.json Add a dummy line change to test if CI triggers
…er changes Exclude building the clr in linker testing
Closing this in favor of getting a review into an updated and easier to read PR in #77569 |
This is a test-only draft PR and is not planned to be merged for more information about these changes please refer to #75278.