Navigation Menu

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

Erasing of optional arguments in interfaces #5706

Closed
vicuna opened this issue Jul 30, 2012 · 12 comments
Closed

Erasing of optional arguments in interfaces #5706

vicuna opened this issue Jul 30, 2012 · 12 comments

Comments

@vicuna
Copy link

vicuna commented Jul 30, 2012

Original bug ID: 5706
Reporter: @johnwhitington
Assigned to: @alainfrisch
Status: closed (set by @xavierleroy on 2015-12-11T18:07:32Z)
Resolution: won't fix
Priority: normal
Severity: feature
Version: 3.12.1
Category: ~DO NOT USE (was: OCaml general)
Monitored by: @hcarty

Bug description

You'll no doubt be familiar with the use of optional arguments to avoid having to write wrapper functions to initialize accumulators:

let rec rev_map f ?(a = []) = function
| h::t -> rev_map f ~a:(h::a) t
| [] -> a

Is there any deep type system reason why OCaml couldn't allow an optional argument which the user of the API is never intended to use (because it's just an initializer) to be erased, so in the interface, we would write

val rev_map : ('a -> 'b) -> 'a list -> 'b list

instead of...

val rev_map : ('a -> 'b) -> ?a:'b list -> 'a list -> 'b list

This would then form a complete solution to the problem of having to litter the code with boilerplate wrapper functions like

let rev_map = rev_map []

@vicuna
Copy link
Author

vicuna commented Jul 30, 2012

Comment author: @lefessan

Suppose you have a function "rev_map" without optional argument, and its counterpart in the interface. Now, you want to add an optional argument for the accumulator, you modify the implementation, modify some other modules that will make use of the optional argument, and then, spend a few hours trying to understand why it does'nt work as expected... until you suddenly discover that you have forgotten to update the interface, the compiler silently dropping the optional argument while coercing the module types.

More generally, I think that having an interface or not should not change the semantics of the code.

For me, that's a good reason to avoid such a feature, don't you think ?

@vicuna
Copy link
Author

vicuna commented Jul 30, 2012

Comment author: @johnwhitington

In the scenario you suggest, the new optional argument couldn't be seen by the new code in other modules, and upon seeing the type error at compile time, the first thing one would do is look at the interface, surely?

I'm not sure what you mean by having an interface changing the semantics; if it doesn't have an interface it has no meaning in other modules, since it can't be called. Do you mean that you don't like the idea of the type being different for a) usage of the function within the module vs b) usage from outside the module?

@vicuna
Copy link
Author

vicuna commented Jul 30, 2012

Comment author: @lefessan

Yes. It is already possible to have a function that is polymorphic inside its implementation, and monomorphic outside, I am ok with that, but I think it is less acceptable for function arity. In particular, many people use interfaces (.mli files) to document the functions, and so, it is a bit weird to either document a non-existing optional parameter (i.e. documenting the implementation of the function from the interface of the function) or either not document the optional parameter, although it appears in the implementation.

Well, that's just my opinion, but I almost never use labels, and even less optional arguments, so I am not really concerned, except when I have to read someone else's code. Maybe Jacques will have a different opinion.

@vicuna
Copy link
Author

vicuna commented Jul 31, 2012

Comment author: @damiendoligez

As far as I can tell, what you are asking for is pretty hard to implement because the calling interface to the function is not the same in the two cases (with or without the optional argument). So you cannot do without a wrapper function. To implement what you want, we would have to automatically produce that wrapper function, and it would have to be done in the module typechecker. Nothing impossible, but it clearly violates separation of concerns and need some pretty complex (and out of place) code in the compiler.

@vicuna
Copy link
Author

vicuna commented Jul 31, 2012

Comment author: @johnwhitington

I agree that having a different type inside and outside the module is not desirable for the reasons you suggest.

At the cost of introducing syntax (??), we could have an accumulating argument private to the function like this:

let rec rev_map f ??(a = []) = function
| h::t -> rev_map f ~a:(h::a) t
| [] -> a

This function would have type

val rev_map : ('a -> 'b) -> ?a:'b list -> 'a list -> 'b list

just inside the let rec, and type

val rev_map : ('a -> 'b) -> 'a list -> 'b list

everywhere else, inside the module and outside.

Presumably inside and outside the let rec are different typing environments anyway...

@vicuna
Copy link
Author

vicuna commented Jul 31, 2012

Comment author: @hcarty

What would the implications be for the C interface? Which definition would the function have?

@vicuna
Copy link
Author

vicuna commented Aug 1, 2012

Comment author: @alainfrisch

I don't think that using optional arguments with a default value to initialize accumulators should be encouraged. In particular, it introduces useless runtime overhead (each call wraps the accumulator in a Some constructor, and immediately deconstructs it).

Moreover, the burden of writing wrappers to initialize the accumulator explicitly is not so big, and it is somehow compensated by the fact that can avoid the more heavy syntax of labeled arguments within the recursive function itself.

I'm marking this feature request as "won't fix". If any other core developer disagrees, feel free to change this decision!

@vicuna
Copy link
Author

vicuna commented Aug 1, 2012

Comment author: @lefessan

Note that in the particular case of a recursive function with an optional parameter, the compiler could just optimize them by having a non-optional argument for the recursion, and the optional argument only visible from the outside.

Such an optimization would be completely independent of this bug report, which is only about hiding optional arguments in interfaces. I would actually suggest re-opening this bug report and creating another one for such an optimization, then marking both of them as resolved-suspended...

@vicuna
Copy link
Author

vicuna commented Aug 1, 2012

Comment author: @alainfrisch

Suspended means that we would welcome a contributed patch. I'm not sure we really want to add complexity, new syntax, etc to address this feature request, even if a patch is provided, hence the "won't fix".

(Concerning your other suggestion: this would only apply if all self calls within the function provide the argument. This seems to be very specific optimization, and can be quite easily hand-coded by not using an optional parameter at all.)

@vicuna
Copy link
Author

vicuna commented Aug 1, 2012

Comment author: @lefessan

(Concerning your other suggestion: this would only apply if all self calls within the function provide the argument.

No, since all recursive calls are known, it is possible to just provide the default argument when it is missing in such a call.

This seems to be very specific optimization,

I am not a big fan of optional arguments, but maybe it is a very common idiom to have recursive functions with optional arguments when you use them, in which case such an optimization might tend to be very useful, no ?

and can be quite easily hand-coded by not using an optional parameter at all.)

Yes, of course, but except people with the "developer" status on Mantis, nobody else knows if the compiler will optimize it or not, and so, if such an optimization should be done by hand (ad-warning except if they have followed OCamlPro's course on writting efficient OCaml code... ;-) ).

@vicuna
Copy link
Author

vicuna commented Aug 1, 2012

Comment author: @alainfrisch

No, since all recursive calls are known, it is possible to just provide the default argument when it is missing in such a call.

The expression to be evaluated for the default value can be complex and depend on previous arguments. So you'd need to inline this expression (or turn off the optimization in the case of a non-trivial expression)... And what to do if the recursive function is used as a first-class value within its own body? Frankly, this sounds complex and high risk / low rewards in terms of performance.

maybe it is a very common idiom to have recursive functions with optional arguments when you use them

I've rarely seen this idiom. Maybe it's not uncommon, but I don't believe it is so widespread.

except people with the "developer" status on Mantis, nobody else knows if the compiler will optimize it or not

There are quite a lot of advanced users who know about -dlambda, -dcmm, etc, and who are aware that ocamlopt compiles quite directly, without rearranging the code too much.

@vicuna
Copy link
Author

vicuna commented Aug 3, 2012

Comment author: @garrigue

These feature has been suggested for a long time already.
However, I don't think that this is the job of interfaces to do more than instantiating types.
There are some interesting other suggestions, like optimizing recursive functions using optional arguments.
I agree that this would be more efficient for some situations.
However, I'm afraid this is not the behavior one wants.

let rec rev_map f ?(a = []) = function
| (Some h)::t -> rev_map f ~a:(f h::a) t
| None :: t -> rev_map f t
| [] -> a

In this situation I would like the second call to mean: call rev_map
with the same a. However, this is clearly not the semantics of functional
arguments. So this considerably reduces the interest of this pattern.

I agree with Alain that there is nothing we can do about this report.
(I strongly disagree with saying that using labeled arguments is heavier :-)

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

No branches or pull requests

2 participants