Merged
Conversation
* From .NET FX 4.8 to .NET 10 * Code gen now null-aware * Removed unnecessary Qbservable.NAry.tt because the HomoIcon tool handles these methods just fine
* Replace MethodBase.GetCurrentMethod with delegate (because the trimmer understands the latter but not the former) * Suppress warning on ObservableEx * Use C# type names in comments for decimal and float, not runtime names Add new .Homoicon.cs extension to list skipped in license header test. (No longer using .Generated.cs because that was causing us not to see diagnostics for any of the HomoIcon-generated code.)
The IQbservable forms of the tuplet-based versions of these operators all go up to 16 arguments, but for some reason the IObservable forms go up only to 8. Since we've resurrected the HomoIcon tool, this discrepancy has caused a problem: the code gen only generates IQbservable forms corresponding to IObservable forms, and so we end up with the IQbservable API surface area shrinking. To avoid that, this expands the IObservable CombineLatest and Zip operators to match what IQbservable was already offering.
There was a problem hiding this comment.
Pull request overview
This PR modernizes the HomoIcon code-generation tool used to produce IQbservable<T>/query-space operator wrappers, aligning the generated code with .NET SDK trimming analysis and adding nullability-aware generation. It also expands IObservable<T> operator surface area (notably CombineLatest/Zip tuple overloads) to match long-standing IQbservable<T> capabilities.
Changes:
- Port HomoIcon to an SDK-style .NET tool (now
net10.0), and update its generated code to avoid trimmer warnings (delegate-basedMethodInfoacquisition instead ofGetCurrentMethod/MakeGenericMethodpatterns). - Rename HomoIcon-generated files from
*.Generated.csto*.Homoicon.cs(and adjust license-header scanning exclusions accordingly). - Extend
IObservable<T>/query languageCombineLatestandZiptuple-returning overloads up to 16 inputs to align with query-space APIs.
Reviewed changes
Copilot reviewed 26 out of 30 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| Rx.NET/tools/HomoIcon/Properties/AssemblyInfo.cs | Removes legacy .NET Framework assembly attribute file as part of SDK-style migration. |
| Rx.NET/tools/HomoIcon/Program.cs | Updates generator logic for trimming-friendly MethodInfo retrieval, file naming, TFM selection, and nullability emission. |
| Rx.NET/tools/HomoIcon/HomoIcon.slnx | Adds new solution format for the tool. |
| Rx.NET/tools/HomoIcon/HomoIcon.sln | Removes legacy VS2010 solution file. |
| Rx.NET/tools/HomoIcon/HomoIcon.csproj | Migrates the tool to SDK-style project targeting net10.0. |
| Rx.NET/tools/HomoIcon/app.config | Removes .NET Framework runtime config. |
| Rx.NET/Source/tests/Tests.System.Reactive/Tests/LicenseHeaderTest.cs | Excludes *.Homoicon.cs files from license-header scanning (like *.Generated.cs). |
| Rx.NET/Source/src/System.Reactive/System.Reactive.csproj | Removes T4 item metadata tied to deleted QbservableEx.NAry template output. |
| Rx.NET/Source/src/System.Reactive/Platforms/Desktop/Linq/Observable.Remoting.cs | Switches query expression MethodInfo retrieval to delegate-based approach for trimming friendliness. |
| Rx.NET/Source/src/System.Reactive/Linq/QueryLanguage.Multiple.Zip.tt | Expands generated Zip tuple overloads up to 16 inputs. |
| Rx.NET/Source/src/System.Reactive/Linq/QueryLanguage.Multiple.Zip.cs | Regenerates query language Zip overloads to match updated template. |
| Rx.NET/Source/src/System.Reactive/Linq/QueryLanguage.Multiple.CombineLatest.tt | Expands generated CombineLatest tuple overloads up to 16 inputs. |
| Rx.NET/Source/src/System.Reactive/Linq/QueryLanguage.Multiple.CombineLatest.cs | Regenerates query language CombineLatest overloads to match updated template. |
| Rx.NET/Source/src/System.Reactive/Linq/QbservableEx.NAry.tt | Removes legacy T4 generator for QbservableEx N-ary operators (superseded by HomoIcon output). |
| Rx.NET/Source/src/System.Reactive/Linq/QbservableEx.Homoicon.cs | Updates generated QbservableEx operators to trimming-friendly MethodInfo patterns and improved nullability pragmas/docs. |
| Rx.NET/Source/src/System.Reactive/Linq/QbservableEx.Generated.cs | Removes prior generated file in favor of *.Homoicon.cs naming/output. |
| Rx.NET/Source/src/System.Reactive/Linq/Qbservable.Joins.cs | Replaces GetCurrentMethod/MakeGenericMethod usage with delegate-based MethodInfo retrieval. |
| Rx.NET/Source/src/System.Reactive/Linq/Qbservable.cs | Replaces GetCurrentMethod/MakeGenericMethod usage with delegate-based MethodInfo retrieval. |
| Rx.NET/Source/src/System.Reactive/Linq/Observable.Multiple.Zip.tt | Expands generated ObservableEx.Zip tuple overloads up to 16 inputs (and adjusts CA1711 suppression placement). |
| Rx.NET/Source/src/System.Reactive/Linq/Observable.Multiple.Zip.cs | Regenerates ObservableEx.Zip overloads to match updated template. |
| Rx.NET/Source/src/System.Reactive/Linq/Observable.Multiple.CombineLatest.tt | Expands generated ObservableEx.CombineLatest tuple overloads up to 16 inputs (and adjusts CA1711 suppression placement). |
| Rx.NET/Source/src/System.Reactive/Linq/Observable.Multiple.CombineLatest.cs | Regenerates ObservableEx.CombineLatest overloads to match updated template. |
| Rx.NET/Source/src/System.Reactive/Linq/IQueryLanguage.NAry.tt | Expands generated interface IQueryLanguageEx signatures to include tuple overloads up to 16 inputs. |
| Rx.NET/Source/src/System.Reactive/Linq/IQueryLanguage.NAry.cs | Regenerates IQueryLanguageEx N-ary signatures to match updated template. |
| Rx.NET/Source/src/System.Reactive/Linq/FuncExtra.cs | Adds a 17-argument Func delegate type to support delegate-based MethodInfo retrieval beyond BCL Func arity. |
| Rx.NET/Source/src/System.Reactive.Observable.Aliases/Qbservable.Aliases.Homoicon.cs | Updates generated alias query operators (nullability pragmas/imports and regenerated header). |
| Rx.NET/Documentation/adr/0008-trimming-vs-qbservable.md | Adds ADR documenting trimming warnings and the delegate-based mitigation strategy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| Whether an operator creates a brand new source from scratch (as in `Range`) or bolts onto an existing sequence (as in `Where`), these `IQbservable<T>` operators are all implemented in much the same way: they call the `IQbservableProvider.CreateQuery<int>` method passing an `Expression` describing the query. (In the case of combinators like `Where`, they obtain the `IQbservableProvider` from the `Provider` property of the incoming source). | ||
|
|
||
| This expression is available as part of `IQbservable<T>` (or more precisely, its base type, `IQbservable`), so we can inspect the expression for an `IQbserable<T>`: |
Collaborator
HowardvanRooijen
left a comment
HowardvanRooijen
left a comment
There was a problem hiding this comment.
Have you created an integration test for running behaviour successfully on a trimmed assembly?
Collaborator
Author
I considered this but it was rather beyond the scope of this particular PR: the primary goal was to preserve existing functionality but to remove build warnings. To test trimming properly I think we'd need to broaden the integration testing. We'd want to include Blazor since that was the original motivation for adding trimming annotations in the first place. Since wasm testing still appears to be a pain, that's not straightforward and might not even be possible yet. (I've not yet seen anything to indicate that the new test platform did anything to improve testability on WASM. I would hope that it did, and if so, migrating to that would be a prerequisite.) So I've created #2310 to track this. |
HowardvanRooijen
approved these changes
Mar 25, 2026
This file contains hidden or 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
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.
This updates the tool that generates code supporting
IQueryable<T>(the HomoIcon tool) so that the generated code is better aligned with the capabilities of the .NET SDK's IL trimming functionality.The most obvious change is that this fixes some build warnings reporting trimmability analysis problems. But it has also enabled us to remove the suppressions for various diagnostics that had been added as a stopgap to get basic trimming support working in Rx 6. This may also enable the trimming static analysis to better understand the relevant methods.
This also finally drags the HomoIcon tool into the modernish era: it's now a .NET 10 tool instead of .NET FX, and it is also able to generate nullability annotations. (For several years, nullability had been handled by error-prone manual adjustments to the files this tool generates.)
One upshot of updating this tool is that it has revealed inconsistencies between the
IQbservable<T>andIObservable<T>APIs, most notably the fact that there were a load of overloads available forCombineLatestandZiponIQbservable<T>that did not haveIObservable<T>equivalents! (Specifically, the forms that return tuples with more than 8 values.) The mainIObservable<T>operators have now been extended to align with what the query-space versions have apparently offered for years.