Resolve new ILLink warnings in Microsoft.Extensions.DependencyInjection #49887
Conversation
These two new warnings came in because of: 1. dotnet#48823 2. dotnet#47938
|
Tagging subscribers to this area: @eerhardt, @maryamariyan Issue DetailsThese two new warnings came in because of:
When we address #48488 we will be able to catch these in the future.
|
eerhardt
added
the
linkable-framework
|
Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @tannergooding, @sbomer Issue DetailsThese two new warnings came in because of:
When we address #48488 we will be able to catch these in the future.
|
The short answer is The whole story is with #49843 I tried removing a suppression in the DI code, and needed to run the linker to ensure the suppression I was removing was valid to remove (it wasn't). When looking at that warning, I noticed these two new warnings. |
|
I see, if we wanted, we could theoretically use the trimming tests to catch if any new warnings are introduced even for OOBs. What I'm thinking is we could add a trimming test for DI where we don't suppress linker warnings, and we pass in a Suppressions.xml that has only the warnings that we are expecting, that way the build would break if new ones were introduced. Of course the ideal solution is to run the linker against OOBs as well, but this could be a point in time fix if we wanted. |
That would only catch warnings in methods called by the trimming tests. It wouldn't catch warnings in other methods of the library. I guess I'd prefer if we were to spend time, we would spend it on fixing #48488 for real. |
| @@ -29,6 +30,8 @@ private DependencyInjectionEventSource() : base(EventSourceSettings.EtwSelfDescr | |||
| // - A stop event's event id must be next one after its start event. | |||
| // - Avoid renaming methods or parameters marked with EventAttribute. EventSource uses these to form the event object. | |||
|
|
|||
| [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", | |||
There was a problem hiding this comment.
/cc @LakshanF as an FYI - otherwise ignore this comment 😉
src/libraries/Microsoft.Extensions.DependencyInjection/src/DependencyInjectionEventSource.cs
Show resolved
Hide resolved
ghost
locked as resolved and limited conversation to collaborators
These two new warnings came in because of:
When we address #48488 we will be able to catch these in the future.