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

fix the use of end and begin as part of indexing #20

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

schlichtanders
Copy link

This is part of a little memory-leak bug which was hard to debug.

Because begin and end had been identified as normal variable names, the use of it inside indexing causes new workspaces to be created.

I guess in normal Pluto this is not so decisive, as anyway a new workspace is created every time, but in my fork I am reusing workspaces where this appeared then.

Maybe it also helps normal Pluto users somehow.

@schlichtanders schlichtanders changed the title fix the use of end and begin as part of indexing fix the use of end and begin as part of indexing May 15, 2024
@Pangoraw
Copy link
Member

One can use var"begin" anywhere so ideally this behavior should be enabled only inside indexing.

julia> let
           var"begin" = 2
           x = [1,2,3]
           x[var"begin"]
       end
1

@fonsp
Copy link
Member

fonsp commented May 23, 2024

So let's add a flag is_inside_index to ScopeState?

@fonsp
Copy link
Member

fonsp commented Aug 8, 2024

@schlichtanders Could you make that change? We also need some tests, about begin inside an index, and begin as a variable like the evil example the Paul gave.

@fonsp
Copy link
Member

fonsp commented Oct 4, 2024

We're so close!

@schlichtanders
Copy link
Author

Hi, yes I can make these changes. It is just that I probably will find time around December. As other things are currently quite high in priority.

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.

3 participants