Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jul 30, 2025

Problem

Types like Span<T> and ReadOnlySpan<T> were being incorrectly suppressed from IntelliSense completion lists due to having both ObsoleteAttribute and IsByRefLikeAttribute. The completion system was treating any type with ObsoleteAttribute as obsolete, regardless of other attributes.

// This would not show Span in completion
let test = System.Sp

Root Cause

The issue occurred because:

  1. Span<T> and ReadOnlySpan<T> have ObsoleteAttribute in .NET for specific technical reasons
  2. They also have IsByRefLikeAttribute to indicate they are stack-only types
  3. The completion system has two attribute checking paths: F# types use CheckFSharpAttributesForObsolete while .NET types use CheckILAttributesForUnseen
  4. Only the F# attribute checking path had the IsByRefLikeAttribute exclusion logic
  5. Since Span types are .NET types, they went through IL attribute checking and were incorrectly suppressed

Solution

Extended both attribute checking functions to exclude types marked with IsByRefLikeAttribute from being considered obsolete:

  • F# types: Updated CheckFSharpAttributesForObsolete in AttributeChecking.fs
  • IL/.NET types: Updated CheckILAttributesForUnseen in AttributeChecking.fs
  • Added attrib_IsByRefLikeAttribute_opt to TcGlobals using the established tryFindSysAttrib pattern
  • Treats the attribute as optional since not every runtime has it

Both functions now use the same logic: if a type has ObsoleteAttribute but also has IsByRefLikeAttribute, it should not be considered obsolete for completion purposes.

Testing

Added a unit test in CompletionTests.fs that verifies:

  • Typing System.Sp shows Span in completion suggestions
  • By-ref-like types are not suppressed despite having dual attributes
  • Test is conditioned to only run on .NET Core where Span<T> is available
  • All 37 completion tests pass successfully

This ensures that by-ref-like types remain visible and accessible to developers while maintaining the existing obsolete attribute behavior for other types.

This pull request was created as a result of the following prompt from Copilot chat.

Extend the CheckFSharpAttributesForObsolete function in src/Compiler/Checking/AttributeChecking.fs to exclude types marked with IsByRefLikeAttribute from being considered obsolete, even if ObsoleteAttribute is present. Add a code comment explaining why IsByRefLikeAttribute is excluded: specifically, this avoids improper suppression of types like Span and ReadOnlySpan in completion lists due to their dual attributes. Additionally, add a unit/integration test in CompletionProviderTests to verify that typing System.Sp triggers completion suggestions for Span and ReadOnlySpan, and that they are not marked obsolete.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 30, 2025

❗ Release notes required


✅ Found changes and release notes in following paths:

Warning

No PR link found in some release notes, please consider adding it.

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/10.0.100.md No current pull request URL (#18784) found, please consider adding it

Copilot AI changed the title [WIP] Fix completion for Span/ReadOnlySpan: exclude IsByRefLike from obsolete filtering and add test Fix IsByRefLikeAttribute types being incorrectly suppressed in completion lists Jul 30, 2025
Copilot AI requested a review from T-Gro July 30, 2025 11:15
Copilot finished work on behalf of T-Gro July 30, 2025 11:15
T-Gro and others added 2 commits July 30, 2025 14:55
- Add attrib_IsByRefLikeAttribute_opt to TcGlobals as optional attribute
- Fix CheckFSharpAttributesForObsolete to use HasFSharpAttributeOpt pattern
- Add proper signature in TcGlobals.fsi
- Update test to expect only 'Span' for 'System.Sp' completion

Co-authored-by: T-Gro <[email protected]>
Copilot AI requested a review from T-Gro July 30, 2025 13:37
Copilot finished work on behalf of T-Gro July 30, 2025 13:37
@T-Gro T-Gro marked this pull request as ready for review July 30, 2025 13:41
@T-Gro T-Gro requested a review from a team as a code owner July 30, 2025 13:41
@T-Gro
Copy link
Member

T-Gro commented Jul 30, 2025

Also add release notes to docs/release-notes/.FSharp.Compiler.Service/10.0.100.md

Copy link
Contributor Author

Copilot AI commented Jul 30, 2025

Also add release notes to docs/release-notes/.FSharp.Compiler.Service/10.0.100.md

Added release notes documenting the IsByRefLikeAttribute completion fix. The entry explains that types like Span<T> and ReadOnlySpan<T> now appear correctly in IntelliSense completion lists. Commit: 0a90d01

Copilot AI requested a review from T-Gro July 30, 2025 13:55
Copilot finished work on behalf of T-Gro July 30, 2025 13:55
Copilot finished work on behalf of T-Gro July 31, 2025 10:02
Copilot AI requested a review from T-Gro July 31, 2025 10:02
@T-Gro T-Gro enabled auto-merge (squash) July 31, 2025 10:27
@abonie
Copy link
Member

abonie commented Jul 31, 2025

@copilot
A test failed in the pipeline run

[xUnit.net 00:00:16.79]     FSharp.Compiler.Service.Tests.CompletionTests.Span appears in completion and is not marked obsolete [FAIL]
  Failed FSharp.Compiler.Service.Tests.CompletionTests.Span appears in completion and is not marked obsolete [827 ms]
  Error Message:
   Assert.True() Failure
Expected: True
Actual:   False
  Stack Trace:
     at FSharp.Compiler.Service.Tests.CompletionTests.assertItemsWithNames(Boolean contains, IEnumerable`1 names, DeclarationListInfo completionInfo) in /Users/runner/work/1/s/tests/FSharp.Compiler.Service.Tests/CompletionTests.fs:line 10
   at FSharp.Compiler.Service.Tests.CompletionTests.assertHasItemWithNames(IEnumerable`1 names, DeclarationListInfo completionInfo) in /Users/runner/work/1/s/tests/FSharp.Compiler.Service.Tests/CompletionTests.fs:line 13
   at FSharp.Compiler.Service.Tests.CompletionTests.Span appears in completion and is not marked obsolete() in /Users/runner/work/1/s/tests/FSharp.Compiler.Service.Tests/CompletionTests.fs:line 372
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

The original fix only handled F# types through CheckFSharpAttributesForObsolete but types like Span<T> and ReadOnlySpan<T> are .NET types that go through IL attribute checking. Updated CheckILAttributesForUnseen to also exclude types with IsByRefLikeAttribute from being considered obsolete, ensuring they appear in completion lists despite having ObsoleteAttribute.

Co-authored-by: abonie <[email protected]>
auto-merge was automatically disabled July 31, 2025 16:20

Head branch was pushed to by a user without write access

Copy link
Contributor Author

Copilot AI commented Jul 31, 2025

@copilot
A test failed in the pipeline run

...

Fixed the test failure. The issue was that the original fix only handled F# types through CheckFSharpAttributesForObsolete, but types like Span<T> and ReadOnlySpan<T> are .NET types that go through IL attribute checking via CheckILAttributesForUnseen. Updated the IL attribute checking function to also exclude types with IsByRefLikeAttribute from being considered obsolete. The test now passes and all 37 completion tests are passing. Commit: 2f4b4f8

Copilot AI requested a review from abonie July 31, 2025 16:22
Copilot finished work on behalf of abonie July 31, 2025 16:22
@T-Gro T-Gro merged commit e4fb0b8 into main Aug 4, 2025
38 checks passed
@T-Gro T-Gro deleted the copilot/fix-d5bdf7a8-1734-4c7c-bb81-4dc22604beff branch August 4, 2025 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants