Refactor code structure for improved readability and maintainability#14
Refactor code structure for improved readability and maintainability#14
Conversation
koenbeuk
commented
Mar 30, 2026
koenbeuk
commented
- Removed dead code
- Simplified emitter by finding ways to reuse/unify emitters
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| if (isElementSelector) | ||
| delegateFqn2 = $"global::System.Func<{elemRef}, {thirdRef}>"; | ||
| else | ||
| delegateFqn2 = $"global::System.Func<{keyRef}, global::System.Collections.Generic.IEnumerable<{elemRef}>, {thirdRef}>"; |
There was a problem hiding this comment.
Pull request overview
Refactors the Polyfill interceptor source generator and expression-tree emitter plumbing to remove unused static-field emission paths and unify several emitter implementations, especially around anonymous-type handling.
Changes:
- Removed
EmitResult.StaticFieldsand related “static field declarations” emission from the generator pipeline. - Simplified
ReflectionFieldCache/ExpressionTreeEmitterconstruction and removed reflection-expression caching logic. - Unified several per-operator emitters (e.g.,
Select/indexedSelect) and consolidated anonymous-type alias handling helpers.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/ExpressiveSharp.Generator/PolyfillInterceptorGenerator.cs | Refactors interceptor emission, unifies emitters, and changes anonymous-type alias handling + lambda emission plumbing. |
| src/ExpressiveSharp.Generator/Emitter/ReflectionFieldCache.cs | Simplifies cache to always emit inline reflection expressions; removes internal caching/declarations API. |
| src/ExpressiveSharp.Generator/Emitter/ExpressionTreeEmitter.cs | Updates emitter to the new ReflectionFieldCache / EmitResult APIs. |
| src/ExpressiveSharp.Generator/Emitter/EmitResult.cs | Removes StaticFields from the emit result model. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Build generic Func<> — map each func type arg to T0, T1, etc. | ||
| var funcArgStrings = new string[funcTypeArgs.Length]; | ||
| for (int i = 0; i < funcTypeArgs.Length; i++) | ||
| funcArgStrings[i] = $"T{i}"; | ||
| var funcFqnGeneric = "global::System.Func<" + string.Join(", ", funcArgStrings) + ">"; | ||
|
|
||
| delegateFqn = "global::System.Func<" + | ||
| string.Join(", ", funcTypeArgs.Select((t, i) => | ||
| IsAnonymousType(t) ? funcArgStrings[i] : t.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat))) + ">"; |
There was a problem hiding this comment.
In EmitGenericSingleLambda's hasAnyAnon branch, funcFqnGeneric is built as Func<T0, T1, ...> based on the delegate arity, but typeParams only declares T0..T{method.TypeArguments.Length-1}. For common operators like Count/Any/TakeWhile (method type args = 1, Func args = 2), this yields emitted interceptor signatures like __Polyfill_Count_0<T0>(..., Func<T0, T1> _), which won’t compile (undeclared T1) and also doesn’t match the call-site delegate type (should be Func<T0, bool>). Consider building the interceptor parameter type from the actual funcTypeArgs (substituting aliases for anonymous types) — e.g., reuse delegateFqn for the placeholder parameter — instead of synthesizing T{i} for each Func type argument.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'ExpressiveSharp Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: 0d1b29a | Previous: dc0d77d | Ratio |
|---|---|---|---|
ExpressiveSharp.Benchmarks.EFCoreQueryOverheadBenchmarks.WithExpressives_Method |
28162.139526367188 ns (± 9314.371390135553) |
21135.239705403645 ns (± 5378.281048071639) |
1.33 |
ExpressiveSharp.Benchmarks.GeneratorBenchmarks.RunGenerator_ExpressiveChange(ExpressiveCount: 100) |
124966579.33333333 ns (± 3421441.1373593393) |
90394538.5 ns (± 22293193.035217773) |
1.38 |
This comment was automatically generated by workflow using github-action-benchmark.
Resolve conflicts in ReflectionFieldCache.cs and PolyfillInterceptorGenerator.cs — both branches independently made the same functional changes (removed obsolete cache, renamed fieldPrefix to varPrefix), so the PR's refactored structure is preserved with main's changes incorporated. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
