Skip to content

Commit cc830e6

Browse files
authored
Merge pull request #69552 from artemcm/DepScanTautologicalImports
[Dependency Scanning] Detect and ignore (and warn about) tautological imports
2 parents 6a81131 + 9c58b9f commit cc830e6

File tree

6 files changed

+55
-24
lines changed

6 files changed

+55
-24
lines changed

include/swift/AST/ModuleDependencies.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ using ModuleDependencyIDSetVector =
123123

124124
namespace dependencies {
125125
std::string createEncodedModuleKindAndName(ModuleDependencyID id);
126+
bool checkImportNotTautological(const ImportPath::Module,
127+
const SourceLoc,
128+
const SourceFile&,
129+
bool);
126130
}
127131

128132
/// Base class for the variant storage of ModuleDependencyInfo.

lib/AST/ModuleDependencies.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "swift/AST/ModuleDependencies.h"
1717
#include "swift/AST/Decl.h"
1818
#include "swift/AST/DiagnosticsFrontend.h"
19+
#include "swift/AST/DiagnosticsSema.h"
1920
#include "swift/AST/SourceFile.h"
2021
#include "swift/Frontend/Frontend.h"
2122
#include "llvm/CAS/CASProvidingFileSystem.h"
@@ -145,6 +146,11 @@ void ModuleDependencyInfo::addModuleImport(
145146
if (importedModuleName == BUILTIN_NAME)
146147
continue;
147148

149+
// Ignore/diagnose tautological imports akin to import resolution
150+
if (!swift::dependencies::checkImportNotTautological(
151+
realPath, importDecl->getLoc(), sf, importDecl->isExported()))
152+
continue;
153+
148154
addModuleImport(realPath, &alreadyAddedModules);
149155

150156
// Additionally, keep track of which dependencies of a Source
@@ -408,6 +414,35 @@ SwiftDependencyScanningService::SwiftDependencyScanningService() {
408414
SharedFilesystemCache.emplace();
409415
}
410416

417+
bool
418+
swift::dependencies::checkImportNotTautological(const ImportPath::Module modulePath,
419+
const SourceLoc importLoc,
420+
const SourceFile &SF,
421+
bool isExported) {
422+
if (modulePath.front().Item != SF.getParentModule()->getName() ||
423+
// Overlays use an @_exported self-import to load their clang module.
424+
isExported ||
425+
// Imports of your own submodules are allowed in cross-language libraries.
426+
modulePath.size() != 1 ||
427+
// SIL files self-import to get decls from the rest of the module.
428+
SF.Kind == SourceFileKind::SIL)
429+
return true;
430+
431+
ASTContext &ctx = SF.getASTContext();
432+
433+
StringRef filename = llvm::sys::path::filename(SF.getFilename());
434+
if (filename.empty())
435+
ctx.Diags.diagnose(importLoc, diag::sema_import_current_module,
436+
modulePath.front().Item);
437+
else
438+
ctx.Diags.diagnose(importLoc, diag::sema_import_current_module_with_file,
439+
filename, modulePath.front().Item);
440+
441+
return false;
442+
443+
return false;
444+
}
445+
411446
void SwiftDependencyTracker::addCommonSearchPathDeps(
412447
const SearchPathOptions &Opts) {
413448
// Add SDKSetting file.

lib/Sema/ImportResolution.cpp

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#define DEBUG_TYPE "swift-import-resolution"
1818
#include "swift/AST/ASTWalker.h"
1919
#include "swift/AST/DiagnosticsSema.h"
20+
#include "swift/AST/ModuleDependencies.h"
2021
#include "swift/AST/ModuleLoader.h"
2122
#include "swift/AST/ModuleNameLookup.h"
2223
#include "swift/AST/NameLookup.h"
@@ -602,28 +603,9 @@ UnboundImport::UnboundImport(ImportDecl *ID)
602603
}
603604

604605
bool UnboundImport::checkNotTautological(const SourceFile &SF) {
605-
// Exit early if this is not a self-import.
606-
auto modulePath = import.module.getModulePath();
607-
if (modulePath.front().Item != SF.getParentModule()->getName() ||
608-
// Overlays use an @_exported self-import to load their clang module.
609-
import.options.contains(ImportFlags::Exported) ||
610-
// Imports of your own submodules are allowed in cross-language libraries.
611-
modulePath.size() != 1 ||
612-
// SIL files self-import to get decls from the rest of the module.
613-
SF.Kind == SourceFileKind::SIL)
614-
return true;
615-
616-
ASTContext &ctx = SF.getASTContext();
617-
618-
StringRef filename = llvm::sys::path::filename(SF.getFilename());
619-
if (filename.empty())
620-
ctx.Diags.diagnose(importLoc, diag::sema_import_current_module,
621-
modulePath.front().Item);
622-
else
623-
ctx.Diags.diagnose(importLoc, diag::sema_import_current_module_with_file,
624-
filename, modulePath.front().Item);
625-
626-
return false;
606+
return swift::dependencies::checkImportNotTautological(
607+
import.module.getModulePath(), importLoc, SF,
608+
import.options.contains(ImportFlags::Exported));
627609
}
628610

629611
bool UnboundImport::checkModuleLoaded(ModuleDecl *M, SourceFile &SF) {

test/ScanDependencies/module_deps_cross_import_overlay.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
// REQUIRES: executable_test
1212
// REQUIRES: objc_interop
1313

14-
import CrossImportTestModule
14+
@_exported import CrossImportTestModule
1515
import EWrapper
1616
import SubEWrapper
1717

test/ScanDependencies/no_main_module_cross_import.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
// Ordinarily, importing `E` and `SubE` triggers a cross-import of `_cross_import_E`, but not here, because we are building `SubE` Swift module itself.
1111
import EWrapper
12-
import SubE
12+
@_exported import SubE
1313

1414
// CHECK: "directDependencies": [
1515
// CHECK-DAG: "swift": "EWrapper"
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %empty-directory(%t/clang-module-cache)
3+
4+
// RUN: %target-swift-frontend -scan-dependencies -module-cache-path %t/clang-module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -module-name TautologicalModule -verify
5+
// Check the contents of the JSON output
6+
// RUN: %validate-json %t/deps.json | %FileCheck %s
7+
8+
import TautologicalModule // expected-warning {{file 'tautological_import.swift' is part of module 'TautologicalModule'; ignoring import}}
9+
10+
// CHECK: "mainModuleName": "TautologicalModule"

0 commit comments

Comments
 (0)