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

Support using Slice to enable slicing syntax expr1[expr2..expr3] #17377

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

brianrourkeboll
Copy link
Contributor

@brianrourkeboll brianrourkeboll commented Jul 2, 2024

Description

Checklist

  • Test cases added.
  • Release notes entry updated.

Notes

  • It was rather difficult to follow the existing flow of logic in the indexing/slicing typechecking, and it became even harder as I added the new logic, so I refactored it. I hope it's at least a little bit clearer: the main control flow logic is now centralized in the final pattern match.

    match (decodedIndexArgs, setInfo), exprTy with
    // Look for FSharp.Core array and string indexing/slicing helpers.
    | (_, Array) & (ArrayIndexerOrSlicer (path, meth, args), _)
    | (_, String) & (StringIndexerOrSlicer (path, meth, args), _) -> tcArrayOrStringIndexing (path, meth, args)
    // Look for an indexer property, or else assume `Item`.
    | (Indexing, Getting), Indexable indexer
    | (Indexing, Getting), (Array | Nominal) & PossiblyIndexable indexer ->
    propagateThenTcDelayed tpenv expr exprTy (mkDelayedIndexedGet indexer decodedIndexArgs @ delayed)
    // Look for `GetSlice`.
    | (Slicing, Getting), Nominal & GetSliceable slicer ->
    propagateThenTcDelayed tpenv expr exprTy (mkDelayedGetSlice slicer decodedIndexArgs @ delayed)
    // In the absence of `GetSlice`, look for `Slice`.
    | ((Slicing, Getting), Nominal) & Sliceable (tpenv, expr, exprTy) ->
    propagateThenTcDelayed tpenv expr exprTy delayed
    // In the immediate absence of either, assume `GetSlice`.
    | (Slicing, Getting), PossiblyGetSliceable slicer ->
    propagateThenTcDelayed tpenv expr exprTy (mkDelayedGetSlice slicer decodedIndexArgs @ delayed)
    // Look for an indexer property, or else assume `Item`.
    | (Indexing, Setting (setArg, mOfLeftOfSet)), Indexable indexer
    | (Indexing, Setting (setArg, mOfLeftOfSet)), (Array | Nominal) & PossiblyIndexable indexer ->
    propagateThenTcDelayed tpenv expr exprTy (mkDelayedIndexedSet indexer decodedIndexArgs setArg mOfLeftOfSet @ delayed)
    // Assume `SetSlice`.
    | (Slicing, Setting (setArg, mOfLeftOfSet)), (Array | Nominal) ->
    propagateThenTcDelayed tpenv expr exprTy (mkDelayedSetSlice decodedIndexArgs setArg mOfLeftOfSet @ delayed)
    | _ ->
    // deprecated constrained lookup
    error(Error(FSComp.SR.tcObjectOfIndeterminateTypeUsedRequireTypeConstraint(), mWholeExpr))

Copy link
Contributor

github-actions bot commented Jul 2, 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/8.0.400.md

@dotnet dotnet deleted a comment from github-actions bot Dec 16, 2024
@T-Gro
Copy link
Member

T-Gro commented Dec 16, 2024

Hi @brianrourkeboll , I noticed this PR which was put on hold during the release freeze prior to .NET 9 ;; that is not a concern anymore.

Is it ready for review, WDYT ?

@brianrourkeboll
Copy link
Contributor Author

@T-Gro no, this is not ready. I'll need to get back to working on the RFC and either flesh out an updated design for from-end slicing — and thus also how we should deal with System.Index and System.Range — or else see if I can somehow ignore those considerations for a pared-down version of this feature while keeping our options open for those later.

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.

2 participants