feat: claim binding call sites through interceptors - #89
Merged
Conversation
- RXUIBIND011 reports a call that extension-method lookup sent to ReactiveUI's mixin rather than a generated overload. Nothing reported this before: the extractors recognise a call by its declaring type, so such a call was never one of ours, generated no dispatch, and took the runtime expression engine with the build still green. - A file reaches that state by importing ReactiveUI for ReactiveCommand alongside this package, or by an editor's import cleanup removing the binding import that the generated overloads had made look unused. - The declaring type is read after walking out of any nested type, so an extension declared in an extension block is recognised by the class the consumer wrote rather than by the synthesized one holding its members. Closes #87
… overload - The scenario imported ReactiveUI and not this package, so its WhenAnyValue call reached ReactiveUI's mixin and exercised the reflection engine rather than the generated observation the scenario is named for. - Found by RXUIBIND011 on its first run over the repository.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #89 +/- ##
========================================
Coverage 99.48% 99.48%
========================================
Files 246 250 +4
Lines 10410 10651 +241
Branches 1125 1180 +55
========================================
+ Hits 10356 10596 +240
Misses 42 42
- Partials 12 13 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…iler allows - Compile the generator a second time against Roslyn 4.13, the first release where the call-site description API is supported rather than experimental. The two builds share every source file and differ only in what ROSLYN_4_13 turns on. - Where that build is loaded and the consumer has opted the generated namespace into interception, an observation claims its call site by name instead of competing for it through extension-method lookup. That removes the namespace placement, the generated import and the per-call expression-text match. - A call site the compiler declines to describe is left alone, which is the same outcome an out-of-reach dispatch overload already produces. - Read the description in one helper and write the attribute in one emitter, so neither build grows its own copy and no API-specific code repeats it. Towards #81
…ation APIs - Move the grouping, the attribute and the parameter list into InterceptorEmitter, so WhenAny reuses them rather than repeating them and each API supplies only the signature it is called with. - Claim WhenAny call sites through the same path as WhenChanged, WhenChanging and WhenAnyValue.
…eptors - BindOneWay, BindTwoWay, OneWayBind and Bind already describe their output through one dispatch descriptor, so the interceptor shape is written once against that descriptor rather than four times. - The interceptor takes what the call site passes and forwards to the worker, so the dispatch parameters and the run-time selector-text match go away.
…sites - Emit interceptors for BindTo, BindCommand and BindInteraction, completing the tier across every generated API. - Share the parameter writers between each API's dispatch overload and its interceptor, so one description of a signature serves both.
- Give every interceptor the signature of the stub it replaces, including the receiver and the caller-info parameters, which is what the compiler requires of one. - Emit each API's parameter list from one writer serving both its dispatch overload and its interceptor. - Ship the generator and analyzer in a slot per compiler generation, with targets that keep a build handed both from loading both and that list the generated namespace where interception can be honoured. - Stay silent on the reach of a dispatch overload where the call site is claimed outright instead. - Stop reporting a stream's value type as unobservable behind BindTo, whose first type argument names no object anything observes. - Run the generator and analyzer suites against both compiler builds. - Replace the substituted symbols with ones a compilation produces: a function pointer's signature belongs to no type, and an extension block declares its members in a type with no name.
- Set out the interceptor and overload mechanisms, what each package slot carries, and the settings a consumer has. - List BindTo among the supported APIs. - Assert the shipped slots against the versions the targets select between. - Measure a generation pass with and without the interception opt-in, against both compiler builds.
- Pass the language-feature snapshot as an argument rather than closing over it, so the writer each observation API supplies captures nothing and the compiler caches one delegate for the whole compilation. - Drop the dependency group entry for a package nothing references.
glennawatson
changed the title
- Name a defined language version instead of the moving preview, which the test-only Roslyn bump to 5.9.0 makes expressible. - Exercise all twelve binding APIs through the interceptor tier, so every emitter's interceptor path is emitted and its signature validated. - Cover RXUIBIND009 for a build that lists the generated namespace and for one that lists another package's. - Let a scenario hand parse options straight to the analyzer helper, which is how the interception opt-in is expressed.
- Ask one question where the generator and the analyzer each asked two: a compiler that cannot describe a call site answers before the opt-in is read, so neither has to pair the two tests itself. - Drop a null guard on a named type's namespace. A named type always has one, and the shapes that do not - an array, a pointer, a function pointer - are not named types and never arrive there. - Claim a BindCommand with an observable parameter and a WhenAnyObservable with a selector, so both interceptor bodies are emitted and compiled. - Read a call site the compiler declines to describe, which is the answer every caller already handles. - Say what the array case proves, rather than naming it for a guard it never reaches.
- Compare the Android widget names in order rather than switching over them, so each comparison is a branch a caller can reach rather than a bucket of a jump table the compiler derived. - Give TimePicker's four names a predicate that says why there are four: the hour and the minute are spelled one way from API 23 and another before it.
|
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
1 participant
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.
What kind of change does this PR introduce?
Feature: compile-time bindings reach every call site, plus the analyzer diagnostic for the calls that never
reached the generator at all.
What is the new behavior?
A binding call site is claimed by an interceptor wherever the compiler can honour one.
it. Nothing about the redirection goes through extension-method lookup, so it serves the call sites an
overload cannot: a file declared outside the project's root namespace, and a project below C# 10 whose
assembly exposes its internals.
<LangVersion>7.3</LangVersion>project on a current SDK gets it.WhenChanged,WhenChanging,WhenAnyValue,WhenAny,WhenAnyObservable,BindOneWay,BindTwoWay,OneWayBind,Bind,BindTo,BindCommandandBindInteraction.parameters, which is what the compiler requires of one. Each API writes that list once, and the same writer
serves its dispatch overload.
<ReactiveUIBindingUseInterceptors>false</ReactiveUIBindingUseInterceptors>keeps the overloads on acompiler that could intercept.
Each runtime package carries the generator and its analyzer once per compiler generation.
analyzers/dotnet/roslyn4.8/csandanalyzers/dotnet/roslyn4.13/cs. The .NET SDK resolves the highest slotat or below the compiler building the project.
compiler is not served by, so the generator is not loaded twice and no dispatch file is emitted twice.
with nothing to point at. The targets fail the build with
RXUIBIND100naming both versions instead.RXUIBIND011 reports a binding call that ReactiveUI's own mixin answered.
ReactiveUIand notReactiveUI.Bindingbinds to ReactiveUI's mixin, and every extractor recognises a call by its declaringtype, so that call is not one of ours: no dispatch is generated and no other diagnostic has anything to say
about it.
block is recognised by the class the consumer wrote rather than the synthesized one its members belong to.
RXUIBIND009 stays silent where the call site is claimed outright. The reach of a dispatch overload decides
nothing when no overload is what serves the call.
RXUIBIND002 stays silent behind
BindTo, whose first type argument names the value type of a stream thecaller already built rather than an object anything observes.
The suites run against both compiler builds. The generator and analyzer test projects each have a mirror
compiled against Roslyn 4.13, so the tier a build gets is asserted on the build that gets it.
NSubstitute is gone. The symbol shapes those tests needed come from a compilation instead: a function
pointer's signature belongs to no type, an array type to no namespace, and an extension block declares its
members in a type with no name.
What is the current behavior?
Every call site is reached by a concrete overload competing in extension-method lookup. A file outside the root
namespace, or a project below C# 10 whose assembly exposes its internals, is out of that overload's reach and
falls through to the runtime stub - which throws.
Closes #81
Closes #87
What might this PR break?
site from RXUIBIND011.
generator.
ReactiveUI.Binding.Generated.Interceptorsnamespace where interception is ineffect. Anything reaching into the generated dispatch class by name would not find it there; nothing in the
public surface does.
Checklist
mainbranchAdditional information
Verified against real builds rather than only the suite, since which tier a project gets is decided by the
compiler and not by anything in the repository:
net10.0net462,<LangVersion>7.3net462, both slots handed overnet462A generation pass measured with the EventPipe
GcVerboseprofiler allocates the same either way - 1.92 MB atone view-model pair, 29.54 MB at sixteen, 118.1 MB at sixty-four - and the trace attributes no allocation site
to this package at all. Every one it names is Roslyn binding the call: extension-method type-argument
inference, alpha renaming and parameter substitution.