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

document local methods, warn about branches #41013

Merged
merged 5 commits into from
May 9, 2022

Conversation

tpapp
Copy link
Contributor

@tpapp tpapp commented May 30, 2021

Briefly explain local methods and limitations in the manual.

The main motivation is people running into variants of #15602, this should be documented.

@matthias314
Copy link
Contributor

Maybe one could write "leads to undefined behavior" instead of "prevents various compiler optimizations" because such code can actually produce errors. Also, it might be worthwhile to mention that "conditional definitions" include those following a return statement, as in

function c2()
    function f end
    return f
    f() = 0
end

c2()  # UndefVarError: f not defined

@tpapp
Copy link
Contributor Author

tpapp commented May 31, 2021

@matthias314: thanks, I implemented this. I am not totally sure whether to document the ideal we are going for (once #15602 is fixed, ideally all of these things should error), or the status quo (which is indeed undefined behavior or an error message that is hard to interpret).

We can always edit this section in the future though, but it would be important to mention this in the manual in some form.

@vtjnash vtjnash added the domain:docs This change adds or pertains to documentation label May 31, 2021
@tpapp
Copy link
Contributor Author

tpapp commented Jul 5, 2021

Friendly bump, I wonder if someone could review this short docs PR.

g() = 0
end
```
as this leads to undefined behavior.
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I think describing this as generally "undefined behavior "is a bit harsh. It is typically used to mean absolutely anything can happen and your system can be in an arbitrary state. I wonder if this could be softened somewhat.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about "unexpected errors"?

@KristofferC KristofferC added the status:merge me PR is reviewed. Merge when all tests are passing label May 6, 2022
@vtjnash vtjnash merged commit e315231 into JuliaLang:master May 9, 2022
@tpapp tpapp deleted the tp/document-local-methods branch May 9, 2022 15:52
@giordano giordano removed the status:merge me PR is reviewed. Merge when all tests are passing label May 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants