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

Function redefinition is ignored #16

Closed
oxinabox opened this issue Nov 22, 2019 · 5 comments
Closed

Function redefinition is ignored #16

oxinabox opened this issue Nov 22, 2019 · 5 comments

Comments

@oxinabox
Copy link
Contributor

Fresh build today, using the REPL:

It said it redefined, but it did not

>=> f :: Real -> Real
... f x = 2.0*x

>=> f 2.3
4.6
>=> f :: Real -> Real
... f x = 3.0*x
Variable redefined: f
>=> f 2.3
4.6
@dougalm
Copy link
Collaborator

dougalm commented Nov 22, 2019

It's currently treated as an error to shadow or redefine variables. The message you saw Variable redefined: f was an (unclear) error report, not a notification or warning. c91078f tries to make it clearer that it's considered an error. The message is now Error: variable already defined: f.

Long-term, I'm not sure what sure what shadowing behavior we want. Error? Warning? Allowed? There are good arguments for each. I lean towards keeping the strict no-shadowing behavior but I admit it's quite annoying in the REPL.

@oxinabox
Copy link
Contributor Author

I think that is a good call. At least for now.
Allowing redefintions adds a ton of complexity.
(This led to JuliaLang/julia#265)

@dougalm
Copy link
Collaborator

dougalm commented Nov 22, 2019

Very interesting to know about the Julia experience. That sounds like another reason to keep it strict and simple for now. It's easy to liberalize it later if needed.

@oxinabox
Copy link
Contributor Author

Very interesting to know about the Julia experience. That sounds like another reason to keep it strict and simple for now. It's easy to liberalize it later if needed.

Will you be at NeurIPS?

The short version of the JuliaLang/julia#265 thing is that if you allow functions to be redefined,
and if you have static calls (or inlined IR at some level) to functions compiled into other functions,
then if those functions are redefined you need to update those references.
So of g calls f, and if f is redefined, then g needs to be recompiled to call the new f.
And if h calls g then h needs to be recompiled too.

So you need to track the locations of all static calls of all functions incase they get redefined;
so you can do reconmpilation.
This is the whole backedges thing.

There are maybe other reasons to keep track of backedges.
One also needs it for method deletion; and I think for a few other things.
But I am not sure how many apply in the static world with type-classes.

@dougalm
Copy link
Collaborator

dougalm commented Nov 23, 2019

Thanks for explaining the recompilation issues. There may be similar issues in Dex's notebook runtime, which only re-evaluates top-level declarations which have changed in the source file, and those that depend on them. Ruling out variable shadowing makes the caching logic quite a bit simpler.

Will you be at NeurIPS?

Yes, I'll be there for the workshops. Let's catch up then!

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

2 participants