Skip to content

Conversation

@QuietMisdreavus
Copy link
Contributor

@QuietMisdreavus QuietMisdreavus commented Jan 14, 2022

Resolves rdar://87601741

In #40810, ModuleDecl::getDisplayDecls started to recurse into modules that were marked as @_exported import for collecting decls. This caused some failures in source compatibility testing, due to recursing into Clang modules during serialization that tripped an assertion. This PR adds a check against these modules before recursing, so that Clang modules that should not be recursed into can be skipped.

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test source compatibility

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

@QuietMisdreavus
Copy link
Contributor Author

After testing locally with one of the original source-compat failures, it seems like this still creates a failure if you try to generate symbol graphs. I'm going to try a different approach and push that up.

@QuietMisdreavus QuietMisdreavus force-pushed the QuietMisdreavus/display-decl-recur branch from f7db5ba to ed0d6d4 Compare January 14, 2022 23:28
@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test source compatibility

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

@QuietMisdreavus QuietMisdreavus changed the title don't recurse into imports except for SymbolGraphGen [AST] check modules before recursing for display decls Jan 15, 2022
@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - ed0d6d4

Copy link
Contributor

@slavapestov slavapestov left a comment

Choose a reason for hiding this comment

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

Can you add a reduced test case for the crash?

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - ed0d6d4

@QuietMisdreavus QuietMisdreavus force-pushed the QuietMisdreavus/display-decl-recur branch from ed0d6d4 to 7338eb0 Compare January 19, 2022 23:08
@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test source compatibility

@QuietMisdreavus
Copy link
Contributor Author

This PR now contains the change from #40810, as it was reverted in #40877.

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.

4 participants