Skip to content

Sema: Diagnose inconsistent use of @_weakLinked import #60519

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

Merged
merged 4 commits into from
Aug 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -2978,6 +2978,13 @@ ERROR(implementation_only_override_import_without_attr,none,
"override of %0 imported as implementation-only must be declared "
"'@_implementationOnly'", (DescriptiveDeclKind))

ERROR(import_attr_conflict,none,
"%0 inconsistently imported with %1",
(Identifier, DeclAttribute))
NOTE(import_attr_conflict_here,none,
"imported with %0 here",
(DeclAttribute))

// Derived conformances
ERROR(cannot_synthesize_init_in_extension_of_nonfinal,none,
"implementation of %0 for non-final class cannot be automatically "
Expand Down
2 changes: 2 additions & 0 deletions include/swift/AST/Import.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ enum class ImportFlags {
/// \see ImportFlags
using ImportOptions = OptionSet<ImportFlags>;

void simple_display(llvm::raw_ostream &out, ImportOptions options);

// MARK: - Import Paths

namespace detail {
Expand Down
14 changes: 12 additions & 2 deletions include/swift/AST/SourceFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,16 @@ class SourceFile final : public FileUnit {
ParserStatePtr DelayedParserState =
ParserStatePtr(/*ptr*/ nullptr, /*deleter*/ nullptr);

friend class HasImportsMatchingFlagRequest;

/// Indicates which import options have valid caches. Storage for
/// \c HasImportsMatchingFlagRequest.
ImportOptions validCachedImportOptions;

/// The cached computation of which import flags are present in the file.
/// Storage for \c HasImportsMatchingFlagRequest.
ImportOptions cachedImportOptions;

friend ASTContext;

public:
Expand Down Expand Up @@ -342,9 +352,9 @@ class SourceFile final : public FileUnit {
hasTestableOrPrivateImport(AccessLevel accessLevel, const ValueDecl *ofDecl,
ImportQueryKind kind = TestableAndPrivate) const;

/// Does this source file have any implementation-only imports?
/// Does this source file have any imports with \c flag?
/// If not, we can fast-path module checks.
bool hasImplementationOnlyImports() const;
bool hasImportsWithFlag(ImportFlags flag) const;

/// Get the most permissive restriction applied to the imports of \p module.
RestrictedImportKind getRestrictedImportKind(const ModuleDecl *module) const;
Expand Down
43 changes: 36 additions & 7 deletions include/swift/AST/TypeCheckRequests.h
Original file line number Diff line number Diff line change
Expand Up @@ -3202,21 +3202,24 @@ class ModuleImplicitImportsRequest
bool isCached() const { return true; }
};

/// Checks whether a file performs an implementation-only import.
class HasImplementationOnlyImportsRequest
: public SimpleRequest<HasImplementationOnlyImportsRequest,
bool(SourceFile *), RequestFlags::Cached> {
/// Checks whether a file contains any import declarations with the given flag.
class HasImportsMatchingFlagRequest
: public SimpleRequest<HasImportsMatchingFlagRequest,
bool(SourceFile *, ImportFlags),
RequestFlags::SeparatelyCached> {
public:
using SimpleRequest::SimpleRequest;

private:
friend SimpleRequest;

bool evaluate(Evaluator &evaluator, SourceFile *SF) const;
bool evaluate(Evaluator &evaluator, SourceFile *SF, ImportFlags flag) const;

public:
// Cached.
bool isCached() const { return true; }
Optional<bool> getCachedResult() const;
void cacheResult(bool value) const;
};

/// Get the library level of a module.
Expand Down Expand Up @@ -3260,15 +3263,15 @@ class ResolveTypeRequest
void simple_display(llvm::raw_ostream &out, const TypeResolution *resolution);
SourceLoc extractNearestSourceLoc(const TypeRepr *repr);

/// Checks to see if any of the imports in a module use `@_implementationOnly`
/// Checks to see if any of the imports in a module use \c @_implementationOnly
/// in one file and not in another.
///
/// Like redeclaration checking, but for imports.
///
/// This is a request purely to ensure that we don't need to perform the same
/// checking for each file we resolve imports for.
/// FIXME: Once import resolution operates at module-level, this checking can
/// integrated into it.
/// be integrated into it.
class CheckInconsistentImplementationOnlyImportsRequest
: public SimpleRequest<CheckInconsistentImplementationOnlyImportsRequest,
evaluator::SideEffect(ModuleDecl *),
Expand All @@ -3286,6 +3289,32 @@ class CheckInconsistentImplementationOnlyImportsRequest
bool isCached() const { return true; }
};

/// Checks to see if any of the imports in a module use \c @_weakLinked
/// in one file and not in another.
///
/// Like redeclaration checking, but for imports.
///
/// This is a request purely to ensure that we don't need to perform the same
/// checking for each file we resolve imports for.
/// FIXME: Once import resolution operates at module-level, this checking can
/// be integrated into it.
class CheckInconsistentWeakLinkedImportsRequest
: public SimpleRequest<CheckInconsistentWeakLinkedImportsRequest,
evaluator::SideEffect(ModuleDecl *),
RequestFlags::Cached> {
public:
using SimpleRequest::SimpleRequest;

private:
friend SimpleRequest;

evaluator::SideEffect evaluate(Evaluator &evaluator, ModuleDecl *mod) const;

public:
// Cached.
bool isCached() const { return true; }
};

/// Retrieves the primary source files in the main module.
// FIXME: This isn't really a type-checking request, if we ever split off a
// zone for more basic AST requests, this should be moved there.
Expand Down
6 changes: 4 additions & 2 deletions include/swift/AST/TypeCheckerTypeIDZone.def
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ SWIFT_REQUEST(TypeChecker, CallerSideDefaultArgExprRequest,
Expr *(DefaultArgumentExpr *), SeparatelyCached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, CheckInconsistentImplementationOnlyImportsRequest,
evaluator::SideEffect(ModuleDecl *), Cached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, CheckInconsistentWeakLinkedImportsRequest,
evaluator::SideEffect(ModuleDecl *), Cached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, CheckRedeclarationRequest,
evaluator::SideEffect(ValueDecl *),
Uncached, NoLocationInfo)
Expand Down Expand Up @@ -169,8 +171,8 @@ SWIFT_REQUEST(TypeChecker, HasCircularInheritedProtocolsRequest,
bool(ProtocolDecl *), Cached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, HasCircularRawValueRequest,
bool(EnumDecl *), Cached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, HasImplementationOnlyImportsRequest,
bool(SourceFile *), Cached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, HasImportsMatchingFlagRequest,
bool(SourceFile *, ImportOptions), Cached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, ModuleLibraryLevelRequest,
LibraryLevel(ModuleDecl *), Cached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, InferredGenericSignatureRequest,
Expand Down
61 changes: 53 additions & 8 deletions lib/AST/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2389,19 +2389,64 @@ void SourceFile::setImportUsedPreconcurrency(
PreconcurrencyImportsUsed.insert(import);
}

bool HasImplementationOnlyImportsRequest::evaluate(Evaluator &evaluator,
SourceFile *SF) const {
return llvm::any_of(SF->getImports(),
[](AttributedImport<ImportedModule> desc) {
return desc.options.contains(ImportFlags::ImplementationOnly);
});
bool HasImportsMatchingFlagRequest::evaluate(Evaluator &evaluator,
SourceFile *SF,
ImportFlags flag) const {
for (auto desc : SF->getImports()) {
if (desc.options.contains(flag))
return true;
}
return false;
}

Optional<bool> HasImportsMatchingFlagRequest::getCachedResult() const {
SourceFile *sourceFile = std::get<0>(getStorage());
ImportFlags flag = std::get<1>(getStorage());
if (sourceFile->validCachedImportOptions.contains(flag))
return sourceFile->cachedImportOptions.contains(flag);

return None;
}

void HasImportsMatchingFlagRequest::cacheResult(bool value) const {
SourceFile *sourceFile = std::get<0>(getStorage());
ImportFlags flag = std::get<1>(getStorage());

sourceFile->validCachedImportOptions |= flag;
if (value)
sourceFile->cachedImportOptions |= flag;
}

void swift::simple_display(llvm::raw_ostream &out, ImportOptions options) {
using Flag = std::pair<ImportFlags, StringRef>;
Flag possibleFlags[] = {
#define FLAG(Name) {ImportFlags::Name, #Name},
FLAG(Exported)
FLAG(Testable)
FLAG(PrivateImport)
FLAG(ImplementationOnly)
FLAG(SPIAccessControl)
FLAG(Preconcurrency)
FLAG(WeakLinked)
FLAG(Reserved)
#undef FLAG
};

auto flagsToPrint = llvm::make_filter_range(
possibleFlags, [&](Flag flag) { return options & flag.first; });

out << "{ ";
interleave(
flagsToPrint, [&](Flag flag) { out << flag.second; },
[&] { out << ", "; });
out << " }";
}

bool SourceFile::hasImplementationOnlyImports() const {
bool SourceFile::hasImportsWithFlag(ImportFlags flag) const {
auto &ctx = getASTContext();
auto *mutableThis = const_cast<SourceFile *>(this);
return evaluateOrDefault(
ctx.evaluator, HasImplementationOnlyImportsRequest{mutableThis}, false);
ctx.evaluator, HasImportsMatchingFlagRequest{mutableThis, flag}, false);
}

bool SourceFile::hasTestableOrPrivateImport(
Expand Down
6 changes: 6 additions & 0 deletions lib/AST/TypeCheckRequests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1474,6 +1474,12 @@ void swift::simple_display(llvm::raw_ostream &out,
out << ")";
}

if (import.options.contains(ImportFlags::Preconcurrency))
out << " preconcurrency";

if (import.options.contains(ImportFlags::WeakLinked))
out << " weak-linked";

out << " ]";
}

Expand Down
Loading