Skip to content

Conversation

@forki
Copy link
Contributor

@forki forki commented Jan 19, 2020

No description provided.

@smoothdeveloper
Copy link
Contributor

smoothdeveloper commented Jan 19, 2020

Do we know stripTyEqns can turn some TType which is a TType_app into another one?

If this is known to be false, isAppTy could be simplified to skip call to stripTyEqns altogether, which should be much faster.

The opposite (a not TType_app turned into one through stripTyEqns) is know to be false already.

@smoothdeveloper
Copy link
Contributor

TType_app can turn into something else in stripTyEqns so can't skip that call.

Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

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

Minor nit but I like the change

// Check the (possibly inferred) structural dependencies
(tinst, tcref.TyparsNoRange)
||> List.lengthsEqAndForall2 (fun ty tp -> not tp.EqualityConditionalOn || TypeDefinitelyHasEquality g ty)
| _ -> false
Copy link
Contributor

@cartermp cartermp Jan 20, 2020

Choose a reason for hiding this comment

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

Nit: the _ case (as mentioned in another PR) feels less appropriate since ValueNone is the only other case here. The above usage is fine though since the ValueSome case prior also has a when clause.

@KevinRansom KevinRansom merged commit bb46e37 into dotnet:master Jan 21, 2020
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants