Skip to content

Derive the SILDebugScope for a variable declaration from its owning ASTScope. #65783

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
May 11, 2023

Conversation

adrian-prantl
Copy link
Contributor

rdar://108940570

@adrian-prantl adrian-prantl requested a review from a team as a code owner May 9, 2023 01:21
@adrian-prantl
Copy link
Contributor Author

@swift-ci test

@adrian-prantl
Copy link
Contributor Author

@swift-ci test

@adrian-prantl
Copy link
Contributor Author

@swift-ci test

1 similar comment
@adrian-prantl
Copy link
Contributor Author

@swift-ci test

@adrian-prantl adrian-prantl requested a review from slavapestov May 9, 2023 20:58
@adrian-prantl adrian-prantl changed the title Don't ignore ConditionalClauseInitializerScopes in the debug info. Derive the SILDebugScope for a variable declaration from its owning ASTScope. May 9, 2023
@adrian-prantl
Copy link
Contributor Author

@swift-ci test

@adrian-prantl
Copy link
Contributor Author

test with swiftlang/llvm-project#6827
@swift-ci test

@adrian-prantl
Copy link
Contributor Author

test with swiftlang/llvm-project#6827
@swift-ci test

2 similar comments
@adrian-prantl
Copy link
Contributor Author

test with swiftlang/llvm-project#6827
@swift-ci test

@adrian-prantl
Copy link
Contributor Author

test with swiftlang/llvm-project#6827
@swift-ci test

// Collect all variable declarations in this scope.
struct Consumer : public namelookup::AbstractASTScopeDeclConsumer {
const ast_scope::ASTScopeImpl *ASTScope;
VarDeclScopeMapTy& VarDeclScopeMap;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be a multi-map in case the VarDecl appears in more than one scope?

bool consume(ArrayRef<ValueDecl *> values,
NullablePtr<DeclContext> baseDC) override {
for (auto &value : values)
VarDeclScopeMap.insert({value, ASTScope});
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe assert here that we didn't have a duplicate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. (And fixed a few missed caching opportunities along the way).

…STScope.

The previous code made the assumption that the ASTScope for a variable
declaration should be the one of the declaration's source location. That is not
necessarily the case, in some cases it should be an ancestor scope. This patch
introduces a map from ValueDecl -> ASTScope that is derived from querying each
ASTScope for its locals, which matches also what happens in name lookup.  This
patch also fixes the nesting of SILDebugScopes created for guard statement
bodies, which are incorrectly nested in the ASTScope hierarchy.

rdar://108940570
@adrian-prantl
Copy link
Contributor Author

test with swiftlang/llvm-project#6827
@swift-ci test

@adrian-prantl
Copy link
Contributor Author

(pushed to the wrong branch previously)

@adrian-prantl
Copy link
Contributor Author

test with swiftlang/llvm-project#6827
@swift-ci test

@adrian-prantl adrian-prantl merged commit f9701df into swiftlang:release/5.9 May 11, 2023
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