Skip to content

Fix for missing IntelliSense after type constraint error #1568

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.

Sign up for GitHub

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

Jump to bottom
Merged
dsyme merged 4 commits into from
Nov 27, 2016
Merged

Fix for missing IntelliSense after type constraint error #1568

dsyme merged 4 commits into from
Nov 27, 2016

Conversation

@liboz
Copy link
Contributor

@liboz liboz commented Sep 28, 2016

Rebasing @mpetruska's PR #848 to a more recent point. It attempts to fixes #629
intellisense fix

dsyme
dsyme reviewed Sep 28, 2016
View reviewed changes
src/fsharp/TypeChecker.fs Outdated
Copy link
Contributor

@dsyme dsyme Sep 28, 2016

Choose a reason for hiding this comment

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

I think this would be called UnifyTypesAndRecover. Also it feels like you should be calling errorRecovery in the error path rather than ignoring any exception completely.

Copy link
Contributor Author

@liboz liboz Sep 29, 2016

Choose a reason for hiding this comment

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

Done. I hadn't done any cleanup after my excitement in getting something working!

@liboz
Copy link
Contributor Author

liboz commented Sep 29, 2016

Looking at the errors, it appears that the following 3 tests are failing; some advice on how to proceed with regards to them would be appreciated.

Test Name Suite Comment
E_Regression02 FSharpQA\Conformance\TypesAndTypeConstraints\CheckingSyntacticTypes It appears that it only gives the first 2 failures instead of the 64 expected. It also appears to give the additional error that P1/P2/P3/etc are not defined. I believe however that the additional error is also a valid error.
neg1 FSharp\typecheck\sig Give an additional typecheck error of cast being invalid. Might be harmless.
neg20 FSharp\typeProviders\negTests Seems the error still appears, but is not repeated twice. Seems harmless and we can just update the test.

@dsyme
Copy link
Contributor

dsyme commented Sep 30, 2016

@liboz Those errors appear harmless and you can just make test fixes (we'll review the test fixes in any case)

The typecheck\sigs test is actually about 200 tests, and I believe it stops on first failure. So you might have to wade through a whole bunch of updates. If possible try to run these on your local machine. We should really break this out into 200 independently failing tests (if you get a chance to do that... :) )

@liboz liboz force-pushed the working branch 4 times, most recently from c0fb770 to 536ddf6 Compare October 1, 2016 11:46
@liboz
Copy link
Contributor Author

liboz commented Oct 1, 2016

I cleaned up the git history a bit and fixed all the relevant tests.

I messed up my previous chart with regards to which was expected/new. Here's a new one for all the failing tests:

Test Name Suite Comment
E_Regression02 FSharpQA\Conformance\TypesAndTypeConstraints\CheckingSyntacticTypes Each error spot generates an additional error that P1/P2/P3/etc are not defined. The additional error is a valid error. However, the current fsc seems to only support 100 errors before it exits. Because of this I had to remove a number of the error conditions and I'm not sure if this still does the check necessary for the regression
neg1 FSharp\typeProviders\negTests Give an additional typecheck error of cast being invalid.
neg4 FSharp\typecheck\sig Give an additional typecheck error for the numeric literal and repeats the original error.
neg20 FSharp\typecheck\sig Generate an additional repeat of the original error
neg22 FSharp\typecheck\sig Generates an additional error in the same line and repeats the original error.

It seems the additional error might be because of the extra errorRecovery. So maybe the error path should just be ().

@liboz liboz changed the title [WIP] Fix for missing IntelliSense after type constraint error Fix for missing IntelliSense after type constraint error Oct 1, 2016
@liboz
Copy link
Contributor Author

liboz commented Oct 1, 2016

When running the tests without the errorRecovery, I found that although the previous FSharp test suites don't need additional modification, the following code let simpleLibraryCall10 inps = Seq.map id inps apparently becomes a potential critical tailcall, which seems quite baffling.

@liboz
Copy link
Contributor Author

liboz commented Nov 10, 2016 •
edited
Loading

@dsyme Is there any chance of this making F# 4.1? And/or getting into FCS so that Ionide can benefit?

@dsyme
Copy link
Contributor

dsyme commented Nov 11, 2016

I'll close/reopen to trigger CI

@dsyme dsyme closed this Nov 11, 2016
@dsyme dsyme reopened this Nov 11, 2016
@liboz
Copy link
Contributor Author

liboz commented Nov 12, 2016

@dsyme The first time CI ran, there were unexpected errors such as on E_NullableOperators01, but I was unable to replicate on my computer so I rebased and this time the CI passed.

@liboz
Copy link
Contributor Author

liboz commented Nov 25, 2016

@dsyme, @KevinRansom Any chance this can be reviewed?

@dsyme
Copy link
Contributor

dsyme commented Nov 26, 2016

This is a good change and will make a significant difference in intellisense accuracy for erroneous code.

@dsyme dsyme merged commit 66a5f53 into dotnet:master Nov 27, 2016
@TIHan TIHan mentioned this pull request Aug 15, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dsyme dsyme dsyme approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Intellisense does not appear if there is an error inside lambda

4 participants

@liboz @dsyme @msftclas @mpetruska