Skip to content

[AST|ASTWalker] Fix assertion hit walking an invalid AST with a protocol decl inside an extension #29338

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

nathawes
Copy link
Contributor

@nathawes nathawes commented Jan 21, 2020

The AST walker calling getGenericParams() on a protocol decl would eventually call computeNominalType. computeNominalType checks that the protocol (as a nominal type decl) appears within a type context, and if so, asks for the SelfNominalTypeDecl of that context. If the context is an extension, that ends up asking for its extended nominal, which is a problem if we're walking a pre-typechecked AST – we hit the assertion "Extension must have already been bound (by bindExtensions)").

Unlike other nominal types, it's not valid Swift for protocols to appear within type contexts, so exclude protocol decls from taking this code path. This results in us providing Type() as the parent type of the produced NominalType, just like we do for protocols inside functions or other invalid contexts.

Resolves rdar://problem/58549036

…col decl inside an extension

The AST walker calling getGenericParams on a protocol decl would eventually
call computeNominalType. computeNominalType checks that the protocol (as a
nomimnal type decl) appears within a type context, and if so, asks for the
SelfNominalTypeDecl of that context. If the context is an extension, that
ends up asking for its extended nominal, which is a problem if we're walking
a pre-typechecked AST – we hit the assertion "Extension must have already been
bound (by bindExtensions)").

Unlike other nominal types, it's not valid Swift for protocols to appear within
type contexts, so exclude protocol decls from taking this code path. This
results in us providing Type() as the parent type of the produced NominalType,
just like we do for protocols inside functions or other invalid contexts.

Resolves <rdar://problem/58549036>
@nathawes
Copy link
Contributor Author

@swift-ci please test

@nathawes nathawes requested a review from rintaro January 21, 2020 23:21
@nathawes nathawes marked this pull request as ready for review January 21, 2020 23:21
Copy link
Member

@rintaro rintaro left a comment

Choose a reason for hiding this comment

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

LGTM!

@nathawes nathawes merged commit 3322f6d into swiftlang:master Jan 21, 2020
@nathawes nathawes deleted the dont-require-extensions-to-be-bound-in-astwalker branch January 21, 2020 23:47
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