Remove dynamic load of QueryDebugger #1968
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
(Just use new() instead.) This dynamic load has been unnecessary since commit 40e8696 modified CurrentPlatformEnlightenmentProvider to look for QueryDebugger in the assembly named System.Reactive. Before that, it used to look for one called System.Reactive.Debugger, in which case a dynamic load would have made sense. But since for almost 6 years now, it has been passing "System.Reactive" as the assembly name, there is absolutely no reason for this to be a dynamic load. The code that performs this dynamic load is also in System.Reactive, so the dynamic load is completely pointless. This only ever happened when the debugger was attached, but it has become problematic because assembly trimming doesn't like dynamic loads. If you set <PublishAot>true</PublishAot>, this dynamic loading code in CurrentPlatformEnlightenmentProvider causes a trim analysis warning (IL2057). The warning could safely be ignored, but it shouldn't happen in the first place. This change should stop that warning from emerging.
mwadams
approved these changes
Jun 14, 2023
HowardvanRooijen
approved these changes
Jun 14, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(Just use new() instead.)
Resolves #1942
This dynamic load has been unnecessary since commit 40e8696 modified CurrentPlatformEnlightenmentProvider to look for QueryDebugger in the assembly named System.Reactive. Before that, it used to look for one called System.Reactive.Debugger, in which case a dynamic load would have made sense. But since for almost 6 years now, it has been passing "System.Reactive" as the assembly name, there is absolutely no reason for this to be a dynamic load. The code that performs this dynamic load is also in System.Reactive, so the dynamic load is completely pointless.
This only ever happened when the debugger was attached, but it has become problematic because assembly trimming doesn't like dynamic loads. If you set true, this dynamic loading code in CurrentPlatformEnlightenmentProvider causes a trim analysis warning (IL2057). The warning could safely be ignored, but it shouldn't happen in the first place. This change should stop that warning from emerging.