Conversation
Member
|
@copilot : Please proceed |
Contributor
Author
|
@T-Gro Unfortunately I hit an unexpected error while processing your comment. I've automatically reported this to GitHub. You can ask me to try again later by mentioning me in a new comment. If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: Sorry for the inconvenience! |
Member
|
@copilot : Please proceed |
Tests added following TDD methodology: - Instance extension members with same simple name should error - Extension members on generic types (List<'T>) should error - Different member names but same type simple name should error - Extensions in nested modules should succeed (separate IL containers) - Multiple extension members for same duplicate type errors per member All 26 ExtensionMethodTests pass, 4891 component tests pass with 0 failures.
Change duplicate extension member detection to compare CompilationPath.MangledPath instead of Stamp. This correctly allows extensions for Expr and Expr<'T> (same namespace, different generic arity) while still detecting conflicts for types with same simple name but different namespaces (e.g., System.Threading.Tasks.Task vs MyModule.Task). Fixes test.fsx eval test failures caused by false positive FS3356 errors.
Change the grouping in CheckForDuplicateExtensionMemberNames from DisplayNameCore to LogicalName. The LogicalName includes the generic arity suffix (e.g., Expr`1 for Expr<'T>) which matches how types are compiled to IL. This fixes the false positive where extending both Expr and Expr<'T> from the same module was incorrectly flagged as a duplicate extension error. These types have different LogicalNames (Expr vs Expr`1) so they compile to different IL container types. The check now: 1. Groups extension members by MemberApparentEntity.LogicalName 2. For each group, checks if members extend types from different namespaces/assemblies 3. Only errors if types with same LogicalName come from different fully qualified paths This allows: - Expr and Expr<'T> extensions in the same module (different LogicalNames) This still errors: - System.Threading.Tasks.Task and MyModule.Task extensions in same module (same LogicalName 'Task', different namespaces) - NS1.List<'T> and NS2.List<'T> extensions in same module (same LogicalName 'List`1', different namespaces) Fixes the eval-FSI and eval-FSC_OPTIMIZED test failures.
Contributor
|
abonie
approved these changes
Jan 22, 2026
This was referenced Jan 22, 2026
T-Gro
added a commit
that referenced
this pull request
Feb 9, 2026
…plicate type name conflict Instance extension members compile with the extended type as the first IL parameter, so they can never produce duplicate IL method signatures even when the simple type name matches. Only static extension members lack this distinguishing parameter. This fixes a regression where libraries like IcedTasks that define instance (inline) extension members on builder types from different namespaces (e.g. Microsoft.FSharp.Control.TaskBuilderBase and IcedTasks.TaskBase.TaskBuilderBase) were incorrectly rejected. Regression introduced by PR #18821 (commit e948a68).
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.
Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.
Original description:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.