Skip to content

[RFC FS-1110, 1111] index syntax and ref cell op deprecation#11900

Merged
dsyme merged 34 commits intomainfrom
feature/index-syntax
Aug 23, 2021
Merged

[RFC FS-1110, 1111] index syntax and ref cell op deprecation#11900
dsyme merged 34 commits intomainfrom
feature/index-syntax

Conversation

@dsyme
Copy link
Contributor

@dsyme dsyme commented Jul 27, 2021 •
edited
Loading

@dsyme dsyme mentioned this pull request Jul 27, 2021
Closed
@dsyme
Copy link
Contributor Author

dsyme commented Jul 27, 2021

This is ready for an initial review, also design review on the RFC

@KevinRansom let's find some time to talk this through

@dsyme
Copy link
Contributor Author

dsyme commented Jul 28, 2021 •
edited
Loading

I've updated this PR with information messages suggested here: https://github.com/fsharp/fslang-design/blob/main/RFCs/FS-1111-refcell-op-deprecation.md

@dsyme dsyme changed the title [WIP RFC FS-1110] index syntax arr[idx] [WIP RFC FS-1110, 1111] index syntax and ref cell op deprecation Jul 28, 2021
@dsyme
Copy link
Contributor Author

dsyme commented Jul 28, 2021 •
edited
Loading

Failure:

2021-07-28T19:14:43.4679149Z   Failed Array.AfterOperator...Bug65732_B [80 ms]
2021-07-28T19:14:43.4679710Z   Error Message:
2021-07-28T19:14:43.4680301Z    Couldn't find 'abs' in completion list: [||]

Also

2021-07-28T19:27:28.2525810Z   Failed OverloadLib accepts overloaded internal and external extensions [945 ms]
2021-07-28T19:27:28.2539707Z   Failed OverloadLib accepts overloaded internal extension methods [932 ms]
2021-07-28T19:27:28.2559158Z   Failed OverloadLib accepts overloaded methods [701 ms]
2021-07-28T19:27:28.2569274Z   Failed Custom collection with GetSlice and GetReverseIndex should support reverse index set slicing [666 ms]
2021-07-28T19:27:28.2579423Z   Failed Custom collection with Item and GetReverseIndex should support n-rank reverse index indexing [705 ms]
2021-07-28T19:27:28.2589197Z   Failed Custom collection with GetSlice and GetReverseIndex should support reverse index slicing [841 ms]
2021-07-28T19:27:28.7237687Z   Failed Custom collection with Item and GetReverseIndex should support n-rank reverse index mutation [988 ms]
2021-07-28T19:27:28.7254138Z   Failed Custom collection with Item and GetReverseIndex should support reverse index mutation [556 ms]
2021-07-28T19:27:30.2540712Z   Failed Custom collection with Item and GetReverseIndex should support reverse index indexing [693 ms]
2021-07-28T19:27:42.8973776Z   Failed At operator should not be usable in prefix context [6 ms]
2021-07-28T19:27:42.8980310Z   Failed Hat operator should not be overloadable in prefix context [18 ms]
2021-07-28T19:27:44.0702898Z   Failed Reverse slicing should work with overloaded infix hat [838 ms]
2021-07-28T19:41:42.1453648Z   Failed ref-ops-deprecation-langversion-preview [2 s]

@dsyme
Copy link
Contributor Author

dsyme commented Aug 13, 2021 •
edited
Loading

Errors

2021-08-13T13:44:57.2190242Z   Failed PopupsVersusCtrlSpaceOnDotDot.SecondDot.CtrlSpace [122 ms]
2021-08-13T14:13:54.6542342Z   Failed type check neg24 version 4.7 [942 ms]
2021-08-13T14:13:54.6633745Z   Failed type check neg24 version preview [928 ms]
2021-08-13T14:14:38.3733530Z   Failed type check neg78 [658 ms]

@dsyme
Copy link
Contributor Author

dsyme commented Aug 14, 2021

Unrelated test failure on Linux related to tasks, strange this is the first time I've seen this, it' may be a timing error in the test though

2021-08-14T01:02:05.4720896Z running testNoStackOverflowWithYieldResult
2021-08-14T01:02:05.4782800Z   Failed FSharp.Core.UnitTests.Control.Tasks.Basics.testNoDelay [22 ms]
2021-08-14T01:02:05.4783589Z   Error Message:
2021-08-14T01:02:05.4784713Z    System.Exception : first part didn't run yet
2021-08-14T01:02:05.4785236Z   Stack Trace:
2021-08-14T01:02:05.4785921Z      at FSharp.Core.UnitTests.Control.Tasks.Basics.testNoDelay() in /home/vsts/work/1/s/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Control/Tasks.fs:line 239

@dsyme dsyme changed the title [WIP RFC FS-1110, 1111] index syntax and ref cell op deprecation [RFC FS-1110, 1111] index syntax and ref cell op deprecation Aug 20, 2021
KevinRansom
KevinRansom previously requested changes Aug 20, 2021
View reviewed changes
Copy link
Contributor

@KevinRansom KevinRansom left a comment

Choose a reason for hiding this comment

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

I can't wait for this to be in. Some minor questions and nits.

Also I would like to know about let x = (*)

Thanks for this

@dsyme
Copy link
Contributor Author

dsyme commented Aug 21, 2021

@KevinRansom Thanks code review issues resolved

@dsyme
Copy link
Contributor Author

dsyme commented Aug 21, 2021

Also I would like to know about let x = (*)

For this specific case above, the code let x = (*) continues to tokenize, parse and check the same way, as does variations with spaces e.g. let x = ( * ) or let x = ( *)

The changes in the code around this were needed because ( * ) and ( *) are now be parsed as parens-of-IndexRange. We disambiguate this in CheckExpressions.

Please list out any cases around * you can think of and I'll make sure we have tests for those, or add them separately

vzarytovskii
vzarytovskii reviewed Aug 21, 2021
View reviewed changes
@dsyme
Copy link
Contributor Author

dsyme commented Aug 23, 2021

@KevinRansom I cleared your review since all the things have been resolved, and will merge. Let me know if you think of anything more for follow up testing.

@dsyme dsyme merged commit 699291a into main Aug 23, 2021
@dsyme dsyme added the Theme-Simple-F# A cross-community initiative called "Simple F#", keeping people in the sweet spot of the language. label Aug 31, 2021
@dsyme dsyme mentioned this pull request Sep 6, 2021
Closed
@dsyme dsyme deleted the feature/index-syntax branch October 8, 2021 14:14
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Jan 26, 2022
@dsyme
[RFC FS-1110, 1111] index syntax and ref cell op deprecation (dotnet#…
df81d71 
…11900)

* indexer notation expr[expr]

* add tests

* back compat

* update tests

* fix build

* fix build

* fix additional case of syntactic sugar triggering warning

* fix warning#

* fix warnings

* fix warning#

* improve messages

* remvoe test case no longer of interest

* emit informationals

* update baselines

* don't produce informationals for expr.[idx] as yet

* tweak error messages

* remove dead code

* merge main

* fix tests

* fix tests

* fix tests

* fix tests

* improve diagnostics

* add aka.ms links

* update baselines

* fix error messages

* change warning to informational in preview

* fix test

* code review feedback

* code review feedback

* Update salsa.fs

Co-authored-by: Don Syme <donsyme@fastmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vzarytovskii vzarytovskii vzarytovskii approved these changes

@KevinRansom KevinRansom Awaiting requested review from KevinRansom

Assignees

No one assigned

Labels

Theme-Simple-F# A cross-community initiative called "Simple F#", keeping people in the sweet spot of the language.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dsyme @vzarytovskii @KevinRansom