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-1009 - optionally declare mutually referential types and modules over larger scopes within files #1132

Merged
merged 59 commits into from Jun 15, 2016

Conversation

Projects
None yet
4 participants
@dsyme
Contributor

dsyme commented Apr 28, 2016

This is the PR for RFC FS-1009 - Allow mutually referential types and modules over larger scopes within files, being pushed partly for automated testing.

Please only discuss the implementation in this PR. Discuss the RFC on the RFC discussion thread.

@dsyme dsyme changed the title from [WIP] Implement FS-1009 - optionally declare mutually referential types and modules over larger scopes within files to [WIP] FS-1009 - optionally declare mutually referential types and modules over larger scopes within files Apr 28, 2016

@dsyme dsyme changed the title from [WIP] FS-1009 - optionally declare mutually referential types and modules over larger scopes within files to [WIP] RFC FS-1009 - optionally declare mutually referential types and modules over larger scopes within files Apr 28, 2016

@@ -2,7 +2,7 @@
#nowarn "40"
namespace Microsoft.VisualStudio.FSharp.ProjectSystem
namespace rec Microsoft.VisualStudio.FSharp.ProjectSystem

This comment has been minimized.

@smoothdeveloper

smoothdeveloper Apr 28, 2016

Contributor

I like how this is being dogfooded right in the same checkin, without the need to even have last known good carrying the feature 👍

@smoothdeveloper

smoothdeveloper Apr 28, 2016

Contributor

I like how this is being dogfooded right in the same checkin, without the need to even have last known good carrying the feature 👍

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Apr 29, 2016

Contributor

There's one failing test at the moment

tests\fsharpqa\Source\Conformance\ObjectOrientedTypeDefinitions\TypeExtensions\basic\E_InvalidForwardRef01.fs
[00:35:30] *** The following necessary lines were never matched:
[00:35:30] *** (16,21-16,22):.+error FS0430:.+The member '( + )' is used in an invalid way. A use of '( + )' has been inferred prior to its definition.+

Contributor

dsyme commented Apr 29, 2016

There's one failing test at the moment

tests\fsharpqa\Source\Conformance\ObjectOrientedTypeDefinitions\TypeExtensions\basic\E_InvalidForwardRef01.fs
[00:35:30] *** The following necessary lines were never matched:
[00:35:30] *** (16,21-16,22):.+error FS0430:.+The member '( + )' is used in an invalid way. A use of '( + )' has been inferred prior to its definition.+

dsyme added some commits Apr 29, 2016

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme May 12, 2016

Contributor

I've updated this further and started to add a lot of testing (by taking existing tests - especially for object-oriented constructs - and adding "rec")

Contributor

dsyme commented May 12, 2016

I've updated this further and started to add a lot of testing (by taking existing tests - especially for object-oriented constructs - and adding "rec")

dsyme added some commits May 26, 2016

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme May 28, 2016

Contributor

There is one remaining error here:

+++ Conformance\ObjectOrientedTypeDefinitions\ClassTypes\AsDeclarations (SanityCheck01.fs) +++
Failed to Find Any Target:  
FAIL

I don't understand the failure - it may be a hesien-error related to running tests in parallel. The SanityCheck01.fs compiles and runs correctly when runn by hand

Contributor

dsyme commented May 28, 2016

There is one remaining error here:

+++ Conformance\ObjectOrientedTypeDefinitions\ClassTypes\AsDeclarations (SanityCheck01.fs) +++
Failed to Find Any Target:  
FAIL

I don't understand the failure - it may be a hesien-error related to running tests in parallel. The SanityCheck01.fs compiles and runs correctly when runn by hand

@dsyme dsyme closed this May 28, 2016

@dsyme dsyme reopened this May 28, 2016

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme May 28, 2016

Contributor

closed/reopened to trigger CI

Contributor

dsyme commented May 28, 2016

closed/reopened to trigger CI

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme May 28, 2016

Contributor

OK, all passing now. I think this RFC/PR is getting quite close to being ready. I'll be doing more testing next week.

Contributor

dsyme commented May 28, 2016

OK, all passing now. I think this RFC/PR is getting quite close to being ready. I'll be doing more testing next week.

@dsyme dsyme changed the title from [WIP] RFC FS-1009 - optionally declare mutually referential types and modules over larger scopes within files to RFC FS-1009 - optionally declare mutually referential types and modules over larger scopes within files Jun 2, 2016

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Jun 2, 2016

Contributor

I've removed the WIP from this as I believe it is ready to go.

If anyone is able to review this change I'd appreciate it. Unfortunately it hits the infuriating github limit for diff size, so you will need to review most of the changes in a local git client.

Contributor

dsyme commented Jun 2, 2016

I've removed the WIP from this as I believe it is ready to go.

If anyone is able to review this change I'd appreciate it. Unfortunately it hits the infuriating github limit for diff size, so you will need to review most of the changes in a local git client.

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Jun 15, 2016

Contributor

There's a timeout in the AppVeyor testing

Contributor

dsyme commented Jun 15, 2016

There's a timeout in the AppVeyor testing

@dsyme dsyme closed this Jun 15, 2016

@dsyme dsyme reopened this Jun 15, 2016

@KevinRansom KevinRansom merged commit 7ff92d4 into Microsoft:master Jun 15, 2016

4 checks passed

Windows_NT Debug Build Build finished.
Details
Windows_NT Release_ci_part1 Build Build finished.
Details
Windows_NT Release_ci_part2 Build Build finished.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@forki forki referenced this pull request Jun 29, 2016

Closed

Try to reproduce #1296 #1297

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment