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

Add description of local kw in global scope #42794

Merged
merged 5 commits into from
Jan 14, 2022

Conversation

tomtuamnuq
Copy link
Contributor

Closes #42664
Adds an explanation of local keyword usage in a top-level expression to the Julia Manual.
It contains the description @JeffBezanson gave in #10472 and an example as @bkamins gave in #42664.

@tomtuamnuq
Copy link
Contributor Author

Think it is ready to merge. It just adds an example with explanation regarding the scope to the docs.

@bkamins

@bkamins
Copy link
Member

bkamins commented Jan 11, 2022

@tomtuamnuq - I am not a JuliaLang member. I do not have rights to approve nor merge PRs here. We need to wait for some of the core developers to review this PR.

@fredrikekre fredrikekre added the docs This change adds or pertains to documentation label Jan 11, 2022
@StefanKarpinski
Copy link
Member

I think that @JeffBezanson needs to review this but it looks ok to me.

@@ -99,6 +99,19 @@ julia> module E
ERROR: cannot assign variables in other modules
```

Each top-level expression has an invisible scope block around it. If a top-level expression contains a variable declaration with keyword `local`, then that variable is in the local scope of that expression. An example is to declare `local x` in a `begin` or `if` block at the top-level:
Copy link
Member

Choose a reason for hiding this comment

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

Each top-level expression has an invisible scope block around it.

I think that statement is a bit confusing, since toplevel expressions don't introduce an actual scope block like let or while where new assignments are always scoped by default. We should make clear that this only affects variables with an explicit local declaration.

Copy link
Member

Choose a reason for hiding this comment

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

This is why I don't feel like I can clearly explain what's happening here. Is there a scope or not? If there isn't a scope, then what is the variable local to? Maybe there's a way to explain it, but if so, I don't know how to.

Copy link
Member

Choose a reason for hiding this comment

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

I think I would mostly leave out the word scope here and just say that local inside top-level expressions means that the variable won't be accessible outside that expression.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@simeonschaub Since the usage of scope seems to be confusing here I removed it.

Copy link
Member

@simeonschaub simeonschaub left a comment

Choose a reason for hiding this comment

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

Thanks! One small tweak, otherwise this looks good to me.

@@ -99,6 +99,22 @@ julia> module E
ERROR: cannot assign variables in other modules
```

If a top-level expression contains a variable declaration with keyword `local`,
then that variable is not accessible outside that expression.
The variable inside the expression does not affect variables of the same name.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The variable inside the expression does not affect variables of the same name.
The variable inside the expression does not affect global variables of the same name.

@simeonschaub
Copy link
Member

Ah, and it seems like you need to fix the whitespace.

@simeonschaub
Copy link
Member

Doctest hang is likely unrelated, but let's rerun it just to be safe.

@simeonschaub simeonschaub reopened this Jan 13, 2022
@simeonschaub simeonschaub added the merge me PR is reviewed. Merge when all tests are passing label Jan 13, 2022
@tomtuamnuq
Copy link
Contributor Author

@simeonschaub Thanks for your guidance. tester_linux64 failed again. However, I have not found something about it in open issues.

@simeonschaub simeonschaub merged commit f70a86a into JuliaLang:master Jan 14, 2022
@simeonschaub
Copy link
Member

That one passed before, I believe that hang is a known issue with CI. Definitely nothing that would be caused by this PR.

nickrobinson251 pushed a commit to nickrobinson251/julia that referenced this pull request Jan 14, 2022
nickrobinson251 pushed a commit to nickrobinson251/julia that referenced this pull request Jan 14, 2022
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Jan 14, 2022
N5N3 pushed a commit to N5N3/julia that referenced this pull request Jan 24, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make documentation clear about handling of local in global scope
7 participants