Skip to content

Commit 75a83da

Browse files
committed
Implement SE-0075: CanImport
This implementation required a compromise between parser performance and AST structuring. On the one hand, Parse must be fast in order to keep things in the IDE zippy, on the other we must hit the disk to properly resolve 'canImport' conditions and inject members of the active clause into the AST. Additionally, a Parse-only pass may not provide platform-specific information to the compiler invocation and so may mistakenly activate or de-activate branches in the if-configuration decl. The compromise is to perform condition evaluation only when continuing on to semantic analysis. This keeps the parser quick and avoids the unpacking that parse does for active conditions while still retaining the ability to see through to an active condition when we know we're moving on to semantic analysis anyways.
1 parent 0bae2e5 commit 75a83da

File tree

16 files changed

+39
-22
lines changed

16 files changed

+39
-22
lines changed

include/swift/AST/ASTContext.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,9 @@ class ASTContext {
262262
llvm::DenseMap<const Pattern *, DeclContext *>
263263
DelayedPatternContexts;
264264

265+
/// Cache of module names that fail the 'canImport' test in this context.
266+
llvm::SmallPtrSet<Identifier, 8> FailedModuleImportNames;
267+
265268
public:
266269
/// \brief Retrieve the allocator for the given arena.
267270
llvm::BumpPtrAllocator &

include/swift/Basic/LangOptions.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,9 @@ namespace swift {
4343
Endianness,
4444
/// Runtime support (_ObjC or _Native)
4545
Runtime,
46+
/// Conditional import of module
47+
CanImport,
4648
};
47-
enum { NumPlatformConditionKind = 4 };
4849

4950
/// Describes which Swift 3 Objective-C inference warnings should be
5051
/// emitted.
@@ -341,8 +342,7 @@ namespace swift {
341342
}
342343

343344
private:
344-
llvm::SmallVector<std::pair<PlatformConditionKind, std::string>,
345-
NumPlatformConditionKind>
345+
llvm::SmallVector<std::pair<PlatformConditionKind, std::string>, 4>
346346
PlatformConditionValues;
347347
llvm::SmallVector<std::string, 2> CustomConditionalCompilationFlags;
348348
};

include/swift/Frontend/Frontend.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,9 @@ class CompilerInstance {
426426

427427
/// Parses the input file but does no type-checking or module imports.
428428
/// Note that this only supports parsing an invocation with a single file.
429-
void performParseOnly();
429+
///
430+
///
431+
void performParseOnly(bool EvaluateConditionals = false);
430432

431433
/// Frees up the ASTContext and SILModule objects that this instance is
432434
/// holding on.

include/swift/Parse/PersistentParserState.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,9 @@ class PersistentParserState {
9898
};
9999

100100
bool InPoundLineEnvironment = false;
101-
101+
// FIXME: When condition evaluation moves to a later phase, remove this bit
102+
// and adjust the client call 'performParseOnly'.
103+
bool PerformConditionEvaluation = true;
102104
private:
103105
ScopeInfo ScopeInfo;
104106
typedef llvm::DenseMap<AbstractFunctionDecl *,

lib/AST/ASTContext.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1458,12 +1458,18 @@ bool ASTContext::canImportModule(std::pair<Identifier, SourceLoc> ModulePath) {
14581458
if (getLoadedModule(ModulePath) != nullptr)
14591459
return true;
14601460

1461+
// If we've failed loading this module before, don't look for it again.
1462+
if (FailedModuleImportNames.count(ModulePath.first))
1463+
return false;
1464+
14611465
// Otherwise, ask the module loaders.
14621466
for (auto &importer : Impl.ModuleLoaders) {
14631467
if (importer->canImportModule(ModulePath)) {
14641468
return true;
14651469
}
14661470
}
1471+
1472+
FailedModuleImportNames.insert(ModulePath.first);
14671473
return false;
14681474
}
14691475

lib/Basic/LangOptions.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ checkPlatformConditionSupported(PlatformConditionKind Kind, StringRef Value,
9898
case PlatformConditionKind::Runtime:
9999
return contains(SupportedConditionalCompilationRuntimes, Value,
100100
suggestions);
101+
case PlatformConditionKind::CanImport:
102+
// All importable names are valid.
103+
// FIXME: Perform some kind of validation of the string?
104+
return true;
101105
}
102106
llvm_unreachable("Unhandled enum value");
103107
}

lib/Frontend/Frontend.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,7 @@ void CompilerInstance::performSema() {
581581
finishTypeChecking(*SF);
582582
}
583583

584-
void CompilerInstance::performParseOnly() {
584+
void CompilerInstance::performParseOnly(bool EvaluateConditionals) {
585585
const InputFileKind Kind = Invocation.getInputKind();
586586
ModuleDecl *MainModule = getMainModule();
587587
Context->LoadedModules[MainModule->getName()] = MainModule;
@@ -608,6 +608,7 @@ void CompilerInstance::performParseOnly() {
608608
}
609609

610610
PersistentParserState PersistentState;
611+
PersistentState.PerformConditionEvaluation = EvaluateConditionals;
611612
// Parse all the library files.
612613
for (auto BufferID : BufferIDs) {
613614
if (BufferID == MainBufferID)

lib/Parse/ParseIfConfig.cpp

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ Optional<PlatformConditionKind> getPlatformConditionKind(StringRef Name) {
3636
.Case("arch", PlatformConditionKind::Arch)
3737
.Case("_endian", PlatformConditionKind::Endianness)
3838
.Case("_runtime", PlatformConditionKind::Runtime)
39+
.Case("canImport", PlatformConditionKind::CanImport)
3940
.Default(None);
4041
}
4142

@@ -289,7 +290,7 @@ class ValidateIfConfigCondition :
289290
return E;
290291
}
291292

292-
// ( 'os' | 'arch' | '_endian' | '_runtime' ) '(' identifier ')''
293+
// ( 'os' | 'arch' | '_endian' | '_runtime' | 'canImport') '(' identifier ')''
293294
auto Kind = getPlatformConditionKind(*KindName);
294295
if (!Kind.hasValue()) {
295296
D.diagnose(E->getLoc(), diag::unsupported_platform_condition_expression);
@@ -331,6 +332,8 @@ class ValidateIfConfigCondition :
331332
DiagName = "architecture"; break;
332333
case PlatformConditionKind::Endianness:
333334
DiagName = "endianness"; break;
335+
case PlatformConditionKind::CanImport:
336+
DiagName = "import conditional"; break;
334337
case PlatformConditionKind::Runtime:
335338
llvm_unreachable("handled above");
336339
}
@@ -450,6 +453,9 @@ class EvaluateIfConfigCondition :
450453
Str, SourceLoc(), nullptr).getValue();
451454
auto thisVersion = Ctx.LangOpts.EffectiveLanguageVersion;
452455
return thisVersion >= Val;
456+
} else if (KindName == "canImport") {
457+
auto Str = extractExprSource(Ctx.SourceMgr, Arg);
458+
return Ctx.canImportModule({ Ctx.getIdentifier(Str) , E->getLoc() });
453459
}
454460

455461
auto Val = getDeclRefStr(Arg);
@@ -567,9 +573,10 @@ ParserResult<IfConfigDecl> Parser::parseIfConfig(
567573
Expr *Condition = nullptr;
568574
bool isActive = false;
569575

570-
// Parse and evaluate the directive.
576+
// Parse the condition. Evaluate it to determine the active
577+
// clause unless we're doing a parse-only pass.
571578
if (isElse) {
572-
isActive = !foundActive;
579+
isActive = !foundActive && State->PerformConditionEvaluation;
573580
} else {
574581
llvm::SaveAndRestore<bool> S(InPoundIfEnvironment, true);
575582
ParserResult<Expr> Result = parseExprSequence(diag::expected_expr,
@@ -582,8 +589,9 @@ ParserResult<IfConfigDecl> Parser::parseIfConfig(
582589
// Error in the condition;
583590
isActive = false;
584591
isVersionCondition = false;
585-
} else if (!foundActive) {
586-
// Evaluate the condition only if we haven't found any active one.
592+
} else if (!foundActive && State->PerformConditionEvaluation) {
593+
// Evaluate the condition only if we haven't found any active one and
594+
// we're not in parse-only mode.
587595
isActive = evaluateIfConfigCondition(Condition, Context);
588596
isVersionCondition = isVersionIfConfigCondition(Condition);
589597
}

test/ClangImporter/MixedSource/can_import_objc_idempotent.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource -I %t) -I %S/../Inputs/custom-modules -typecheck %s -verify
1414

1515
// REQUIRES: objc_interop
16-
// REQUIRES: can_import
1716

1817
// Test that 'canImport(Foo)' directives do not open symbols from 'Foo' into the
1918
// current module. Only an 'import Foo' statement should do this.

test/ClangImporter/can_import_missing_requirement.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -I %S/Inputs/missing-requirement %s -verify
22

33
// REQUIRES: objc_interop
4-
// REQUIRES: can_import
54

65
class Unique {}
76

0 commit comments

Comments
 (0)