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

Python: Basic implementation of variable capture #14944

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

yoff
Copy link
Contributor

@yoff yoff commented Nov 28, 2023

Instantiates the shared library to handle variable capture in basic cases.

// Note: Learn from JS, https://github.com/github/codeql/pull/14412
// - JS: Capture flow
// - JS: Disallow consecutive captured contents
private module Debug {

Check warning

Code scanning / CodeQL

Dead code Warning

This code is never used, and it's not publicly exported.
@yoff yoff force-pushed the python/captured-variables-basic branch from 5cf6ec2 to c043f0c Compare December 4, 2023 16:44
This provides variable capture in standard situations:
- nested functions
- lambdas
There are some deficiencies:
- we do not yet handle objects capturing variables.
- we do not handle variables captured via the `nonlocal` keyword.
  This should be solved at the AST level, though, and then it
  should "just work".

There is still inconsistencies the case of where
a `SynthesizedCaptureNode` has a comprehensions
as its enclosing callable. In this case,
`TFunction(cn.getEnclosingCallable())` is not
defined and so getEnclosingCallable does not exist
for the `CaptureNode`.
an extra node is now in play.
@yoff yoff force-pushed the python/captured-variables-basic branch from c043f0c to a6e3746 Compare December 5, 2023 19:03
TODO:
The member predicate `LibraryLambdaMethod::getACall` is
currently too permissive.
Ideally, we would have `libraryCallHasLambdaArg`
as in Ruby. But even a more precise
`libraryCall` predicate might be fine.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant