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

feat: Operator overload from Function #408

Conversation

nikolaymatrosov
Copy link
Contributor

Closes #362

expr.go Outdated
@@ -127,6 +127,10 @@ func Function(name string, fn func(params ...interface{}) (interface{}, error),
Func: fn,
Types: ts,
}
c.Types[name] = conf.Tag{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@antonmedv You said that functions can come from two places: TypesTable and Function.
So here I made the wrong decision, putting delegate from Function to the TypesTable? Instead, I should use config.Functions map in the config.Check function, and where we are searching for suitable operator overload, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds right.

@nikolaymatrosov nikolaymatrosov force-pushed the feature/operator-overload-from-function branch from 71e88c2 to 7094458 Compare August 23, 2023 14:51
@nikolaymatrosov
Copy link
Contributor Author

I have entirely rewritten the PR. I do not no more mix Functions and Types. Instead, when the operator looks for a suitable overload, it first checks the Functions and then Types. So now it is possible to override the method defined in the Env with the function with the same signature. There is a test for this behavior.

Unfortunately, I can not figure out how to make work Function with the same name as Type but a different signature. I still don't know if it is a valid scenario, so I pushed my solution, missing this piece.


}

func TestOperator_FunctionOverTypesPrecedence(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you suggested I also added test on precedence order.

@antonmedv
Copy link
Member

Ok, looks good. Please rebase.

@Q16G
Copy link

Q16G commented Feb 16, 2024

In recent versions, this issue does not seem to have been solved, and it is very important for rewriting operations with functions

@antonmedv
Copy link
Member

Will take a look.

@nikolaymatrosov nikolaymatrosov force-pushed the feature/operator-overload-from-function branch from 4b677b2 to d9b833c Compare February 17, 2024 12:48
@nikolaymatrosov
Copy link
Contributor Author

I have rebased on the latest master.

@antonmedv antonmedv merged commit 4cac5f6 into expr-lang:master Feb 17, 2024
9 checks passed
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.

Operating overloading does not work with functions defined using expr.Function
3 participants