Skip to content

[swift/release/5.9] [clang][AST] Temporarily disable assert in getInjectedClassNameType #7007

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

Conversation

Michael137
Copy link

When LLDB ASTImports a class declaration it will create a new default-constructed RecordDecl. Such a default-constructerd decl doesn't have its TypeForDecl set correctly yet. To determine the TypeForDecl, ASTImporter calls DeclContext::lookup() on the new decl. If we have an external source like the ASTReader installed on the ASTContext, we can end up calling into getInjectedClassNameType. If the external source created a redeclaration chain between this new ASTImporter decl and the external one, we can end up temporarily setting the TypeForDecl to one that is not an InjectedClassNameType. This invariant is only briefly violated and later corrected by the ASTImporter. But at that point we already tripped this assert during
DeclContext::lookup().

Because this potentially requires significant/risky changes to LLDB, disable this assert for now in order to unblock swift compiler devs working with nightly (i.e., assert-enabled) builds of LLDB.

rdar://109876539

When LLDB ASTImports a class declaration it will create a new
default-constructed RecordDecl. Such a default-constructerd decl
doesn't have its `TypeForDecl` set correctly yet. To determine the
`TypeForDecl`, ASTImporter calls DeclContext::lookup()
on the new decl. If we have an external source like the ASTReader
installed on the ASTContext, we can end up calling into
getInjectedClassNameType. If the external source created a redeclaration
chain between this new ASTImporter decl and the external one,
we can end up temporarily setting the `TypeForDecl` to one
that is not an `InjectedClassNameType`. This invariant is only
briefly violated and later corrected by the ASTImporter. But at
that point we already tripped this assert during
`DeclContext::lookup()`.

Because this potentially requires significant/risky changes to LLDB,
disable this assert for now in order to unblock swift compiler devs
working with nightly (i.e., assert-enabled) builds of LLDB.

rdar://109876539
@Michael137 Michael137 force-pushed the bugfix/lldb-injected-class-name-assert-5.9 branch from b17954e to aefb176 Compare June 16, 2023 13:52
@Michael137
Copy link
Author

@swift-ci test

Copy link

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

thanks!

@adrian-prantl adrian-prantl merged commit e2b11c2 into swiftlang:swift/release/5.9 Jun 16, 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