diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 54e1bb2918f..826d374a9bb 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -8149,15 +8149,22 @@ NOTE(note_reference_exclusivity_unchecked,none, NOTE(note_use_of_unsafe_conformance_is_unsafe,none, "@unsafe conformance of %0 to %kind1 involves unsafe code", (Type, const ValueDecl *)) +NOTE(note_unsafe_storage,none, + "%kindbase1 involves unsafe type %2", (bool, const ValueDecl *, Type)) -GROUPED_WARNING(decl_signature_involves_unsafe,Unsafe,none, - "%kindbase0 has an interface that involves unsafe types", +GROUPED_WARNING(decl_unsafe_storage,Unsafe,none, + "%kindbase0 has storage involving unsafe types", (const Decl *)) -NOTE(decl_signature_mark_unsafe,none, - "add '@unsafe' to indicate that this declaration is unsafe to use", +NOTE(decl_storage_mark_unsafe,none, + "add '@unsafe' if this type is also unsafe to use", ()) -NOTE(decl_signature_mark_safe,none, - "add '@safe' to indicate that this declaration is memory-safe to use", ()) +NOTE(decl_storage_mark_safe,none, + "add '@safe' if this type encapsulates the unsafe storage in " + "a safe interface", ()) + +GROUPED_WARNING(unsafe_superclass,Unsafe,none, + "%kindbase0 has superclass involving unsafe type %1", + (const ValueDecl *, Type)) GROUPED_WARNING(conformance_involves_unsafe,Unsafe,none, "conformance of %0 to %kind1 involves unsafe code; use '@unsafe' to " diff --git a/include/swift/AST/UnsafeUse.h b/include/swift/AST/UnsafeUse.h index b96c7bfc830..30e21da5e07 100644 --- a/include/swift/AST/UnsafeUse.h +++ b/include/swift/AST/UnsafeUse.h @@ -45,6 +45,8 @@ public: NonisolatedUnsafe, /// A reference to an unsafe declaration. ReferenceToUnsafe, + /// A reference to an unsafe storage. + ReferenceToUnsafeStorage, /// A reference to a typealias that is not itself unsafe, but has /// an unsafe underlying type. ReferenceToUnsafeThroughTypealias, @@ -113,6 +115,7 @@ private: kind == ExclusivityUnchecked || kind == NonisolatedUnsafe || kind == ReferenceToUnsafe || + kind == ReferenceToUnsafeStorage || kind == ReferenceToUnsafeThroughTypealias || kind == CallToUnsafe); @@ -179,6 +182,12 @@ public: decl, type, location); } + static UnsafeUse forReferenceToUnsafeStorage(const Decl *decl, + Type type, + SourceLoc location) { + return forReference(ReferenceToUnsafeStorage, decl, type, location); + } + static UnsafeUse forReferenceToUnsafeThroughTypealias(const Decl *decl, Type type, SourceLoc location) { @@ -215,6 +224,7 @@ public: case ExclusivityUnchecked: case NonisolatedUnsafe: case ReferenceToUnsafe: + case ReferenceToUnsafeStorage: case ReferenceToUnsafeThroughTypealias: case CallToUnsafe: return SourceLoc( @@ -246,6 +256,7 @@ public: case ExclusivityUnchecked: case NonisolatedUnsafe: case ReferenceToUnsafe: + case ReferenceToUnsafeStorage: case ReferenceToUnsafeThroughTypealias: case CallToUnsafe: storage.entity.location = loc.getOpaquePointerValue(); @@ -266,6 +277,7 @@ public: case ExclusivityUnchecked: case NonisolatedUnsafe: case ReferenceToUnsafe: + case ReferenceToUnsafeStorage: case ReferenceToUnsafeThroughTypealias: case CallToUnsafe: return storage.entity.decl; @@ -299,6 +311,7 @@ public: case NonisolatedUnsafe: case ReferenceToUnsafe: case ReferenceToUnsafeThroughTypealias: + case ReferenceToUnsafeStorage: case CallToUnsafe: case UnsafeConformance: case PreconcurrencyImport: @@ -324,6 +337,7 @@ public: case ExclusivityUnchecked: case NonisolatedUnsafe: case ReferenceToUnsafe: + case ReferenceToUnsafeStorage: case ReferenceToUnsafeThroughTypealias: case CallToUnsafe: return storage.entity.type; @@ -348,6 +362,7 @@ public: case ExclusivityUnchecked: case NonisolatedUnsafe: case ReferenceToUnsafe: + case ReferenceToUnsafeStorage: case ReferenceToUnsafeThroughTypealias: case CallToUnsafe: case PreconcurrencyImport: diff --git a/include/swift/ClangImporter/ClangImporterRequests.h b/include/swift/ClangImporter/ClangImporterRequests.h index f91f89b755d..0b8472709db 100644 --- a/include/swift/ClangImporter/ClangImporterRequests.h +++ b/include/swift/ClangImporter/ClangImporterRequests.h @@ -31,6 +31,7 @@ namespace swift { class Decl; class DeclName; class EnumDecl; +enum class ExplicitSafety; /// The input type for a clang direct lookup request. struct ClangDirectLookupDescriptor final { @@ -564,6 +565,25 @@ private: void simple_display(llvm::raw_ostream &out, EscapabilityLookupDescriptor desc); SourceLoc extractNearestSourceLoc(EscapabilityLookupDescriptor desc); +/// Determine the safety of the given Clang declaration. +class ClangDeclExplicitSafety + : public SimpleRequest { +public: + using SimpleRequest::SimpleRequest; + + // Source location + SourceLoc getNearestLoc() const { return SourceLoc(); }; + bool isCached() const; + +private: + friend SimpleRequest; + + // Evaluation. + ExplicitSafety evaluate(Evaluator &evaluator, SafeUseOfCxxDeclDescriptor desc) const; +}; + #define SWIFT_TYPEID_ZONE ClangImporter #define SWIFT_TYPEID_HEADER "swift/ClangImporter/ClangImporterTypeIDZone.def" #include "swift/Basic/DefineTypeIDZone.h" diff --git a/include/swift/ClangImporter/ClangImporterTypeIDZone.def b/include/swift/ClangImporter/ClangImporterTypeIDZone.def index 4993eacd90e..b4ea68a4053 100644 --- a/include/swift/ClangImporter/ClangImporterTypeIDZone.def +++ b/include/swift/ClangImporter/ClangImporterTypeIDZone.def @@ -45,3 +45,6 @@ SWIFT_REQUEST(ClangImporter, CustomRefCountingOperation, SWIFT_REQUEST(ClangImporter, ClangTypeEscapability, CxxEscapability(EscapabilityLookupDescriptor), Cached, NoLocationInfo) +SWIFT_REQUEST(ClangImporter, ClangDeclExplicitSafety, + ExplicitSafety(SafeUseOfCxxDeclDescriptor), Cached, + NoLocationInfo) diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 2d02f368a21..8406d025812 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -1150,6 +1150,14 @@ ExplicitSafety Decl::getExplicitSafety() const { if (auto safety = getExplicitSafetyFromAttrs(this)) return *safety; + // If this declaration is from C, ask the Clang importer. + if (auto clangDecl = getClangDecl()) { + ASTContext &ctx = getASTContext(); + return evaluateOrDefault( + ctx.evaluator, ClangDeclExplicitSafety({clangDecl, ctx}), + ExplicitSafety::Unspecified); + } + // Inference: Check the enclosing context. if (auto enclosingDC = getDeclContext()) { // Is this an extension with @safe or @unsafe on it? diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index 75158605be4..6eae8f17c74 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -8215,6 +8215,106 @@ CustomRefCountingOperationResult CustomRefCountingOperation::evaluate( return {CustomRefCountingOperationResult::tooManyFound, nullptr, name}; } +/// Check whether the given Clang type involves an unsafe type. +static bool hasUnsafeType( + Evaluator &evaluator, ASTContext &swiftContext, clang::QualType clangType +) { + // Handle pointers. + auto pointeeType = clangType->getPointeeType(); + if (!pointeeType.isNull()) { + // Function pointers are okay. + if (pointeeType->isFunctionType()) + return false; + + // Pointers to record types are okay if they come in as foreign reference + // types. + if (auto recordDecl = pointeeType->getAsRecordDecl()) { + if (hasImportAsRefAttr(recordDecl)) + return false; + } + + // All other pointers are considered unsafe. + return true; + } + + // Handle records recursively. + if (auto recordDecl = clangType->getAsTagDecl()) { + auto safety = evaluateOrDefault( + evaluator, + ClangDeclExplicitSafety({recordDecl, swiftContext}), + ExplicitSafety::Unspecified); + switch (safety) { + case ExplicitSafety::Unsafe: + return true; + + case ExplicitSafety::Safe: + case ExplicitSafety::Unspecified: + return false; + } + } + + // Everything else is safe. + return false; +} + +ExplicitSafety ClangDeclExplicitSafety::evaluate( + Evaluator &evaluator, + SafeUseOfCxxDeclDescriptor desc +) const { + // FIXME: Somewhat duplicative with importAsUnsafe. + // FIXME: Also similar to hasPointerInSubobjects + // FIXME: should probably also subsume IsSafeUseOfCxxDecl + + // Explicitly unsafe. + auto decl = desc.decl; + if (hasUnsafeAPIAttr(decl) || hasSwiftAttribute(decl, "unsafe")) + return ExplicitSafety::Unsafe; + + // Explicitly safe. + if (hasSwiftAttribute(decl, "safe")) + return ExplicitSafety::Safe; + + // Enums are always safe. + if (isa(decl)) + return ExplicitSafety::Safe; + + // If it's not a record, leave it unspecified. + auto recordDecl = dyn_cast(decl); + if (!recordDecl) + return ExplicitSafety::Unspecified; + + // Escapable and non-escapable annotations imply that the declaration is + // safe. + if (hasNonEscapableAttr(recordDecl) || hasEscapableAttr(recordDecl)) + return ExplicitSafety::Safe; + + // If we don't have a definition, leave it unspecified. + recordDecl = recordDecl->getDefinition(); + if (!recordDecl) + return ExplicitSafety::Unspecified; + + // If this is a C++ class, check its bases. + if (auto cxxRecordDecl = dyn_cast(recordDecl)) { + for (auto base : cxxRecordDecl->bases()) { + if (hasUnsafeType(evaluator, desc.ctx, base.getType())) + return ExplicitSafety::Unsafe; + } + } + + // Check the fields. + for (auto field : recordDecl->fields()) { + if (hasUnsafeType(evaluator, desc.ctx, field->getType())) + return ExplicitSafety::Unsafe; + } + + // Okay, call it safe. + return ExplicitSafety::Safe; +} + +bool ClangDeclExplicitSafety::isCached() const { + return isa(std::get<0>(getStorage()).decl); +} + void ClangImporter::withSymbolicFeatureEnabled( llvm::function_ref callback) { llvm::SaveAndRestore oldImportSymbolicCXXDecls( diff --git a/lib/Sema/TypeCheckAccess.cpp b/lib/Sema/TypeCheckAccess.cpp index 76dfd891e43..8bbdbb53f90 100644 --- a/lib/Sema/TypeCheckAccess.cpp +++ b/lib/Sema/TypeCheckAccess.cpp @@ -1332,7 +1332,7 @@ public: if (macroDecl) { diagnoseDeclAvailability( macroDecl, customAttr->getTypeRepr()->getSourceRange(), nullptr, - ExportContext::forDeclSignature(const_cast(D), nullptr), + ExportContext::forDeclSignature(const_cast(D)), std::nullopt); } } @@ -2678,35 +2678,9 @@ void swift::checkAccessControl(Decl *D) { if (isa(D)) return; - // If we need to gather all unsafe uses from the declaration signature, - // do so now. - llvm::SmallVector unsafeUsesVec; - llvm::SmallVectorImpl *unsafeUses = nullptr; - if (D->getASTContext().LangOpts.hasFeature(Feature::WarnUnsafe) && - !D->isImplicit() && - D->getExplicitSafety() == ExplicitSafety::Unspecified) { - unsafeUses = &unsafeUsesVec; - } - - auto where = ExportContext::forDeclSignature(D, unsafeUses); + auto where = ExportContext::forDeclSignature(D); if (where.isImplicit()) return; DeclAvailabilityChecker(where).visit(D); - - // If there were any unsafe uses, this declaration needs "@unsafe". - if (!unsafeUsesVec.empty()) { - D->diagnose(diag::decl_signature_involves_unsafe, D); - - ASTContext &ctx = D->getASTContext(); - auto insertLoc = D->getAttributeInsertionLoc(/*forModifier=*/false); - - ctx.Diags.diagnose(insertLoc, diag::decl_signature_mark_unsafe) - .fixItInsert(insertLoc, "@unsafe "); - ctx.Diags.diagnose(insertLoc, diag::decl_signature_mark_safe) - .fixItInsert(insertLoc, "@safe "); - - std::for_each(unsafeUsesVec.begin(), unsafeUsesVec.end(), - diagnoseUnsafeUse); - } } diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index c9d6ca4850a..3d4173275df 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -3280,7 +3280,7 @@ SynthesizeMainFunctionRequest::evaluate(Evaluator &evaluator, return nullptr; } - auto where = ExportContext::forDeclSignature(D, nullptr); + auto where = ExportContext::forDeclSignature(D); diagnoseDeclAvailability(mainFunction, attr->getRange(), nullptr, where, std::nullopt); diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index f17d58d1fd4..be95b1d0b19 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -247,8 +247,7 @@ static void computeExportContextBits(ASTContext &Ctx, Decl *D, bool *spi, } } -ExportContext ExportContext::forDeclSignature( - Decl *D, llvm::SmallVectorImpl *unsafeUses) { +ExportContext ExportContext::forDeclSignature(Decl *D) { auto &Ctx = D->getASTContext(); auto *DC = D->getInnermostDeclContext(); @@ -264,7 +263,7 @@ ExportContext ExportContext::forDeclSignature( bool exported = ::isExported(D); - return ExportContext(DC, availabilityContext, fragileKind, unsafeUses, + return ExportContext(DC, availabilityContext, fragileKind, nullptr, spi, exported, implicit); } @@ -287,8 +286,7 @@ ExportContext ExportContext::forFunctionBody(DeclContext *DC, SourceLoc loc) { ExportContext ExportContext::forConformance(DeclContext *DC, ProtocolDecl *proto) { assert(isa(DC) || isa(DC)); - auto where = forDeclSignature(DC->getInnermostDeclarationDeclContext(), - nullptr); + auto where = forDeclSignature(DC->getInnermostDeclarationDeclContext()); where.Exported &= proto->getFormalAccessScope( DC, /*usableFromInlineAsPublic*/true).isPublic(); @@ -2914,7 +2912,7 @@ void swift::diagnoseOverrideOfUnavailableDecl(ValueDecl *override, // FIXME: [availability] Take an unsatisfied constraint as input instead of // recomputing it. - ExportContext where = ExportContext::forDeclSignature(override, nullptr); + ExportContext where = ExportContext::forDeclSignature(override); auto constraint = getAvailabilityConstraintsForDecl(base, where.getAvailability()) .getPrimaryConstraint(); diff --git a/lib/Sema/TypeCheckAvailability.h b/lib/Sema/TypeCheckAvailability.h index be9970aa0c4..a907545ffa1 100644 --- a/lib/Sema/TypeCheckAvailability.h +++ b/lib/Sema/TypeCheckAvailability.h @@ -127,8 +127,7 @@ public: /// /// If the declaration is exported, the resulting context is restricted to /// referencing exported types only. Otherwise it can reference anything. - static ExportContext forDeclSignature( - Decl *D, llvm::SmallVectorImpl *unsafeUses); + static ExportContext forDeclSignature(Decl *D); /// Create an instance describing the declarations that can be referenced /// from the given function's body. diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index 334f15f6b69..7eae7e9d457 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -2387,12 +2387,19 @@ public: "`" + VD->getBaseName().userFacingName().str() + "`"); } - // Expand extension macros. if (auto *nominal = dyn_cast(VD)) { + // Expand extension macros. (void)evaluateOrDefault( Ctx.evaluator, ExpandExtensionMacros{nominal}, { }); + + // If strict memory safety checking is enabled, check the storage + // of the nominal type. + if (Ctx.LangOpts.hasFeature(Feature::WarnUnsafe) && + !isa(nominal)) { + checkUnsafeStorage(nominal); + } } } } diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp index c10aa57e8ec..1f8bed90804 100644 --- a/lib/Sema/TypeCheckStorage.cpp +++ b/lib/Sema/TypeCheckStorage.cpp @@ -1236,7 +1236,7 @@ static Expr *buildStorageReference(AccessorDecl *accessor, diagnoseDeclAvailability( wrappedValue, var->getAttachedPropertyWrappers()[i]->getRangeWithAt(), nullptr, - ExportContext::forDeclSignature(accessor, nullptr)); + ExportContext::forDeclSignature(accessor)); } underlyingVars.push_back({ wrappedValue, isWrapperRefLValue }); diff --git a/lib/Sema/TypeCheckUnsafe.cpp b/lib/Sema/TypeCheckUnsafe.cpp index f8bcd8f5491..91ddb02a522 100644 --- a/lib/Sema/TypeCheckUnsafe.cpp +++ b/lib/Sema/TypeCheckUnsafe.cpp @@ -16,6 +16,7 @@ #include "TypeCheckAvailability.h" #include "TypeCheckConcurrency.h" +#include "TypeCheckInvertible.h" #include "TypeCheckType.h" #include "TypeCheckUnsafe.h" @@ -121,6 +122,7 @@ void swift::diagnoseUnsafeUse(const UnsafeUse &use) { } case UnsafeUse::ReferenceToUnsafe: + case UnsafeUse::ReferenceToUnsafeStorage: case UnsafeUse::CallToUnsafe: { bool isCall = use.getKind() == UnsafeUse::CallToUnsafe; auto decl = cast_or_null(use.getDecl()); @@ -133,7 +135,8 @@ void swift::diagnoseUnsafeUse(const UnsafeUse &use) { [&](Type specificType) { ctx.Diags.diagnose( loc, - diag::note_reference_to_unsafe_typed_decl, + use.getKind() == UnsafeUse::ReferenceToUnsafeStorage ? diag::note_unsafe_storage + : diag::note_reference_to_unsafe_typed_decl, isCall, decl, specificType); }); } else if (type) { @@ -376,3 +379,81 @@ void swift::diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type, diagnose(specificType ? specificType : type); } + +void swift::checkUnsafeStorage(NominalTypeDecl *nominal) { + // If the type is marked explicitly with @safe or @unsafe, there's nothing + // to check. + switch (nominal->getExplicitSafety()) { + case ExplicitSafety::Safe: + case ExplicitSafety::Unsafe: + return; + + case ExplicitSafety::Unspecified: + break; + } + + // Check whether the superclass is unsafe. If so, the only thing one can + // do is mark the class unsafe. + ASTContext &ctx = nominal->getASTContext(); + if (auto classDecl = dyn_cast(nominal)) { + if (Type superclassType = classDecl->getSuperclass()) { + superclassType = classDecl->mapTypeIntoContext(superclassType); + bool diagnosed = false; + diagnoseUnsafeType(ctx, classDecl->getLoc(), superclassType, [&](Type type) { + if (diagnosed) + return; + + classDecl->diagnose(diag::unsafe_superclass, classDecl, type) + .fixItInsert(classDecl->getAttributeInsertionLoc(false), "@unsafe "); + diagnosed = true; + }); + + if (diagnosed) + return; + } + } + + // Visitor that finds unsafe storage. + class UnsafeStorageVisitor: public StorageVisitor { + ASTContext &ctx; + SmallVectorImpl &unsafeUses; + + public: + UnsafeStorageVisitor(ASTContext &ctx, SmallVectorImpl &unsafeUses) + : ctx(ctx), unsafeUses(unsafeUses) { } + + bool operator()(VarDecl *property, Type propertyType) override { + diagnoseUnsafeType(ctx, property->getLoc(), propertyType, [&](Type type) { + unsafeUses.push_back( + UnsafeUse::forReferenceToUnsafeStorage( + property, propertyType, property->getLoc())); + }); + return false; + } + + bool operator()(EnumElementDecl *element, Type elementType) override { + diagnoseUnsafeType(ctx, element->getLoc(), elementType, [&](Type type) { + unsafeUses.push_back( + UnsafeUse::forReferenceToUnsafeStorage( + element, elementType, element->getLoc())); + }); + return false; + } + }; + + // Look for any unsafe storage in this nominal type. + SmallVector unsafeUses; + UnsafeStorageVisitor(ctx, unsafeUses).visit(nominal, nominal->getDeclContext()); + + // If we didn't find any unsafe storage, there's nothing to do. + if (unsafeUses.empty()) + return; + + // Complain about this type needing @safe or @unsafe. + nominal->diagnose(diag::decl_unsafe_storage, nominal); + nominal->diagnose(diag::decl_storage_mark_unsafe) + .fixItInsert(nominal->getAttributeInsertionLoc(false), "@unsafe "); + nominal->diagnose(diag::decl_storage_mark_safe) + .fixItInsert(nominal->getAttributeInsertionLoc(false), "@safe "); + std::for_each(unsafeUses.begin(), unsafeUses.end(), diagnoseUnsafeUse); +} diff --git a/lib/Sema/TypeCheckUnsafe.h b/lib/Sema/TypeCheckUnsafe.h index bc9c02284f7..0122e3c86ee 100644 --- a/lib/Sema/TypeCheckUnsafe.h +++ b/lib/Sema/TypeCheckUnsafe.h @@ -70,6 +70,9 @@ bool isUnsafeInConformance(const ValueDecl *requirement, void diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type, llvm::function_ref diagnose); +/// Check for unsafe storage within this nominal type declaration. +void checkUnsafeStorage(NominalTypeDecl *nominal); + } #endif // SWIFT_SEMA_TYPE_CHECK_UNSAFE_H diff --git a/test/Interop/Cxx/class/safe-interop-mode.swift b/test/Interop/Cxx/class/safe-interop-mode.swift index a7408a020e1..81324bf6101 100644 --- a/test/Interop/Cxx/class/safe-interop-mode.swift +++ b/test/Interop/Cxx/class/safe-interop-mode.swift @@ -71,23 +71,20 @@ import Test import CoreFoundation import CxxStdlib -// expected-warning@+3{{global function 'useUnsafeParam' has an interface that involves unsafe types}} -// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }} -// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{{1-1=@safe }} -func useUnsafeParam(x: Unannotated) { // expected-note{{reference to unsafe struct 'Unannotated'}} +func useUnsafeParam(x: Unannotated) { + // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}} + _ = x // expected-note{{reference to parameter 'x' involves unsafe type}} } -// expected-warning@+4{{global function 'useUnsafeParam2' has an interface that involves unsafe types}} -// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }} -// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{{1-1=@safe }} @available(SwiftStdlib 5.8, *) -func useUnsafeParam2(x: UnsafeReference) { // expected-note{{reference to unsafe class 'UnsafeReference'}} +func useUnsafeParam2(x: UnsafeReference) { + // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}} + _ = x // expected-note{{reference to parameter 'x' involves unsafe type}} } -// expected-warning@+3{{global function 'useUnsafeParam3' has an interface that involves unsafe types}} -// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }} -// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{{1-1=@safe }} -func useUnsafeParam3(x: UnknownEscapabilityAggregate) { // expected-note{{reference to unsafe struct 'UnknownEscapabilityAggregate'}} +func useUnsafeParam3(x: UnknownEscapabilityAggregate) { + // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}} + _ = x // expected-note{{reference to parameter 'x' involves unsafe type}} } func useSafeParams(x: Owner, y: View, z: SafeEscapableAggregate, c: MyContainer) { @@ -101,10 +98,9 @@ func useCfType(x: CFArray) { func useString(x: std.string) { } -// expected-warning@+3{{global function 'useVecOfPtr' has an interface}} -// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }} -// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{1-1=@safe }} -func useVecOfPtr(x: VecOfPtr) { // expected-note{{reference to unsafe type alias 'VecOfPtr'}} +func useVecOfPtr(x: VecOfPtr) { + // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}} + _ = x // expected-note{{reference to parameter 'x' involves unsafe type}} } func useVecOfInt(x: VecOfInt) { @@ -113,22 +109,19 @@ func useVecOfInt(x: VecOfInt) { func useSafeTuple(x: SafeTuple) { } -// expected-warning@+3{{global function 'useUnsafeTuple' has an interface that involves unsafe types}} -// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }} -// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{1-1=@safe }} -func useUnsafeTuple(x: UnsafeTuple) { // expected-note{{reference to unsafe type alias 'UnsafeTuple'}} +func useUnsafeTuple(x: UnsafeTuple) { + // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}} + _ = x // expected-note{{reference to parameter 'x' involves unsafe type}} } -// expected-warning@+3{{global function 'useCppSpan' has an interface that involves unsafe types}} -// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }} -// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{1-1=@safe }} -func useCppSpan(x: SpanOfInt) { // expected-note{{reference to unsafe type alias 'SpanOfInt'}} +func useCppSpan(x: SpanOfInt) { + // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}} + _ = x // expected-note{{reference to parameter 'x' involves unsafe type}} } -// expected-warning@+3{{global function 'useCppSpan2' has an interface that involves unsafe types}} -// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }} -// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{1-1=@safe }} -func useCppSpan2(x: SpanOfIntAlias) { // expected-note{{reference to unsafe type alias 'SpanOfIntAlias'}} +func useCppSpan2(x: SpanOfIntAlias) { + // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}} + _ = x // expected-note{{reference to parameter 'x' involves unsafe type}} } func useSafeLifetimeAnnotated(v: View) { diff --git a/test/Unsafe/Inputs/unsafe_decls.h b/test/Unsafe/Inputs/unsafe_decls.h index 1743019415c..21643b5d0ac 100644 --- a/test/Unsafe/Inputs/unsafe_decls.h +++ b/test/Unsafe/Inputs/unsafe_decls.h @@ -5,3 +5,62 @@ struct __attribute__((swift_attr("unsafe"))) UnsafeType { }; void print_ints(int *ptr, int count); + +#define _CXX_INTEROP_STRINGIFY(_x) #_x + +#define SWIFT_SHARED_REFERENCE(_retain, _release) \ + __attribute__((swift_attr("import_reference"))) \ + __attribute__((swift_attr(_CXX_INTEROP_STRINGIFY(retain:_retain)))) \ + __attribute__((swift_attr(_CXX_INTEROP_STRINGIFY(release:_release)))) + +#define SWIFT_SAFE __attribute__((swift_attr("@safe"))) +#define SWIFT_UNSAFE __attribute__((swift_attr("@unsafe"))) + +struct NoPointers { + float x, y, z; +}; + +union NoPointersUnion { + float x; + double y; +}; + +struct NoPointersUnsafe { + float x, y, z; +} SWIFT_UNSAFE; + +struct HasPointers { + float *numbers; +}; + + +union HasPointersUnion { + float *numbers; + double x; +}; + +struct HasPointersSafe { + float *numbers; +} SWIFT_SAFE; + +struct RefCountedType { + void *ptr; +} SWIFT_SHARED_REFERENCE(RCRetain, RCRelease); + +struct RefCountedType *RCRetain(struct RefCountedType *object); +void RCRelease(struct RefCountedType *object); + +struct HasRefCounted { + struct RefCountedType *ref; +}; + +struct ListNode { + double data; + struct ListNode *next; +}; + +enum SomeColor { + SCRed, + SCGreen, + SCBlue +}; diff --git a/test/Unsafe/safe.swift b/test/Unsafe/safe.swift index 31bca73c797..fb37433a264 100644 --- a/test/Unsafe/safe.swift +++ b/test/Unsafe/safe.swift @@ -17,10 +17,7 @@ func g() { unsafe unsafeFunction() } -// expected-warning@+3{{global function 'h' has an interface that involves unsafe types}} -// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }} -// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{{1-1=@safe }} -func h(_: UnsafeType) { // expected-note{{reference to unsafe struct 'UnsafeType'}} +func h(_: UnsafeType) { // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}} unsafeFunction() // expected-note{{reference to unsafe global function 'unsafeFunction()'}} @@ -31,16 +28,10 @@ func h(_: UnsafeType) { // expected-note{{reference to unsafe struct 'UnsafeType unsafe g() } -// expected-warning@+3 {{global function 'rethrowing' has an interface that involves unsafe types}} -// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }} -// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{1-1=@safe }} -func rethrowing(body: (UnsafeType) throws -> Void) rethrows { } // expected-note{{reference to unsafe struct 'UnsafeType'}} +func rethrowing(body: (UnsafeType) throws -> Void) rethrows { } class HasStatics { - // expected-warning@+3{{static method 'f' has an interface that involves unsafe types}} -// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{3-3=@unsafe }} -// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{3-3=@safe }} - static internal func f(_: UnsafeType) { } // expected-note{{reference to unsafe struct 'UnsafeType'}} + static internal func f(_: UnsafeType) { } } diff --git a/test/Unsafe/unsafe-suppression.swift b/test/Unsafe/unsafe-suppression.swift index 57e049aaed3..5c58d66f6e5 100644 --- a/test/Unsafe/unsafe-suppression.swift +++ b/test/Unsafe/unsafe-suppression.swift @@ -9,11 +9,7 @@ func iAmUnsafe() { } @unsafe struct UnsafeType { } -// expected-note@+3{{reference to unsafe struct 'UnsafeType'}} -// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }} -// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{1-1=@safe }} func iAmImpliedUnsafe() -> UnsafeType? { nil } -// expected-warning@-1{{global function 'iAmImpliedUnsafe' has an interface that involves unsafe types}} @unsafe func labeledUnsafe(_: UnsafeType) { diff --git a/test/Unsafe/unsafe.swift b/test/Unsafe/unsafe.swift index 5bc2c235ba0..06089d2e9aa 100644 --- a/test/Unsafe/unsafe.swift +++ b/test/Unsafe/unsafe.swift @@ -120,7 +120,7 @@ class ExclusivityChecking { func f() { } }; -// TODO: diagnose the need for @unsafe when there's an unsafe superclass. +// expected-warning@+1{{class 'UnsafeSub' has superclass involving unsafe type 'UnsafeSuper' [Unsafe]}}{{1-1=@unsafe }} class UnsafeSub: UnsafeSuper { } // ----------------------------------------------------------------------- @@ -150,12 +150,9 @@ func testRHS(b: Bool, x: Int) { @unsafe var unsafeVar: Int = 0 -// expected-warning@+3{{global function 'testMe' has an interface that involves unsafe types}} -// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }} -// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{1-1=@safe }} func testMe( - _ pointer: PointerType, // expected-note{{reference to unsafe struct 'PointerType'}} - _ unsafeSuper: UnsafeSuper // expected-note{{reference to unsafe class 'UnsafeSuper'}} + _ pointer: PointerType, + _ unsafeSuper: UnsafeSuper ) { // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}{{3-3=unsafe }} unsafeF() // expected-note{{reference to unsafe global function 'unsafeF()'}} @@ -173,19 +170,24 @@ func testMe( // Various declaration kinds // ----------------------------------------------------------------------- -// expected-warning@+3{{type alias 'SuperUnsafe' has an interface that involves unsafe types}} -// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }} -// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{1-1=@safe }} -typealias SuperUnsafe = UnsafeSuper // expected-note{{reference to unsafe class 'UnsafeSuper'}} +typealias SuperUnsafe = UnsafeSuper @unsafe typealias SuperUnsafe2 = UnsafeSuper +// expected-warning@+3{{enum 'HasUnsafeThings' has storage involving unsafe types [Unsafe]}} +// expected-note@+2{{add '@unsafe' if this type is also unsafe to use}}{{1-1=@unsafe }} +// expected-note@+1{{add '@safe' if this type encapsulates the unsafe storage in a safe interface}}{{1-1=@safe }} enum HasUnsafeThings { -// expected-warning@+3{{enum case 'one' has an interface that involves unsafe types}} -// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }} -// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{1-1=@safe }} -case one(UnsafeSuper) // expected-note{{reference to unsafe class 'UnsafeSuper'}} +case one(UnsafeSuper) // expected-note{{enum case 'one' involves unsafe type 'UnsafeSuper'}} -@unsafe case two(UnsafeSuper) +case two(UnsafeSuper) // expected-note{{enum case 'two' involves unsafe type 'UnsafeSuper'}} +} + +// expected-warning@+3{{class 'ClassWithUnsafeStorage' has storage involving unsafe types [Unsafe]}} +// expected-note@+2{{add '@unsafe' if this type is also unsafe to use}}{{1-1=@unsafe }} +// expected-note@+1{{add '@safe' if this type encapsulates the unsafe storage in a safe interface}}{{1-1=@safe }} +class ClassWithUnsafeStorage { + var int: Int = 0 + var array: [UnsafeSuper]? = nil // expected-note{{property 'array' involves unsafe type 'UnsafeSuper'}} } diff --git a/test/Unsafe/unsafe_c_imports.swift b/test/Unsafe/unsafe_c_imports.swift new file mode 100644 index 00000000000..abcec880681 --- /dev/null +++ b/test/Unsafe/unsafe_c_imports.swift @@ -0,0 +1,25 @@ +// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature WarnUnsafe -I %S/Inputs + +// REQUIRES: swift_feature_AllowUnsafeAttribute +// REQUIRES: swift_feature_WarnUnsafe + +import unsafe_decls + +// expected-warning@+3{{struct 'StoreAllTheThings' has storage involving unsafe types}} +// expected-note@+2{{add '@unsafe' if this type is also unsafe to use}} +// expected-note@+1{{add '@safe' if this type encapsulates the unsafe storage in a safe interface}} +struct StoreAllTheThings { + let np: NoPointers + let npu: NoPointersUnion + let npu2: NoPointersUnsafe // expected-note{{property 'npu2' involves unsafe type 'NoPointersUnsafe'}} + + let hp: HasPointers // expected-note{{property 'hp' involves unsafe type 'HasPointers'}} + let hpu: HasPointersUnion // expected-note{{property 'hpu' involves unsafe type 'HasPointersUnion'}} + let nps: HasPointersSafe + + let hrc: HasRefCounted + + let ln: ListNode // expected-note{{property 'ln' involves unsafe type 'ListNode'}} + + let sc: SomeColor +}; diff --git a/test/Unsafe/unsafe_imports.swift b/test/Unsafe/unsafe_imports.swift index f3eed53a5e2..e113e847529 100644 --- a/test/Unsafe/unsafe_imports.swift +++ b/test/Unsafe/unsafe_imports.swift @@ -9,10 +9,7 @@ import unsafe_decls import unsafe_swift_decls -// expected-warning@+3{{global function 'testUnsafe' has an interface that involves unsafe types}} -// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }} -// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{1-1=@safe }} -func testUnsafe(_ ut: UnsafeType) { // expected-note{{reference to unsafe struct 'UnsafeType'}} +func testUnsafe(_ ut: UnsafeType) { // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}}{{3-3=unsafe }} unsafe_c_function() // expected-note{{reference to unsafe global function 'unsafe_c_function()'}} @@ -25,11 +22,14 @@ func testUnsafe(_ ut: UnsafeType) { // expected-note{{reference to unsafe struct // Reference a typealias that isn't itself @unsafe, but refers to an unsafe // type. -// expected-warning@+3{{global function 'testUnsafeThroughAlias' has an interface that involves unsafe types}} -// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }} -// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{1-1=@safe }} -func testUnsafeThroughAlias(_ ut: UnsafeTypeAlias) { // expected-note{{reference to type alias 'UnsafeTypeAlias' involves unsafe type 'UnsafeTypeAlias' (aka 'PointerType')}} - // TODO: Diagnostic above could be better +func testUnsafeThroughAlias(_ ut: UnsafeTypeAlias) { + +} + +func callThroughAlias(ut: UnsafeTypeAlias) { + // expected-warning@+1{{expression uses unsafe constructs but is not marked with 'unsafe'}} + testUnsafeThroughAlias(ut) // expected-note{{reference to global function 'testUnsafeThroughAlias' involves unsafe type 'UnsafeTypeAlias' (aka 'PointerType')}} + // expected-note@-1{{reference to parameter 'ut' involves unsafe type 'UnsafeTypeAlias' (aka 'PointerType')}} } diff --git a/test/Unsafe/unsafe_stdlib.swift b/test/Unsafe/unsafe_stdlib.swift index 5c0d960f2d1..da7fe3d7d08 100644 --- a/test/Unsafe/unsafe_stdlib.swift +++ b/test/Unsafe/unsafe_stdlib.swift @@ -6,12 +6,9 @@ // REQUIRES: swift_feature_AllowUnsafeAttribute // REQUIRES: swift_feature_WarnUnsafe -// expected-warning@+3{{global function 'test' has an interface that involves unsafe types}} -// expected-note@+2{{add '@unsafe' to indicate that this declaration is unsafe to use}}{{1-1=@unsafe }} -// expected-note@+1{{add '@safe' to indicate that this declaration is memory-safe to use}}{1-1=@safe }} func test( - x: OpaquePointer, // expected-note{{reference to unsafe struct 'OpaquePointer'}} - other: UnsafeMutablePointer // expected-note{{reference to unsafe generic struct 'UnsafeMutablePointer'}} + x: OpaquePointer, + other: UnsafeMutablePointer ) { var array = [1, 2, 3] // expected-warning@+2{{expression uses unsafe constructs but is not marked with 'unsafe'}}{{3-3=unsafe }}