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

Add warning when raise/failwith/failwithf functions are passed extra arguments #46

Closed
KevinRansom opened this issue Jan 18, 2015 · 3 comments

Comments

@KevinRansom
Copy link
Member

opened at CodePlex by: lasandell

I found what I think is a bug in the compiler, as demonstrated in the following F# Interactive session:

let () = raise (Exception("test")) "extra1" "extra2";;

System.Exception: test
at Microsoft.FSharp.Core.Operators.Raise[T](Exception exn)
at Microsoft.FSharp.Core.FSharpFunc2.InvokeFast[V,W](FSharpFunc2 func, T arg1, TResult arg2, V arg3)
at <StartupCode$FSI_0106>.$FSI_0106.main@()
Stopped due to error
As you can see, raise accepts and ignores an arbitrary number of additional arguments after the exception.
This also affects all functions based on raise, such as failwith. The way I initially found this was that I typed failwith when I meant to use failwithf, which caused my formatting arguments to be ignored even though it still compiled.

comments
StronglyTyped wrote Apr 9, 2014 at 8:40 PM [x]
It is not a bug, but the way the unification algorithm works.

failwith;;
val it : (string -> 'a)

value returned by failwith gets unified as a func of 2 args if you use it as

failwith "2121" 1 1;;

but it doesn't get applied.
In OCaml you can observe the same behavior:
% ocaml [0]
OCaml version 4.01.0

failwith "212" 1;;

Warning 20: this argument will not be used by the function.
Exception: Failure "212".

failwith "212" 1 2;;

Warning 20: this argument will not be used by the function.
Warning 20: this argument will not be used by the function.
Exception: Failure "212".

lasandell wrote Apr 9, 2014 at 8:50 PM [x]
Ah, I guess you're right. Seems a bit dangerous if you mistype failwithf as failwith, but what can you do?

lasandell wrote Apr 9, 2014 at 9:28 PM [x]
I guess printing the warning like OCaml does would be a solution.

StronglyTyped wrote Apr 10, 2014 at 9:57 AM [x]
It would be interesting to know an opinion of someone knowledgeable on having such a warning and if it does make sense create a work item for it.

JonHarrop wrote Apr 16, 2014 at 7:56 AM [x]
You could certainly add a warning for this and I think it would be valuable.

dsyme wrote Apr 16, 2014 at 8:34 AM [x]
Yes, it is reasonable to add a warning for this.
One approach would be to add the check in in check.fs (which performs checks in the TAST tree after type checking), looking specifically for raise, failwith and failwithf as "known functions" and checking that these are not applied to additional results. Breakpointing in check.fs would be one way to examine the TAST trees encountered. You may have to add the known functions to the TcGlobals in env.fs, using valRefEq to determine value reference equality.

latkin wrote Jun 5, 2014 at 10:35 AM [x]
Updating title to reflect current sentiment

@dsyme
Copy link
Contributor

dsyme commented Jan 19, 2015

I was looking for a "suggestion" or "better dianostic" GitHub label to apply to this issue but couldn't see the right one to use.

@KevinRansom or @latkin - could you add a new label along these lines to use? I believe you need admin rights to add new labels, and I only have "write" permissions.

Many thanks

@latkin
Copy link
Contributor

latkin commented Jan 19, 2015

Done

@tpetricek
Copy link
Contributor

This would be nice to have :-) fsprojects/FSharp.Formatting#225

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

No branches or pull requests

4 participants