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

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

Merged
merged 34 commits into from
Aug 23, 2021

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented Jul 27, 2021

@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

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

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

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
Copy link
Member

@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

@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 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
…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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Theme-Simple-F# A cross-community initiative called "Simple F#", keeping people in the sweet spot of the language.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants