-
Notifications
You must be signed in to change notification settings - Fork 5
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
RFC: 1026 function overloading #86
Open
aljazerzen
wants to merge
4
commits into
master
Choose a base branch
from
function-overloading
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,177 @@ | ||
.. code:: | ||
|
||
Status: Draft | ||
Type: Feature | ||
Created: 2023-01-25 | ||
Authors: Aljaž Mur Eržen <[email protected]> | ||
|
||
####################################### | ||
RFC 1026: Remove function overloading | ||
####################################### | ||
|
||
********** | ||
Abstract | ||
********** | ||
|
||
Current behavior of function and operator overloading requires a | ||
complicated call resolution algorithm. This RFC proposes a new syntax | ||
for defining multiple function and operator implementations and removing | ||
implicit function overloading. | ||
|
||
************ | ||
Motivation | ||
************ | ||
|
||
Currently, EdgeQL allows overloading function definitions. For example, | ||
there are 11 functions defined under the name ``std::contains``: | ||
|
||
.. code:: | ||
|
||
CREATE FUNCTION std::contains( | ||
haystack: std::str, needle: std::str | ||
) -> std::bool using (...); | ||
CREATE FUNCTION std::contains( | ||
haystack: std::bytes, needle: std::bytes | ||
) -> std::bool using (...); | ||
CREATE FUNCTION std::contains( | ||
haystack: array<anytype>, needle: anytype | ||
) -> std::bool using (...); | ||
CREATE FUNCTION std::contains( | ||
haystack: json, needle: json | ||
) -> std::bool using (...); | ||
CREATE FUNCTION std::contains( | ||
haystack: range<anypoint>, needle: range<anypoint> | ||
) -> std::bool using (...); | ||
CREATE FUNCTION std::contains( | ||
haystack: range<anypoint>, needle: anypoint | ||
) -> std::bool using (...); | ||
CREATE FUNCTION std::contains( | ||
haystack: multirange<anypoint>, needle: multirange<anypoint> | ||
) -> std::bool using (...); | ||
CREATE FUNCTION std::contains( | ||
haystack: multirange<anypoint>, needle: range<anypoint> | ||
) -> std::bool using (...); | ||
CREATE FUNCTION std::contains( | ||
haystack: multirange<anypoint>, needle: anypoint | ||
) -> std::bool using (...); | ||
CREATE FUNCTION std::contains( | ||
haystack: range<cal::local_date>, needle: cal::local_date | ||
) -> std::bool using (...); | ||
CREATE FUNCTION std::contains( | ||
haystack: multirange<cal::local_date>, needle: cal::local_date | ||
) -> std::bool using (...); | ||
|
||
This behavior is desired, as it is useful to implement a function for | ||
multiple types with slight variations. However, it complicates resolving | ||
which definition should be used for a given call. | ||
|
||
As functions can be defined at any location, these definitions do not | ||
have any order. When a function is called, all definitions that match in | ||
name are compared with the function call signature. First, all | ||
definitions whose parameters are not super types of call's arguments are | ||
discarded. Then, we compute a "call distance" between types of arguments | ||
and parameters and pick the function definition with the lowest | ||
distance. | ||
|
||
It might happen that multiple definitions have an equal call distance, | ||
in which case an error saying "please disambiguate" is raised. | ||
|
||
Similar behavior is used for operators. | ||
|
||
This behavior is mostly used in combination with pseudo-types: | ||
|
||
create function foo(a: anyscalar) -> str using ("matched anyscalar"); | ||
create function foo(a: str) -> str using ("matched str"); | ||
|
||
select foo("hello"); # {"matched str"} select foo(12); # {"matched | ||
anyscalar"} | ||
|
||
Note that in user-defined functions, it is not allowed to use | ||
pseudo-types or overload functions that take object types. Overloading | ||
of functions that take scalar types does not work correctly. This means | ||
that this overloading behavior cannot used outside of EdgeDB-defined | ||
functions. | ||
|
||
********** | ||
Proposal | ||
********** | ||
|
||
#. Add the following syntax: | ||
|
||
.. code:: | ||
|
||
CREATE FUNCTION std::contains | ||
(haystack: std::str, needle: std::str) -> std::bool using (...) OR | ||
(haystack: std::bytes, needle: std::bytes) -> std::bool using (...) OR | ||
(haystack: array<anytype>, needle: anytype) -> std::bool using (...); | ||
|
||
Here, all implementations must be defined in a single declaration. | ||
Since they have an inherent order, this order can be used during call | ||
resolution. To resolve a call, we first find the single definition | ||
matching in name. Then we traverse the list of implementations from | ||
first to last and compare their signature to the call. First | ||
signature that matches is resolved to, regardless of potential match | ||
in the following implementations. | ||
|
||
Individual implementations in the list would be allowed to define any | ||
number of parameters with arbitrary types. | ||
|
||
#. Add similar syntax for operators. | ||
#. Forbid implicit overloading of functions and operators. | ||
|
||
*************** | ||
Justification | ||
*************** | ||
|
||
This change would vastly simplify call resolution code. | ||
|
||
It would also make it much more predictable. This is currently | ||
problematic, as there is 36 implementations of ``std::`=```, including | ||
implementations for ``anytype``, ``anyscalar``, ``anynumber``, each of | ||
the numeric types and their combinations. | ||
|
||
The simplified behavior would be easier to implement dynamically (i.e. | ||
for dynamic dispatch), as it has to match a linear list, as opposed to | ||
an inheritance tree. Currently, dynamic dispatch is supported only for | ||
pseudo-types, using appropriate PostgreSQL pseudo-types. | ||
|
||
****************** | ||
Future expansion | ||
****************** | ||
|
||
Function overloading is needed when an extension defines or extends a | ||
type. Specifically, there is a need to implement ``std::`=```. | ||
|
||
As extensions would not be able to alter the original function | ||
definition, they do require overloading. That could be accomplished via | ||
explicit an OVERLOADED keyword. Implementations created this way would | ||
be inject in front of the list, before all other implementations. To | ||
disambiguate between two OVERLOADED implementations, explicit references | ||
between the implementations could be used. | ||
|
||
************************ | ||
Backward compatibility | ||
************************ | ||
|
||
Because function overloading is currently useless in user-code, we can | ||
consider it not part of current EdgeQL spec. This means that this change | ||
is fully backwards compatible, if the proposed change can resolve all | ||
calls to EdgeDB-defined functions and operators as they are resolved | ||
currently. | ||
|
||
That *should* be possible, although I am not certain. | ||
|
||
************************* | ||
Backwards compatibility | ||
************************* | ||
|
||
The proposal is fully backwards compatible. | ||
|
||
********************* | ||
Implementation plan | ||
********************* | ||
|
||
The proposal can be implemented in stages. E.g. EdgeDB version 3.0 will | ||
have the basic ``*`` and ``**`` operators supported in shapes, while | ||
EdgeDB 4.0 or later can have the proposed type language extensions | ||
implemented. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what happens with scalars currently: