Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sink: report SynPat.ArrayOrList type #18127

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

DedSec256
Copy link
Contributor

Reports the inferred SynPat.ArrayOrList type to the sink. This can be used in features like type hints.

@DedSec256 DedSec256 requested a review from a team as a code owner December 9, 2024 22:20
Copy link
Contributor

github-actions bot commented Dec 9, 2024

❗ 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/9.0.200.md

@T-Gro
Copy link
Member

T-Gro commented Dec 10, 2024

Is this the right place? What is the intended type hint usage please?

It does feel adhoc here because none of the other patterns (records, tupes, ..) is making a sink report, hence my question.

@DedSec256
Copy link
Contributor Author

It seems there is no common place where we can add a sink and enable it for records/tuples/etc., but not include it in unnecessary (at least for now) patterns such as As/Wild/Ands etc.
At the same time, in most cases, the FCS code includes sink calls separately for specific expressions as needed.

The difference between hints for the collection pattern is that for records/tuples/etc. annotations are usually shown for fields or tuple elements individually, while collections don't have elements at all (if this is the case [| |] / [ ]), or do all the elements have the same type, and to avoid duplication, it is easier to annotate the collection itself instead of each element individually

image

@T-Gro
Copy link
Member

T-Gro commented Dec 11, 2024

Thanks Alex, I get it now with your last paragraph.
This makes sense.

@T-Gro T-Gro enabled auto-merge (squash) December 11, 2024 09:27
@T-Gro T-Gro merged commit 6af82c8 into dotnet:main Dec 11, 2024
33 checks passed
psfinaki added a commit that referenced this pull request Dec 12, 2024
* Backport :: Bugfix :: Support `ldelem.u8`, `ldelem.u` opcode aliases (#18081) (#18096)

* Streamlining test deps a bit (#18022)

* Streamlining test deps a bit

* up

* Format ILVerify output a bit (#18120)

* fix for race condition in FileIndex.fileIndexOfFile (#18008)

* fix for race condition in FileIndex.fileIndexOfFile

* fantomas

* fixed ilverify baselines (just a single line number changed)

* add release notes entry

* FileToIndex: Added unlocked read so that lock is entered for new files only

* update ilverify baselines (changed line numbers only)

* Fix ILVerify

---------

Co-authored-by: Petr <[email protected]>
Co-authored-by: Vlad Zarytovskii <[email protected]>

* Update F# build version to 200

* Fix how much is trimmed from an interp string part (#18123)

* Fix how much is trimmed from an interp string part

Only trim last 2 characters if they are "%s" and the '%' is not escaped

* Add release note

---------

Co-authored-by: Adam Boniecki <[email protected]>

* Sink: report SynPat.ArrayOrList type (#18127)

---------

Co-authored-by: Tomas Grosup <[email protected]>
Co-authored-by: Petr <[email protected]>
Co-authored-by: Martin <[email protected]>
Co-authored-by: Vlad Zarytovskii <[email protected]>
Co-authored-by: Adam Boniecki <[email protected]>
Co-authored-by: Adam Boniecki <[email protected]>
Co-authored-by: Alex Berezhnykh <[email protected]>
Co-authored-by: Kevin Ransom (msft) <[email protected]>
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