Skip to content

Conversation

@DragonSA
Copy link
Contributor

Description

Attributes attached to the return value of a class method (e.g. [<return: NotNull>]) were instead being attached to the method itself. This did not happen for functions (let bind). I refactored the logic out of TcNormalizedBinding (used by TcLetBinding and used that in AnalyzeAndMakeAndPublishRecursiveValue.

Fixes #19020

Checklist

  • Test cases added
  • Performance benchmarks added in case of performance changes
  • Release notes entry updated:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 22, 2025

❗ Release notes required


✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/11.0.0.md

@DragonSA DragonSA force-pushed the fix-returnattr branch 2 times, most recently from 30f5af1 to 41a384b Compare October 22, 2025 21:02
Copy link
Contributor

Choose a reason for hiding this comment

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

Please minimise the diff. So it is easier to review.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the white-space churn and also fixed a bug this introduced that caused recursive active pattern matches that return struct to fail (see test Rec struct active pattern is possible).

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. To fix the formatting we need to restore the dotnet tools and run dotnet fantomas .

let attrs =
attrs
|> List.filter (function Attrib(targetsOpt = Some flags) -> not (flags.HasFlag(AttributeTargets.ReturnValue)) | _ -> true)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of filtering, one could partition and move the return attributes into ilReturn below. Then, all the changes to CheckExpressions.fs would not be required.

This would be the smaller change, but I feel it would continue the mis-propagation of attributes...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

[<return: X>] not respected on class members

2 participants