Skip to content

Deduplicate polyfill type forwarding via centralized PolyfillTypeForwarders.cs#8596

Closed
Copilot wants to merge 2 commits into
mainfrom
copilot/duplicate-code-typeforwardedto-attribute
Closed

Deduplicate polyfill type forwarding via centralized PolyfillTypeForwarders.cs#8596
Copilot wants to merge 2 commits into
mainfrom
copilot/duplicate-code-typeforwardedto-attribute

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 26, 2026

Polyfill files in src/Polyfills repeated identical #else/#elif NETCOREAPP forwarding patterns with only forwarded types differing. This change consolidates forwarding declarations into one file while keeping existing conditional behavior and polyfill implementations intact.

  • Centralized forwarders

    • Added src/Polyfills/PolyfillTypeForwarders.cs as the single location for assembly-level TypeForwardedTo declarations.
    • Preserves framework guards:
      • #if NETCOREAPP for runtime-provided types
      • #if NET10_0_OR_GREATER for CompilerLoweringPreserveAttribute
  • Removed duplicated per-file forwarding blocks

    • Deleted forwarding-only #else / #elif NETCOREAPP branches from affected polyfill source files.
    • Left non-forwarding code paths unchanged (polyfill definitions remain where they are).
  • Consistency cleanup

    • Uses fully-qualified System.Runtime.CompilerServices.TypeForwardedTo(...) uniformly (eliminates mixed short/fully-qualified forms such as IsExternalInit’s previous short form).
// src/Polyfills/PolyfillTypeForwarders.cs
#if NETCOREAPP
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(System.Runtime.CompilerServices.IsExternalInit))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(System.Index))]
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(System.Range))]
#endif

Copilot AI requested review from Copilot and removed request for Copilot May 26, 2026 05:44
Copilot AI linked an issue May 26, 2026 that may be closed by this pull request
16 tasks
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot May 26, 2026 05:55
Copilot AI changed the title [WIP] Fix duplicate TypeForwardedTo assembly attribute declarations Deduplicate polyfill type forwarding via centralized PolyfillTypeForwarders.cs May 26, 2026
Copilot AI requested a review from Evangelink May 26, 2026 05:56
@Evangelink Evangelink marked this pull request as ready for review May 26, 2026 06:48
Copilot AI review requested due to automatic review settings May 26, 2026 06:48
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the src/Polyfills area by consolidating assembly-level TypeForwardedTo declarations into a single, centralized file (PolyfillTypeForwarders.cs). This reduces duplicated conditional forwarding blocks across many individual polyfill implementations while preserving the existing conditional behavior for when polyfills vs. BCL types are used.

Changes:

  • Added src/Polyfills/PolyfillTypeForwarders.cs to centralize all TypeForwardedTo assembly attributes (guarded by the same TFMs/preprocessor symbols).
  • Removed per-file #else / #elif NETCOREAPP forwarding-only blocks from affected polyfill files, leaving the polyfill implementations unchanged.
  • Standardized forwarding declarations to use fully-qualified System.Runtime.CompilerServices.TypeForwardedTo(...).
Show a summary per file
File Description
src/Polyfills/PolyfillTypeForwarders.cs New centralized location for assembly-level type forwarders (NETCOREAPP + NET10_0_OR_GREATER guards).
src/Polyfills/UnreachableException.cs Removed per-file NETCOREAPP forwarding branch; forwarding now centralized.
src/Polyfills/UnconditionalSuppressMessageAttribute.cs Removed per-file NETCOREAPP forwarding branch; forwarding now centralized.
src/Polyfills/StackTraceHiddenAttribute.cs Removed per-file NETCOREAPP forwarding branch; forwarding now centralized.
src/Polyfills/RequiredMemberAttribute.cs Removed per-file NETCOREAPP forwarding branch; forwarding now centralized.
src/Polyfills/Range.cs Removed per-file NETCOREAPP forwarding branch; forwarding now centralized.
src/Polyfills/PlatformAttributes.cs Removed per-file NETCOREAPP forwarding branch; forwarding now centralized.
src/Polyfills/NullableAttribtues.cs Removed per-file NETCOREAPP forwarding branch; forwarding now centralized.
src/Polyfills/ModuleInitializerAttribute.cs Removed per-file NETCOREAPP forwarding branch; forwarding now centralized.
src/Polyfills/IsExternalInit.cs Removed per-file NETCOREAPP forwarding branch; forwarding now centralized (also eliminates the short-form attribute usage).
src/Polyfills/InterpolatedStringHandlerAttribute.cs Removed per-file NETCOREAPP forwarding branch; forwarding now centralized.
src/Polyfills/InterpolatedStringHandlerArgumentAttribute.cs Removed per-file NETCOREAPP forwarding branch; forwarding now centralized.
src/Polyfills/Index.cs Removed per-file NETCOREAPP forwarding branch; forwarding now centralized.
src/Polyfills/ExperimentalAttribute.cs Removed per-file NETCOREAPP forwarding branch; forwarding now centralized.
src/Polyfills/DynamicallyAccessedMemberTypes.cs Removed per-file NETCOREAPP forwarding branch; forwarding now centralized.
src/Polyfills/DynamicallyAccessedMembersAttribute.cs Removed per-file NETCOREAPP forwarding branch; forwarding now centralized.
src/Polyfills/CompilerLoweringPreserveAttribute.cs Removed per-file forwarding branch; forwarding now centralized under NET10_0_OR_GREATER.
src/Polyfills/CompilerFeatureRequiredAttribute.cs Removed per-file NETCOREAPP forwarding branch; forwarding now centralized.
src/Polyfills/CallerArgumentExpressionAttribute.cs Removed per-file NETCOREAPP forwarding branch; forwarding now centralized.

Copilot's findings

  • Files reviewed: 19/19 changed files
  • Comments generated: 0

Comment on lines +5 to +6
#if NETCOREAPP
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(System.Runtime.CompilerServices.CallerArgumentExpressionAttribute))]
Copy link
Copy Markdown
Member

@Youssef1313 Youssef1313 May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be done this way.

Each polyfill knows exactly the TFM it's polyfilling for and should do the forwarding under #else.

Even all current polyfills are under #if !NETCOREAPP, that's just a coincidence. Also it's much easier to see the type forwarding in the same file that implements the polyfill, so I can easily see that no polyfill is missing type forwarding.

@Evangelink
Copy link
Copy Markdown
Member

Rejecting

@Evangelink Evangelink closed this May 26, 2026
@Evangelink Evangelink deleted the copilot/duplicate-code-typeforwardedto-attribute branch May 26, 2026 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[duplicate-code] Duplicate Code: TypeForwardedTo Assembly Attribute Pattern

4 participants