[AST] scan @_exported import modules of source files for display decls

[QuietMisdreavus]

Resolves rdar://85230067

When collecting decls for SourceFile modules, decls from @_exported import modules (including those added with -import-underlying-module) are not collected. This causes SymbolGraphGen to miss Clang symbols when building in -emit-module mode. This PR adds an override for SourceFile::getDisplayDecls (called indirectly by SymbolGraphGen) that also collects symbols from these exported modules.

[QuietMisdreavus]

@swift-ci Please test

[QuietMisdreavus]

@swift-ci Please build toolchain macOS platform

[QuietMisdreavus]

This PR builds on top of #40696 so that i can use its test.

[QuietMisdreavus]

@swift-ci Please test windows

[belkadan]

:-/ I realize I'm well away from being a compiler contributor by now, and that getDisplayDecls was always intended to be "do the right thing for Xcode", but won't this splat all of the declarations from the ObjC half of a framework into every file of the Swift half of the framework?

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - 7a9c7bd

[QuietMisdreavus]

@belkadan My assumption is that because i'm pulling the imports directly from the file, it will only pull when a file has the @_exported import declaration.

...however, i'll need to double-check about the imports added by -import-underlying-module. 😅 I didn't confirm whether they're added to each file.

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - 7a9c7bd

[slavapestov]

Will this potentially include the same decl more than once if it's imported via two different paths, eg

// C.swift:
public class Class {}

// A.swift:
@_exported import C

// B.swift
@_exported import C

// Foo.swift
import A
import B

// does this visit 'C.Class' twice?

Perhaps a better way is to get the 'flat' list of transitively-imported modules from the ImportCache, and then just get the top-level decls of each one?

[QuietMisdreavus]

It looks like it will, based on how the -import-underlying-module case works (it currently crawls those modules for each file). I'll work on an alternative method to collect these modules ahead of time.

[slavapestov]

Try something like this

auto &importCache = sf.getASTContext().getImportCache();
auto &importSet = importCache.getImportSet(sf);
auto modules = importSet.getTransitiveImports();

[QuietMisdreavus]

Are the transitive imports what we want? I'm concerned about collecting decls that aren't actually part of the file/module, since they wouldn't be @_exported.

[slavapestov]

The transitive imports are the @_exported ones only, I think.

[QuietMisdreavus]

It looks like that includes much more than the @_exported imports; the framework from the test case seems to be including all of Foundation.

[QuietMisdreavus]

I think i have a solution, but i'm not sure where/how to test it. The symbol graph deduplicated these symbols anyway, and i'm not sure where else uses getDisplayDecls() that we could slot a test into.

[slavapestov]

What about something like this in getDisplayDecls():

#ifndef NDEBUG
llvm::DenseSet<Decl *> visited;
for (auto *decl : decls) {
  auto inserted = visited.insert(decl).second;
  assert(inserted);
}
#endif

[QuietMisdreavus]

That seemed to work! I've pushed a commit with my implementation and this assertion. I've updated the test to use two Swift files, which previously caused the underlying Clang module to be scanned twice.

[slavapestov]

Excellent, thanks. The PR looks good to me.

[QuietMisdreavus]

@swift-ci Please test

[QuietMisdreavus]

@swift-ci Please build toolchain macOS platform

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - 8a896a7

[QuietMisdreavus]

@swift-ci Please test Linux

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - 8a896a7

[swift-ci]

macOS Toolchain
Download Toolchain
Git Sha - 8a896a7

Install command
tar -zxf swift-PR-40810-1282-osx.tar.gz --directory ~/

[QuietMisdreavus]

@swift-ci Please smoke test