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

[WIP] add a new arg to disable implicit FSharp.Core reference #3345

Closed
wants to merge 4 commits into from

Conversation

0x53A
Copy link
Contributor

@0x53A 0x53A commented Jul 17, 2017

ref #3340
ref #3335 (comment)

I found the place where the implicit FSharp.Core reference is added. =)

@matthid
Copy link
Contributor

matthid commented Jul 17, 2017

@KevinRansom Why can we not use --noframework for this? Do you really think this would break anyone? Can you give a real-life example?

I'm asking because afaik the compiler itself is suggesting to use --noframework when you want to use a different FSharp.Core so I guess that's what people have been using the flag for. So it seems really strange if people use --noframework in combination with magic FSharp.Core resolution.

But yes if this is really a breaking change we can't go I'm all in for the --noimplicitfsharpcore it just feels strange that we need so many flags just to make the compiler behave properly :/

@0x53A
Copy link
Contributor Author

0x53A commented Jul 17, 2017

@matthid I would love to repurpose --noframework, but backwards compatibility ...

Also, I do think these two concerns are slightly orthogonal.

FSharp.Core has never been included in the .net framework.

@matthid
Copy link
Contributor

matthid commented Jul 17, 2017

yes they are but usually the caller wants to handle references manually or not.

Handling framework references manually but using magic mode for fsharp.core seems to be a really strange mode.

If backwards compat is too important here we could obsolete noframework (leave its behavior and warn when used) and add '--noimplicitreferences' or something like that just to mention another possible route...

@matthid
Copy link
Contributor

matthid commented Jul 17, 2017

Would be nice to have numbers of how many times the compiler is operating in this state...

@0x53A
Copy link
Contributor Author

0x53A commented Jul 17, 2017

I'm just happy if I get any way to disable that, so I'm not picky. =)

Agree that most of the time you would use the new flag, you would also use noframework.

@dsyme
Copy link
Contributor

dsyme commented Jul 18, 2017

@KevinRansom Why can we not use --noframework for this? Do you really think this would break anyone? Can you give a real-life example?

Personally I'd be ok with this in principle (#3340 (comment)). However I suppose it would break something and a new flag should be added

@KevinRansom
Copy link
Member

@dsyme

--noframework probably should have been the switch to use.

Right now I am reluctant to add any new flags unless the flag turns off all compiler resolution magic. We already have too many switches and the behavior of resolution is often unexpected. I expect there are other resolution scenarios that we may encounter that we could add switches for. However, the tooling is not yet in a state where we can do that.

I am afraid that using the ambient FSharp.Core is a common scenario in the community tools. I know this to be true because we recently had to add the optdata and perfdata files to FSharp.Core.dll.

I also suspect that intellisense would likely be broken on an FSharpProject that is configured with the switch but msbuild can't find FSharp.Core.dll But I can't prove it.

If we insist that this must be fixed in the compiler now, then a new compiler option is the only reasonable targeted fix.

I think the least risky solution is to modify the SDK to give an error if it can't resolve FSharp.Core for the project. It wouldn't then impact any current behavior and there are no third party projects to be broken.

Kevin

@dsyme
Copy link
Contributor

dsyme commented Jul 18, 2017

@KevinRansom yes, makes sense

@KevinRansom
Copy link
Member

KevinRansom commented Oct 30, 2017

0x53A, I don't think we want to do this ... this way right now. So I think we should close it. I have a lot of sympathy with this change but I would prefer we got a proper grip on all of the magic resolution that happens in the FSC/FSI/FCS.

Thanks for proposing it, and working on it

Kevin

@0x53A
Copy link
Contributor Author

0x53A commented Oct 30, 2017

... this way right now

So what is the current plan?

Add a check in the Sdk? Then there would be no way to disable the magic resolution in scenarios that directly call fsc.exe.

In my opinion having 10 switches to disable behavior, while confusing, is better than having no way to control it at all.

Thanks for proposing it, and working on it

The working on it part was limited, it was mostly meant to start a discussion. ;)

Maybe this discussion should be moved to an issue instead? Or is your stance clearly NO? Then no discussion is neccessary ;D

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.

6 participants