Consolidate generated expression classes into partial classes#3
Merged
Consolidate generated expression classes into partial classes#3
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates ExpressiveSharp’s codegen naming so multiple generated “expression factory” members for the same declaring type can be consolidated into a single static partial class, and adjusts runtime/registry lookup to find per-member expression factory methods by name.
Changes:
- Switch generated expression containers from “one class per member” to “one partial class per declaring type”, with per-member methods named like
Foo_P0_int_Expression()(and transformers asFoo_Transformers()). - Update
ExpressiveResolverto resolve expression factories viaGenerateClassFullName(...)+GenerateMethodSuffix(...) + "_Expression". - Extend the generator registry model/emitter to store and emit the expression factory method name per entry.
Reviewed changes
Copilot reviewed 218 out of 218 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ExpressiveSharp.Tests/Services/ExpressionClassNameGeneratorTests.cs | Adds unit coverage for new name/suffix generation behaviors. |
| src/ExpressiveSharp/Services/ExpressiveResolver.cs | Updates runtime reflection lookup to target consolidated partial classes and per-member expression methods/transformers. |
| src/ExpressiveSharp.Generator/Registry/ExpressionRegistryEntry.cs | Adds ExpressionMethodName to registry entries. |
| src/ExpressiveSharp.Generator/Registry/ExpressionRegistryEmitter.cs | Emits registry calls including expression method name and derives per-member transformer method name. |
| src/ExpressiveSharp.Generator/Emitter/ExpressionTreeEmitter.cs | Adjusts string.Concat(string,string) MethodInfo resolution fallback emission. |
| tests/ExpressiveSharp.Generator.Tests/**/*.verified.txt | Updates generator snapshot baselines to reflect consolidated partial classes and renamed expression/transformer methods (and inline reflection usage). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...yfillInterceptorGenerator/WhereTests.Where_SimpleCondition_GeneratesInterceptor.verified.txt
Show resolved
Hide resolved
src/ExpressiveSharp.Generator/Registry/ExpressionRegistryEmitter.cs
Outdated
Show resolved
Hide resolved
...iveSharp.Generator.Tests/ExpressiveGenerator/MethodTests.SimpleExpressiveMethod.verified.txt
Show resolved
Hide resolved
koenbeuk
force-pushed
the
feat/consolidated-generated-classes
branch
from
5f228a9 to
10cb0f6
Compare
![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: 9978ec1 | Previous: 4baf560 | Ratio |
|---|---|---|---|
ExpressiveSharp.Benchmarks.ExpressionReplacerBenchmarks.Replace_DeepChain |
13002.490152994791 ns (± 6425.537256203266) |
8015.039352416992 ns (± 559.6747508158047) |
1.62 |
ExpressiveSharp.Benchmarks.TransformerBenchmarks.ExpandExpressives_FullPipeline |
13514.363464355469 ns (± 6951.544460967931) |
8102.771881103516 ns (± 201.41206292411348) |
1.67 |
ExpressiveSharp.Benchmarks.GeneratorBenchmarks.RunGenerator(ExpressiveCount: 1) |
2714891.921875 ns (± 1060104.9464125987) |
934107.2679036459 ns (± 90659.51793884508) |
2.91 |
ExpressiveSharp.Benchmarks.GeneratorBenchmarks.RunGenerator_NoiseChange(ExpressiveCount: 1) |
2297760.6588541665 ns (± 875595.1862309682) |
1876945.7799479167 ns (± 152359.06827196968) |
1.22 |
ExpressiveSharp.Benchmarks.GeneratorBenchmarks.RunGenerator(ExpressiveCount: 100) |
134912102.7777778 ns (± 40366816.45858579) |
71178709.5 ns (± 1356732.741957425) |
1.90 |
ExpressiveSharp.Benchmarks.GeneratorBenchmarks.RunGenerator_Incremental_ExpressiveChange(ExpressiveCount: 100) |
99518402.58333333 ns (± 17769475.68744676) |
79640457.58333333 ns (± 13073268.121748274) |
1.25 |
This comment was automatically generated by workflow using github-action-benchmark.
- Add EndsWith guard around transformer method name Substring derivation in the generated Register helper for defense-in-depth - Re-add [EditorBrowsable(Never)] to generated partial classes via a single attribute-only partial file per unique class, emitted from the registry phase - Include generic/extension classes as metadata-only registry entries so they also receive the EditorBrowsable attribute - Add GenericClass_GetsEditorBrowsableAttribute test verifying generic classes get the attribute with correct type parameters Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
No description provided.