Remove one "impossible" error

[forki]

fixes #5745

[matthid]

But this case is impossible, correct? Shouldn't we report this somehow?

[forki]

"impossible"

[matthid]

What I mean is that item is matched some lines above to be one of the two cases and item is immutable? This case means someone has changed something and missed to do some work. Removing failwith means the mistake will be spotted later (or not at all)

[forki]

actually that is what already happend. the case was added and nobody changed the failwith "impossible"

[matthid]

But line above reads | (Item.UnionCase _ | Item.ExnCase _) as item -> how can this match another case?

[forki]

it does not today. wait until someone adds the active pattern part ;-)

[matthid]

Should we still report that this error message might need to be refined (in case it happens)? Would this be an advantage of failwith "impossible" here?

[forki]

failwith "impossible" is arguably the worst you can do

[matthid]

Yes ideally the code would be written in a way to not compile when written incorrectly, but if that cannot be achieved imho throwing is better than doing/reporting potentially incorrect messages

[cartermp]

Actually I think even having a confusing error message is better than what existed. The current state of the world (without @forki's changes) paints the whole editor red if you use the incorrect code:

These internal errors are critical to root out given this, since it's pretty much undiagnosable where the user error is.

[matthid]

Ok but imho we still should make sure the error message indicates this is not implemented and something which needs to be fixed if it occurs.
Otherwise users get wrong messages on their code which is arguably worse than a compiler crash

[forki]

we don't give a wrong message. We just give a very generic one

[forki]

@KevinRansom is something wrong with master? the error for linux build seems unrelated

[KevinRansom]

@forki not that I know. The error is:

2018-10-09T12:23:11.3515703Z   /home/vsts/work/1/s/src/scripts/scriptlib.fsx(65,25): error FS0041: A unique overload for method 'GetFileName' could not be determined based on type information prior to this program point. A type annotation may be needed. Candidates: Path.GetFileName(path: ReadOnlySpan<char>) : ReadOnlySpan<char>, Path.GetFileName(path: string) : string [/home/vsts/work/1/s/tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj]
2018-10-09T12:23:11.3517355Z   /home/vsts/work/1/s/src/FSharpSource.targets(208,5): error MSB3073: The command "mono /home/vsts/work/1/s/packages/FSharp.Compiler.Tools.4.1.27/tools/fsi.exe --exec "/home/vsts/work/1/s/src/scripts/subst.fsx" --in:"/home/vsts/work/1/s/tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.dll.config" --out:"/home/vsts/work/1/s/tests/FSharp.Core.UnitTests/../../src/../release/net40/bin/FSharp.Core.UnitTests.dll.config" --pattern1:"FSCoreVersion" --replacement1:"4.5.0.0" --pattern2:"" --replacement2:"" " exited with code 1. [/home/vsts/work/1/s/tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj]
2018-10-09T12:23:11.3517711Z 
2

At this point I believe the script is running the built compiler, so I suppose there is some possibility they are related.

[cartermp]

@KevinRansom #5733 also has this error but with no compiler, so something's wrong in the linux build

[KevinRansom]

@cartermp indeed it does.

[forki]

it's noew green after rebase