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

Fix 11771 - cross-project references for projects using generative type providers #11791

Merged
merged 7 commits into from
Jul 7, 2021

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented Jul 6, 2021

This fixes regression #11771

  1. The regression was in the addition of these lines: https://github.com/dotnet/fsharp/pull/11485/files#diff-015b2a27e0e013656556b5d7e867b1f66a17e145606d4a9abea5eeadd8b7820cR1581

  2. We had had this bug before, fixed here: Disable in-memory project references for projects using generative type providers #3236.

  3. An explicit test was added for it however it was disabled three years ago or so and never got re-enabled https://github.com/dotnet/fsharp/blame/main/tests/service/MultiProjectAnalysisTests.fs#L905

  • I have tested manually
  • I have re-enabled compiler service testing for projects referencing type providers (pending getting it green)
  • I have enabled the testing for .NET Core too (pending getting it green)

@dsyme dsyme changed the title Fix 11771 - cross-project references for proejcts using generative type providers Fix 11771 - cross-project references for projects using generative type providers Jul 6, 2021
@dsyme
Copy link
Contributor Author

dsyme commented Jul 7, 2021

@TIHan this is ready, pending green

@dsyme
Copy link
Contributor Author

dsyme commented Jul 7, 2021

OK, ready

@@ -2184,7 +2184,7 @@ type FSharpCheckProjectResults
keepAssemblyContents: bool,
diagnostics: FSharpDiagnostic[],
details:(TcGlobals * TcImports * CcuThunk * ModuleOrNamespaceType * Choice<IncrementalBuilder, TcSymbolUses> *
TopAttribs option * IRawFSharpAssemblyData option * ILAssemblyRef *
TopAttribs option * ILAssemblyRef *
Copy link
Contributor

Choose a reason for hiding this comment

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

Yay! A dep removed :)

return None
// Note 'false' - if a PEReference doesn't find an ILModuleReader then we don't
// continue to try to use an on-disk DLL
return ProjectAssemblyDataResult.Unavailable false
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, so this should behave the same as before. - We basically only do useOnDisk for type providers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, though I suspect it's right for this path too - it's hard to imagine a situation where dropping the reference on the floor is better than reusing an on-disk DLL

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.

2 participants