Jens-G opened a new pull request, #3813: URL: https://github.com/apache/thrift/pull/3813
Fixes [THRIFT-6198](https://issues.apache.org/jira/browse/THRIFT-6198). ### The problem `collect_extensions_types()` stops recursing at the program boundary since THRIFT-5998, but a program that uses a container type of its own still collects it — even when an included program uses the very same container. Both extension classes then declare the same `DeepCopy()` / `Equals()` / `GetHashCode()` signatures, and since both land in the same C# namespace, every call site becomes ambiguous: ``` error CS0121: The call is ambiguous between the following methods or properties: 'TypedefIncludeTestExtensions.DeepCopy(List<InnerStruct>)' and 'TypedefStructTestExtensions.DeepCopy(List<InnerStruct>)' ``` Reproduced against `master` with the ticket's IDL, compiled with `-gen netstd:net10`. Two observations that go beyond the ticket text: * **The typedef is not part of the trigger.** `collect_extensions_types()` calls `resolve_typedef()` before it computes the map key, so plain `list<A.InnerStruct>` on both sides fails exactly the same way. * **It is not limited to included structs.** Two programs that both use `list<i32>` collide identically — the element type never has to leave the base types. ### The fix Determine which container types an included program collects for itself, and drop those from our own set. That is only safe where the included program's extension class is actually reachable from our generated code, so an include is consulted only when * our generated code names at least one type declared in it — the two can then only ever be compiled together, and * its C# namespace is ours — its extension class is then in scope wherever ours is. Otherwise we keep our own copy, so no call site is ever left without a matching extension method. Verified against the cases that could go wrong: | Scenario | Result | |---|---| | Ticket repro (typedef, shared namespace) | duplicate dropped, compiles | | Same, without the typedef | duplicate dropped, compiles | | Only the including program uses the container | kept — the include has nothing to hand over | | Programs in **different** namespaces | both keep it — the other class is out of scope | | `list<i32>` in both, including program references the include | duplicate dropped | ### Deliberately not covered Two variants stay as they are, both predating this change, because neither can be decided from a single program's point of view: * a container over base types only, shared with an include that the including program otherwise makes no use of; * two programs that merely share a C# namespace with no include relation between them (e.g. two siblings of a common parent). Both produce the same CS0121 and would need a rule that spans the whole generator run rather than one program. Happy to file a follow-up ticket if you want those tracked. ### Tests `Thrift6198.thrift` + `Thrift6198.included.thrift`, wired into all four `Thrift.Compile` targets. They cover, in one build: * `list` / `set` / `map` over a struct from the included program — CS0121 without the fix * a base-type container used by both programs — CS0121 without the fix * a container only the including program uses — CS1061 if the new filter takes away too much Both directions therefore fail the build if the filter is ever wrong. ### Verification * `lib/netstd` solution builds clean, all four `Thrift.Compile` targets (net8 / net9 / net10 / netstd2) * `Thrift.Tests`: 84/84 pass * `test/netstd` cross-language test app builds * the ticket's IDL compiles as a standalone C# project 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
