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

feat: Detect undefined symbols #50

Merged
merged 1 commit into from
Feb 14, 2025
Merged

feat: Detect undefined symbols #50

merged 1 commit into from
Feb 14, 2025

Conversation

seanmcl
Copy link
Collaborator

@seanmcl seanmcl commented Feb 13, 2025

Our current Python parser does not notice undefined symbols. While we don't do anything with them now, we want to pass that info back to the caller in the forthcoming linter. In addition, we intended parsing to fail when there are undefined symbols, as indicated in test_basic.py, but it was failing for an unrelated reason before, and broke a few commits ago.

This change just makes a best effort attempt to notice undefined symbols, and crashes accordingly in the klr binary. In the future we will do more with this.

This change fixes the broken CI build.

Our current Python parser does not notice undefined symbols.
While we don't do anything with them now, we want to pass
that info back to the caller in the forthcoming linter.
In addition, we intended parsing to fail when there
are undefined symbols, as indicated in test_basic.py, but
it was failing for an unrelated reason before, and broke
a few commits ago.

This change just makes a best effort attempt to notice
undefined symbols, and crashes accordingly in the klr binary.
In the future we will do more with this.

This change fixes the broken CI build.
@@ -221,14 +225,20 @@ def translate(self, src: str, line: int, f: types.FunctionType, args: ast.argume
# if we find other uses of potentially global names
# and fail to understand them; as long as we find and record
# the "real" uses into the environment for the Lean code.
def lookup(self, s):
return self.f.__globals__.get(s) or self.f.__builtins__.get(s)
def lookup(self, node):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, nice. It was lookup.

@@ -183,12 +183,16 @@ An example of a global is:
else:
...
-/
instance : Repr Lean.Json where
reprPrec jsn _ := jsn.compress
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice.

@@ -250,3 +260,13 @@ def visit_Attribute(self, node):
raise e
except Exception:
return


if __name__ == '__main__':
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was thinking we might do: run_klr here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

And then add a gather subcommand?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, not here. in wherever python -m klr-lang goes to.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ooh good idea. Another PR.

@seanmcl seanmcl merged commit 53a1594 into main Feb 14, 2025
12 checks passed
@seanmcl seanmcl deleted the sm/undefined branch February 14, 2025 01:29
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