Print int[] as int array. #13700
Print int[] as int array. #13700
Conversation
|
@nojaf As we suspected we have a bunch of test failing now . Good first step. let tackle them in the next pair session :) |
nojaf
force-pushed
the
int-array-signature
branch
2 times, most recently
from
e63be54 to
e5b2bd8
Compare
|
We are getting closer :D . We have some baseline tests that I created a PR to you fork to fix them :) Some VS integration test are also failing . Im not sure how to fix them . But we can finish that in our next Pair session :) |
nojaf
force-pushed
the
int-array-signature
branch
4 times, most recently
from
30056b8 to
6e56873
Compare
|
@nojaf I think we need an extra rebasing on to main. to grab the fix for the test that are failing in the CI . Thanks in advance :) |
nojaf
force-pushed
the
int-array-signature
branch
2 times, most recently
from
42d5b17 to
f4f6aee
Compare
nojaf
force-pushed
the
int-array-signature
branch
from
e6d1b3d to
5651cce
Compare
|
@vzarytovskii This is ready for a review :) . |
nojaf
force-pushed
the
int-array-signature
branch
from
5651cce to
266ac9e
Compare
|
@vzarytovskii I'm happy with this. |
|
@vzarytovskii I was wondering if this could make it to the next VS preview version ? . This will also benefit VSCode and Rider :) |
|
|
||
| let (|TTypeMultiDimensionalArrayAsGeneric|_|) (t: TType) = | ||
| let rec (|Impl|_|) t = | ||
| match t with |
There was a problem hiding this comment.
This code is wrong
- once again we're matching directly on TType, we should almost never be doing this
- it looks like it is detecting jagged arrays, which is wrong
There was a problem hiding this comment.
Will have a look next week
| @@ -893,6 +903,10 @@ module PrintTypes = | |||
| // Always prefer 'float' to 'float<1>' | |||
| | TType_app (tc, args, _) when tc.IsMeasureableReprTycon && List.forall (isDimensionless g) args -> | |||
| layoutTypeWithInfoAndPrec denv env prec (reduceTyconRefMeasureableOrProvided g tc args) | |||
|
|
|||
| // Special case for nested array<array<'t>> shape | |||
There was a problem hiding this comment.
This is wrong, and this rule should be removed. Also there are no tests for jagged arrays - they should be added.
There was a problem hiding this comment.
Will update next week. Thanks for the review
Initial attempt to address fsharp/fslang-suggestions#635.
The result of a pair-programming session with @edgarfgp 🎉.