Skip to content

Special-case Pattern Binding Decls Created by LLDB #37742

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

Merged
merged 1 commit into from
Jun 4, 2021

Conversation

CodaFi
Copy link
Contributor

@CodaFi CodaFi commented Jun 2, 2021

When LLDB wraps a user-defined expression in the REPL, it takes something like this

<expr>

and turns it into (very very abstractly)

var result
do {
  result = <expr>
}
print(result)

In the process, it creates an implicit pattern binding and an implicit do block. Of these, only the implicit do is considered by ASTScope lookup to be relevant. This presents a problem when <expr> is or contains a closure, as the parameters of that closure are defined within a scope that will never be expanded. Thus,

> [42].map { x in x } // <- cannot find 'x' in scope

This patch provides the Swift half of the fix wherein we privilege pattern bindings created by the debugger and look through them to the underlying user expression when performing ASTScope expansion.

rdar://78256873

When LLDB wraps a user-defined expression in the REPL, it takes something like this
```
<expr>
```

and turns it into (very very abstractly)

```
var result
do {
  result = <expr>
}
print(result)
```

In the process, it creates an implicit pattern binding and an implicit do block. Of these, only the implicit do is considered by ASTScope lookup to be relevant. This presents a problem when <expr> is or contains a closure, as the parameters of that closure are defined within a scope that will never be expanded. Thus,

```
> [42].map { x in x } // <- cannot find 'x' in scope
```

This patch provides the Swift half of the fix wherein we privilege pattern bindings created by the debugger and look through them to the underlying user expression when performing ASTScope expansion.

rdar://78256873
@CodaFi CodaFi requested a review from slavapestov June 2, 2021 05:26
@CodaFi
Copy link
Contributor Author

CodaFi commented Jun 2, 2021

@@ -40,6 +40,13 @@ static SourceLoc getLocAfterExtendedNominal(const ExtensionDecl *);

void ASTScopeImpl::checkSourceRangeBeforeAddingChild(ASTScopeImpl *child,
const ASTContext &ctx) const {
// Ignore debugger bindings - they're a special mix of user code and implicit
// wrapper code that is too difficult to check for consistency.
if (auto d = getDeclIfAny().getPtrOrNull())
Copy link
Contributor

Choose a reason for hiding this comment

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

We actually need source ranges to be in order for lookups to work correctly, though. Can you ignore the lldb-written part of the binding? It shouldn't have source locations anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe that’s what this does. When we expand the scopes for the binding’s initializer we’ll still run these checks.

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