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

Disable escaping from namespaces for inner functions (#208) #210

Merged
merged 6 commits into from
Oct 23, 2018

Conversation

ChengCat
Copy link
Contributor

@ChengCat ChengCat commented Oct 19, 2018

The namespace escaping behaviour has been removed, and a new machanism for checking invalid variable reference is added.

This is mostly done, and the error reporting could be improved:

  • Error location could be added.
  • I don't understand for now why error-src/anonymous-scopes.dt generates two "can't reference variable in a different function" errors.

I tackle this issue (#208) first instead of #201, because this seems to be easier, and it's more urgent for my use.

It should be invalid to reference a local variable from a different function,
and the check is a work in progress right now.

For the purpose of catching invalid variable references, a set of new 'function
scopes' has been introduced into Context. Every variable will keep track of its
function scope. A variable can be referenced as a value if and only if its
function scope is equal to the current function scope, or its function scope is
the global scope.
* added Context::addVariable, which takes of the function scopes, and routed all
  Namespace::addVariable calls to this function.

* modified Context::getVariable to take care of the function scopes
@ChengCat ChengCat changed the title WIP: disable escaping from namespaces for inner functions (#208) Disable escaping from namespaces for inner functions (#208) Oct 20, 2018
@tomhrr
Copy link
Owner

tomhrr commented Oct 22, 2018

Thanks for this, it looks good. The only change I'd like to make is to drop the ReferenceVariableInDifferentFunction error, because it works differently from the existing 'variable not in scope' errors, and those existing errors will be displayed whenever the new errors are displayed anyway. Does that sound OK?

@ChengCat
Copy link
Contributor Author

Thanks for the feedback. I originally added that error, because my mental model is that the variable is in the scope, but can't be referenced as a value. (Function scope is viewed as a compiler implementation detail.)
I have removed the error, and changed VariableNotInScope's error message to indicate the different possible cause. Does it look OK?

@tomhrr
Copy link
Owner

tomhrr commented Oct 23, 2018

Yep, these changes look good, thanks.

@tomhrr tomhrr merged commit 46dc272 into tomhrr:master Oct 23, 2018
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.

2 participants