From cccf6c1114774a714341d1fe83429cb4f5f69648 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Mon, 22 Jul 2024 10:33:25 -0700 Subject: [PATCH 1/6] Introduce `@unsafe` and the ability to prohibit use of unsafe declarations Allow any declaration to be marked with `@unsafe`, meaning that it involves unsafe code. This also extends to C declarations marked with the `swift_attr("unsafe")` attribute. Under a separate experimental flag (`DisallowUnsafe`), diagnose any attempt to use an `@unsafe` declaration or any unsafe language feature (such as `unowned(unsafe)`, `@unchecked Sendable`). This begins to define a "safe" mode in Swift that prohibits memory-unsafe constructs. --- include/swift/AST/Decl.h | 39 +++++++++---- include/swift/AST/DeclAttr.def | 8 ++- include/swift/AST/DiagnosticsSema.def | 20 +++++++ include/swift/AST/TypeCheckRequests.h | 18 ++++++ include/swift/AST/TypeCheckerTypeIDZone.def | 3 + include/swift/Basic/Features.def | 7 +++ lib/AST/ASTPrinter.cpp | 9 +++ lib/AST/Decl.cpp | 7 +++ lib/AST/FeatureSet.cpp | 6 ++ lib/AST/TypeCheckRequests.cpp | 17 ++++++ lib/ASTGen/Sources/ASTGen/DeclAttrs.swift | 1 + lib/ClangImporter/ImportDecl.cpp | 8 +++ lib/Parse/ParseStmt.cpp | 2 - lib/Sema/TypeCheckAttr.cpp | 26 ++++++++- lib/Sema/TypeCheckAvailability.cpp | 21 +++++++ lib/Sema/TypeCheckDecl.cpp | 19 ++++++ lib/Sema/TypeCheckDeclOverride.cpp | 11 ++++ lib/Sema/TypeCheckProtocol.cpp | 16 ++++++ test/Unsafe/Inputs/module.modulemap | 3 + test/Unsafe/Inputs/unsafe_decls.h | 5 ++ test/Unsafe/interface_printing.swift | 10 ++++ test/Unsafe/unsafe.swift | 64 +++++++++++++++++++++ test/Unsafe/unsafe_concurrency.swift | 20 +++++++ test/Unsafe/unsafe_disallowed.swift | 5 ++ test/Unsafe/unsafe_imports.swift | 7 +++ 25 files changed, 334 insertions(+), 18 deletions(-) create mode 100644 test/Unsafe/Inputs/module.modulemap create mode 100644 test/Unsafe/Inputs/unsafe_decls.h create mode 100644 test/Unsafe/interface_printing.swift create mode 100644 test/Unsafe/unsafe.swift create mode 100644 test/Unsafe/unsafe_concurrency.swift create mode 100644 test/Unsafe/unsafe_disallowed.swift create mode 100644 test/Unsafe/unsafe_imports.swift diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index d4eb6ba0821a8..f13bbad95a87b 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -357,7 +357,7 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated { // for the inline bitfields. union { uint64_t OpaqueBits; - SWIFT_INLINE_BITFIELD_BASE(Decl, bitmax(NumDeclKindBits,8)+1+1+1+1+1+1+1+1+1+1+1, + SWIFT_INLINE_BITFIELD_BASE(Decl, bitmax(NumDeclKindBits,8)+1+1+1+1+1+1+1+1+1+1+1+1, Kind : bitmax(NumDeclKindBits,8), /// Whether this declaration is invalid. @@ -372,10 +372,6 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated { /// Use getClangNode() to retrieve the corresponding Clang AST. FromClang : 1, - /// Whether this declaration was added to the surrounding - /// DeclContext of an active #if config clause. - EscapedFromIfConfig : 1, - /// Whether this declaration is syntactically scoped inside of /// a local context, but should behave like a top-level /// declaration for name lookup purposes. This is used by @@ -404,7 +400,13 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated { /// True if we're in the common case where the SPIGroupsRequest /// request returned an empty array of identifiers. - NoSPIGroups : 1 + NoSPIGroups : 1, + + /// True if we have computed whether this declaration is unsafe. + IsUnsafeComputed : 1, + + /// True if this declaration has been determined to be "unsafe". + IsUnsafe : 1 ); SWIFT_INLINE_BITFIELD_FULL(PatternBindingDecl, Decl, 1+1+2+16, @@ -857,6 +859,7 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated { friend class ExpandPeerMacroRequest; friend class GlobalActorAttributeRequest; friend class SPIGroupsRequest; + friend class IsUnsafeRequest; private: llvm::PointerUnion Context; @@ -916,12 +919,13 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated { Bits.Decl.Invalid = false; Bits.Decl.Implicit = false; Bits.Decl.FromClang = false; - Bits.Decl.EscapedFromIfConfig = false; Bits.Decl.Hoisted = false; Bits.Decl.LacksObjCInterfaceOrImplementation = false; Bits.Decl.NoMemberAttributeMacros = false; Bits.Decl.NoGlobalActorAttribute = false; Bits.Decl.NoSPIGroups = false; + Bits.Decl.IsUnsafeComputed = false; + Bits.Decl.IsUnsafe = false; } /// Get the Clang node associated with this declaration. @@ -1180,15 +1184,26 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated { /// Whether this declaration predates the introduction of concurrency. bool preconcurrency() const; -public: - bool escapedFromIfConfig() const { - return Bits.Decl.EscapedFromIfConfig; + /// Whether this declaration is considered "unsafe", i.e., should not be + /// used in a "safe" dialect. + bool isUnsafe() const; + +private: + bool isUnsafeComputed() const { + return Bits.Decl.IsUnsafeComputed; + } + + bool isUnsafeRaw() const { + return Bits.Decl.IsUnsafe; } - void setEscapedFromIfConfig(bool Escaped) { - Bits.Decl.EscapedFromIfConfig = Escaped; + void setUnsafe(bool value) { + assert(!Bits.Decl.IsUnsafeComputed); + Bits.Decl.IsUnsafe = value; + Bits.Decl.IsUnsafeComputed = true; } +public: bool getSemanticAttrsComputed() const { return Bits.Decl.SemanticAttrsComputed; } diff --git a/include/swift/AST/DeclAttr.def b/include/swift/AST/DeclAttr.def index 49ff70582ac0a..5afb3e32b6fa7 100644 --- a/include/swift/AST/DeclAttr.def +++ b/include/swift/AST/DeclAttr.def @@ -500,7 +500,13 @@ SIMPLE_DECL_ATTR(sensitive, Sensitive, OnStruct | UserInaccessible | ABIStableToAdd | ABIStableToRemove | APIBreakingToAdd | APIStableToRemove, 159) -LAST_DECL_ATTR(PreInverseGenerics) +SIMPLE_DECL_ATTR(unsafe, Unsafe, + OnAbstractFunction | OnSubscript | OnVar | OnMacro | OnNominalType | + UserInaccessible | + ABIStableToAdd | ABIStableToRemove | APIBreakingToAdd | APIStableToRemove, + 160) + +LAST_DECL_ATTR(Unsafe) #undef DECL_ATTR_ALIAS #undef CONTEXTUAL_DECL_ATTR_ALIAS diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 68dda870240c8..148505bf44d64 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -7964,5 +7964,25 @@ NOTE(sending_function_result_with_sending_param_note, none, "isolation domain through a result of an invocation of value", ()) +//------------------------------------------------------------------------------ +// MARK: Strict Safety Diagnostics +//------------------------------------------------------------------------------ +ERROR(unsafe_attr_disabled,none, + "attribute requires '-enable-experimental-feature AllowUnsafeAttribute'", ()) +ERROR(override_safe_withunsafe,none, + "override of safe %0 with unsafe %0", (DescriptiveDeclKind)) +ERROR(witness_unsafe,none, + "unsafe %0 %1 cannot satisfy safe requirement", + (DescriptiveDeclKind, DeclName)) +ERROR(unchecked_conformance_is_unsafe,none, + "@unchecked conformance involves unsafe code", ()) +ERROR(unowned_unsafe_is_unsafe,none, + "unowned(unsafe) involves unsafe code", ()) +ERROR(nonisolated_unsafe_is_unsafe,none, + "nonisolated(unsafe) involves unsafe code", ()) +ERROR(reference_to_unsafe_decl,none, + "%select{reference|call}0 to unsafe %kindbase1", + (bool, const ValueDecl *)) + #define UNDEFINE_DIAGNOSTIC_MACROS #include "DefineDiagnosticMacros.h" diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h index 6dc1f36a50e5d..cc4f31aa05687 100644 --- a/include/swift/AST/TypeCheckRequests.h +++ b/include/swift/AST/TypeCheckRequests.h @@ -5065,6 +5065,24 @@ class SuppressesConformanceRequest bool isCached() const { return true; } }; +class IsUnsafeRequest + : public SimpleRequest { +public: + using SimpleRequest::SimpleRequest; + +private: + friend SimpleRequest; + + bool evaluate(Evaluator &evaluator, Decl *decl) const; + +public: + bool isCached() const { return true; } + std::optional getCachedResult() const; + void cacheResult(bool value) const; +}; + #define SWIFT_TYPEID_ZONE TypeChecker #define SWIFT_TYPEID_HEADER "swift/AST/TypeCheckerTypeIDZone.def" #include "swift/Basic/DefineTypeIDZone.h" diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def index 398d7c2f386db..6b8406b3e2af3 100644 --- a/include/swift/AST/TypeCheckerTypeIDZone.def +++ b/include/swift/AST/TypeCheckerTypeIDZone.def @@ -593,3 +593,6 @@ SWIFT_REQUEST(TypeChecker, CaptureInfoRequest, SWIFT_REQUEST(TypeChecker, ParamCaptureInfoRequest, CaptureInfo(ParamDecl *), SeparatelyCached, NoLocationInfo) +SWIFT_REQUEST(TypeChecker, IsUnsafeRequest, + bool(Decl *), + SeparatelyCached, NoLocationInfo) diff --git a/include/swift/Basic/Features.def b/include/swift/Basic/Features.def index b758de2ae4b79..abd590365d3f3 100644 --- a/include/swift/Basic/Features.def +++ b/include/swift/Basic/Features.def @@ -396,11 +396,18 @@ EXPERIMENTAL_FEATURE(ReinitializeConsumeInMultiBlockDefer, false) EXPERIMENTAL_FEATURE(SE427NoInferenceOnExtension, true) + EXPERIMENTAL_FEATURE(Extern, true) // Enable trailing comma for comma-separated lists. EXPERIMENTAL_FEATURE(TrailingComma, false) +/// Allow the @unsafe attribute. +SUPPRESSIBLE_EXPERIMENTAL_FEATURE(AllowUnsafeAttribute, true) + +/// Disallow use of unsafe code. +EXPERIMENTAL_FEATURE(DisallowUnsafe, true) + #undef EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE #undef EXPERIMENTAL_FEATURE #undef UPCOMING_FEATURE diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp index 4c3387262e03f..7b931b4b0b1f2 100644 --- a/lib/AST/ASTPrinter.cpp +++ b/lib/AST/ASTPrinter.cpp @@ -3056,6 +3056,15 @@ suppressingFeatureBitwiseCopyable2(PrintOptions &options, options.ExcludeAttrList.resize(originalExcludeAttrCount); } +static void +suppressingFeatureAllowUnsafeAttribute(PrintOptions &options, + llvm::function_ref action) { + unsigned originalExcludeAttrCount = options.ExcludeAttrList.size(); + options.ExcludeAttrList.push_back(DeclAttrKind::Unsafe); + action(); + options.ExcludeAttrList.resize(originalExcludeAttrCount); +} + /// Suppress the printing of a particular feature. static void suppressingFeature(PrintOptions &options, Feature feature, llvm::function_ref action) { diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index a68ff54b274db..6bb608cc2b8a7 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -1015,6 +1015,13 @@ bool Decl::preconcurrency() const { return false; } +bool Decl::isUnsafe() const { + return evaluateOrDefault( + getASTContext().evaluator, + IsUnsafeRequest{const_cast(this)}, + false); +} + Type AbstractFunctionDecl::getThrownInterfaceType() const { if (!getThrownTypeRepr()) return ThrownType.getType(); diff --git a/lib/AST/FeatureSet.cpp b/lib/AST/FeatureSet.cpp index 0965c3911ab26..6a32f0dada088 100644 --- a/lib/AST/FeatureSet.cpp +++ b/lib/AST/FeatureSet.cpp @@ -226,6 +226,12 @@ UNINTERESTING_FEATURE(ReinitializeConsumeInMultiBlockDefer) UNINTERESTING_FEATURE(SE427NoInferenceOnExtension) UNINTERESTING_FEATURE(TrailingComma) +static bool usesFeatureAllowUnsafeAttribute(Decl *decl) { + return decl->getAttrs().hasAttribute(); +} + +UNINTERESTING_FEATURE(DisallowUnsafe) + // ---------------------------------------------------------------------------- // MARK: - FeatureSet // ---------------------------------------------------------------------------- diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp index 9f996b6e193d7..4e5e366e5fa4d 100644 --- a/lib/AST/TypeCheckRequests.cpp +++ b/lib/AST/TypeCheckRequests.cpp @@ -2597,3 +2597,20 @@ void ParamCaptureInfoRequest::cacheResult(CaptureInfo info) const { auto *param = std::get<0>(getStorage()); param->setDefaultArgumentCaptureInfo(info); } + +//----------------------------------------------------------------------------// +// IsUnsafeRequest computation. +//----------------------------------------------------------------------------// + +std::optional IsUnsafeRequest::getCachedResult() const { + auto *decl = std::get<0>(getStorage()); + if (!decl->isUnsafeComputed()) + return std::nullopt; + + return decl->isUnsafeRaw(); +} + +void IsUnsafeRequest::cacheResult(bool value) const { + auto *decl = std::get<0>(getStorage()); + decl->setUnsafe(value); +} diff --git a/lib/ASTGen/Sources/ASTGen/DeclAttrs.swift b/lib/ASTGen/Sources/ASTGen/DeclAttrs.swift index eeab952eba627..d9c0c60eb7d5e 100644 --- a/lib/ASTGen/Sources/ASTGen/DeclAttrs.swift +++ b/lib/ASTGen/Sources/ASTGen/DeclAttrs.swift @@ -252,6 +252,7 @@ extension ASTGenVisitor { .testable, .transparent, .uiApplicationMain, + .unsafe, .unsafeInheritExecutor, .unsafeNoObjCTaggedPointer, .unsafeNonEscapableResult, diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index 9db5849e8739c..1cb6712af8058 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -8105,6 +8105,14 @@ ClangImporter::Implementation::importSwiftAttrAttributes(Decl *MappedDecl) { continue; } + if (swiftAttr->getAttribute() == "unsafe") { + if (!SwiftContext.LangOpts.hasFeature(Feature::AllowUnsafeAttribute)) + continue; + auto attr = new (SwiftContext) UnsafeAttr(/*implicit=*/false); + MappedDecl->getAttrs().add(attr); + continue; + } + // Dig out a buffer with the attribute text. unsigned bufferID = getClangSwiftAttrSourceBuffer( swiftAttr->getAttribute()); diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp index 49c821aded501..70d2e107a4be8 100644 --- a/lib/Parse/ParseStmt.cpp +++ b/lib/Parse/ParseStmt.cpp @@ -393,8 +393,6 @@ ParserStatus Parser::parseBraceItems(SmallVectorImpl &Entries, // Don't hoist nested '#if'. continue; Entries.push_back(Entry); - if (Entry.is()) - Entry.get()->setEscapedFromIfConfig(true); } } else { NeedParseErrorRecovery = true; diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index b89047317aed5..93bd73fbb93df 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -364,8 +364,8 @@ class AttributeChecker : public AttributeVisitor { void visitStaticExclusiveOnlyAttr(StaticExclusiveOnlyAttr *attr); void visitWeakLinkedAttr(WeakLinkedAttr *attr); - void visitSILGenNameAttr(SILGenNameAttr *attr); + void visitUnsafeAttr(UnsafeAttr *attr); }; } // end anonymous namespace @@ -4799,7 +4799,8 @@ void AttributeChecker::checkBackDeployedAttrs( Type TypeChecker::checkReferenceOwnershipAttr(VarDecl *var, Type type, ReferenceOwnershipAttr *attr) { - auto &Diags = var->getASTContext().Diags; + ASTContext &ctx = var->getASTContext(); + auto &Diags = ctx.Diags; auto *dc = var->getDeclContext(); // Don't check ownership attribute if the type is invalid. @@ -4888,7 +4889,7 @@ Type TypeChecker::checkReferenceOwnershipAttr(VarDecl *var, Type type, } // Embedded Swift prohibits weak/unowned but allows unowned(unsafe). - if (var->getASTContext().LangOpts.hasFeature(Feature::Embedded)) { + if (ctx.LangOpts.hasFeature(Feature::Embedded)) { if (ownershipKind == ReferenceOwnership::Weak || ownershipKind == ReferenceOwnership::Unowned) { Diags.diagnose(attr->getLocation(), diag::weak_unowned_in_embedded_swift, @@ -4897,6 +4898,12 @@ Type TypeChecker::checkReferenceOwnershipAttr(VarDecl *var, Type type, } } + // unowned(unsafe) is unsafe (duh). + if (ownershipKind == ReferenceOwnership::Unmanaged && + ctx.LangOpts.hasFeature(Feature::DisallowUnsafe)) { + Diags.diagnose(attr->getLocation(), diag::unowned_unsafe_is_unsafe); + } + if (attr->isInvalid()) return type; @@ -7091,6 +7098,12 @@ void AttributeChecker::visitNonisolatedAttr(NonisolatedAttr *attr) { // that do not have storage. auto dc = D->getDeclContext(); + // nonisolated(unsafe) is unsafe, but only under strict concurrency. + if (attr->isUnsafe() && + Ctx.LangOpts.hasFeature(Feature::DisallowUnsafe) && + Ctx.LangOpts.StrictConcurrencyLevel == StrictConcurrency::Complete) + Ctx.Diags.diagnose(attr->getLocation(), diag::nonisolated_unsafe_is_unsafe); + if (auto var = dyn_cast(D)) { // stored properties have limitations as to when they can be nonisolated. auto type = var->getTypeInContext(); @@ -7634,6 +7647,13 @@ void AttributeChecker::visitWeakLinkedAttr(WeakLinkedAttr *attr) { attr->getAttrName(), Ctx.LangOpts.Target.str()); } +void AttributeChecker::visitUnsafeAttr(UnsafeAttr *attr) { + if (Ctx.LangOpts.hasFeature(Feature::AllowUnsafeAttribute)) + return; + + diagnoseAndRemoveAttr(attr, diag::unsafe_attr_disabled); +} + namespace { class ClosureAttributeChecker diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index 4516942d30107..cea6aaf7c957b 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -3943,6 +3943,24 @@ diagnoseDeclAsyncAvailability(const ValueDecl *D, SourceRange R, return true; } +/// Diagnose uses of unsafe declarations. +static bool +diagnoseDeclUnsafe(const ValueDecl *D, SourceRange R, + const Expr *call, const ExportContext &Where) { + ASTContext &ctx = D->getASTContext(); + if (!ctx.LangOpts.hasFeature(Feature::DisallowUnsafe)) + return false; + + if (!D->isUnsafe()) + return false; + + SourceLoc diagLoc = call ? call->getLoc() : R.Start; + ctx.Diags + .diagnose(diagLoc, diag::reference_to_unsafe_decl, call != nullptr, D); + D->diagnose(diag::decl_declared_here, D); + return true; +} + /// Diagnose uses of unavailable declarations. Returns true if a diagnostic /// was emitted. bool swift::diagnoseDeclAvailability(const ValueDecl *D, SourceRange R, @@ -3979,6 +3997,9 @@ bool swift::diagnoseDeclAvailability(const ValueDecl *D, SourceRange R, if (diagnoseDeclAsyncAvailability(D, R, call, Where)) return true; + if (diagnoseDeclUnsafe(D, R, call, Where)) + return true; + // Make sure not to diagnose an accessor's deprecation if we already // complained about the property/subscript. bool isAccessorWithDeprecatedStorage = diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index 8b3cab5cbfc29..b2a22fb8d4b7b 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -3193,3 +3193,22 @@ SourceFile::getIfConfigClausesWithin(SourceRange outer) const { }); return llvm::ArrayRef(lower, upper - lower); } + +//----------------------------------------------------------------------------// +// IsUnsafeRequest +//----------------------------------------------------------------------------// + +bool IsUnsafeRequest::evaluate(Evaluator &evaluator, Decl *decl) const { + // If it's marked @unsafe, it's unsafe. + if (decl->getAttrs().hasAttribute()) + return true; + + // Inference: A member of an @unsafe type is also unsafe. + if (auto enclosingDC = decl->getDeclContext()) { + if (auto enclosingNominal = enclosingDC->getSelfNominalTypeDecl()) + if (enclosingNominal->isUnsafe()) + return true; + } + + return false; +} diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp index 5e7d6cabad8aa..3cf598f34abbe 100644 --- a/lib/Sema/TypeCheckDeclOverride.cpp +++ b/lib/Sema/TypeCheckDeclOverride.cpp @@ -1731,6 +1731,17 @@ namespace { } void visitObjCAttr(ObjCAttr *attr) {} + + void visitUnsafeAttr(UnsafeAttr *attr) { + if (!Base->getASTContext().LangOpts.hasFeature(Feature::DisallowUnsafe)) + return; + + if (Override->isUnsafe() && !Base->isUnsafe()) { + Diags.diagnose(Override, diag::override_safe_withunsafe, + Base->getDescriptiveKind()); + Diags.diagnose(Base, diag::overridden_here); + } + } }; } // end anonymous namespace diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index 6463065318c50..3ad8422e251b1 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -2428,6 +2428,13 @@ checkIndividualConformance(NormalProtocolConformance *conformance) { ComplainLoc, diag::unchecked_conformance_not_special, ProtoType); } + // @unchecked conformances are considered unsafe in strict concurrency mode. + if (conformance->isUnchecked() && + Context.LangOpts.hasFeature(Feature::DisallowUnsafe) && + Context.LangOpts.StrictConcurrencyLevel == StrictConcurrency::Complete) { + Context.Diags.diagnose(ComplainLoc, diag::unchecked_conformance_is_unsafe); + } + bool allowImpliedConditionalConformance = false; if (Proto->isSpecificProtocol(KnownProtocolKind::Sendable)) { // In -swift-version 5 mode, a conditional conformance to a protocol can imply @@ -5023,6 +5030,15 @@ void ConformanceChecker::resolveValueWitnesses() { .withEnterIsolation(*enteringIsolation)); } + // If we're disallowing unsafe code, check for an unsafe witness to a + // safe requirement. + if (C.LangOpts.hasFeature(Feature::DisallowUnsafe) && + witness->isUnsafe() && !requirement->isUnsafe()) { + witness->diagnose(diag::witness_unsafe, + witness->getDescriptiveKind(), + witness->getName()); + } + // Objective-C checking for @objc requirements. if (requirement->isObjC() && requirement->getName() == witness->getName() && diff --git a/test/Unsafe/Inputs/module.modulemap b/test/Unsafe/Inputs/module.modulemap new file mode 100644 index 0000000000000..e788869adea65 --- /dev/null +++ b/test/Unsafe/Inputs/module.modulemap @@ -0,0 +1,3 @@ +module unsafe_decls { + header "unsafe_decls.h" +} \ No newline at end of file diff --git a/test/Unsafe/Inputs/unsafe_decls.h b/test/Unsafe/Inputs/unsafe_decls.h new file mode 100644 index 0000000000000..751f569e186eb --- /dev/null +++ b/test/Unsafe/Inputs/unsafe_decls.h @@ -0,0 +1,5 @@ +void unsafe_c_function(void) __attribute__((swift_attr("unsafe"))); + +struct __attribute__((swift_attr("unsafe"))) UnsafeType { + int field; +}; diff --git a/test/Unsafe/interface_printing.swift b/test/Unsafe/interface_printing.swift new file mode 100644 index 0000000000000..a2255be358eb2 --- /dev/null +++ b/test/Unsafe/interface_printing.swift @@ -0,0 +1,10 @@ +// RUN: %empty-directory(%t) + +// RUN: %target-swift-frontend -swift-version 5 -enable-library-evolution -module-name unsafe -emit-module -o %t/unsafe.swiftmodule -emit-module-interface-path - %s -enable-experimental-feature AllowUnsafeAttribute | %FileCheck %s + +// CHECK: #if compiler(>=5.3) && $AllowUnsafeAttribute +// CHECK: @unsafe public func testFunction() +// CHECK: #else +// CHECK: public func testFunction() +// CHECK: #endif +@unsafe public func testFunction() { } diff --git a/test/Unsafe/unsafe.swift b/test/Unsafe/unsafe.swift new file mode 100644 index 0000000000000..27e873db87a08 --- /dev/null +++ b/test/Unsafe/unsafe.swift @@ -0,0 +1,64 @@ +// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature DisallowUnsafe + +// Make sure everything compiles without error when unsafe code is allowed. +// RUN: %target-swift-frontend -typecheck -enable-experimental-feature AllowUnsafeAttribute %s + +// ----------------------------------------------------------------------- +// Witness matching +// ----------------------------------------------------------------------- +protocol P { + func f() + @unsafe func g() +} + +struct XP: P { + @unsafe func f() { } // expected-error{{unsafe instance method 'f()' cannot satisfy safe requirement}} + @unsafe func g() { } +} + +// ----------------------------------------------------------------------- +// Overrides +// ----------------------------------------------------------------------- +class Super { + func f() { } + @unsafe func g() { } +} + +class Sub: Super { + @unsafe override func f() { } + @unsafe override func g() { } +} + +// ----------------------------------------------------------------------- +// Owned pointers +// ----------------------------------------------------------------------- +struct SuperHolder { + unowned var s1: Super + unowned(unsafe) var s2: Super // expected-error{{unowned(unsafe) involves unsafe code}} +} + +// ----------------------------------------------------------------------- +// Inheritance of @unsafe +// ----------------------------------------------------------------------- +@unsafe class UnsafeSuper { // expected-note 2{{'UnsafeSuper' declared here}} + func f() { } // expected-note{{'f()' declared here}} +}; + +class UnsafeSub: UnsafeSuper { } // expected-error{{reference to unsafe class 'UnsafeSuper'}} + +// ----------------------------------------------------------------------- +// Declaration references +// ----------------------------------------------------------------------- +@unsafe func unsafeF() { } // expected-note{{'unsafeF()' declared here}} +@unsafe var unsafeVar: Int = 0 // expected-note{{'unsafeVar' declared here}} + +@unsafe struct PointerType { } // expected-note{{'PointerType' declared here}} + +func testMe( + _ pointer: PointerType, // expected-error{{reference to unsafe struct 'PointerType'}} + _ unsafeSuper: UnsafeSuper // expected-error{{reference to unsafe class 'UnsafeSuper'}} +) { + unsafeF() // expected-error{{call to unsafe global function 'unsafeF'}} + _ = unsafeVar // expected-error{{reference to unsafe var 'unsafeVar'}} + unsafeSuper.f() // expected-error{{call to unsafe instance method 'f'}} +} diff --git a/test/Unsafe/unsafe_concurrency.swift b/test/Unsafe/unsafe_concurrency.swift new file mode 100644 index 0000000000000..823394fb3c16c --- /dev/null +++ b/test/Unsafe/unsafe_concurrency.swift @@ -0,0 +1,20 @@ +// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature DisallowUnsafe -enable-experimental-feature StrictConcurrency + +// REQUIRES: concurrency + +// expected-error@+1{{@unchecked conformance involves unsafe code}} +class C: @unchecked Sendable { + var counter: Int = 0 +} + +@available(SwiftStdlib 5.1, *) +func f() async { + // expected-error@+1{{nonisolated(unsafe) involves unsafe code}} + nonisolated(unsafe) var counter = 0 + Task.detached { + counter += 1 + } + counter += 1 + print(counter) +} + diff --git a/test/Unsafe/unsafe_disallowed.swift b/test/Unsafe/unsafe_disallowed.swift new file mode 100644 index 0000000000000..3874de05a11e0 --- /dev/null +++ b/test/Unsafe/unsafe_disallowed.swift @@ -0,0 +1,5 @@ +// RUN: %target-typecheck-verify-swift + +@unsafe func f() { } +// expected-error@-1{{attribute requires '-enable-experimental-feature AllowUnsafeAttribute'}} + diff --git a/test/Unsafe/unsafe_imports.swift b/test/Unsafe/unsafe_imports.swift new file mode 100644 index 0000000000000..1eb3e61cffca4 --- /dev/null +++ b/test/Unsafe/unsafe_imports.swift @@ -0,0 +1,7 @@ +// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature DisallowUnsafe -I %S/Inputs + +import unsafe_decls + +func testUnsafe(_ ut: UnsafeType) { // expected-error{{reference to unsafe struct 'UnsafeType'}} + unsafe_c_function() // expected-error{{call to unsafe global function 'unsafe_c_function'}} +} From 39f1e97bf99e9cd4256dd5a512e05f2537a65bcd Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Mon, 22 Jul 2024 17:46:33 -0700 Subject: [PATCH 2/6] [Safe mode] Diagnose uses of declarations involving unsafe types When referencing a declaration, check whether any of the types in that reference are unsafe. This can diagnose cases where the original declaration either wasn't actually unsafe, or is being provided with unsafe types via generics. --- include/swift/AST/DiagnosticsSema.def | 8 ++++ include/swift/AST/Types.h | 22 ++++++--- lib/AST/ASTContext.cpp | 35 +++++++++++++-- lib/AST/Type.cpp | 49 ++++++++++++++++----- lib/Sema/AssociatedTypeInference.cpp | 17 +++++++ lib/Sema/TypeCheckAvailability.cpp | 22 +++++++++ lib/Sema/TypeCheckType.cpp | 32 ++++++++++++++ lib/Sema/TypeCheckType.h | 8 ++++ test/Unsafe/Inputs/unsafe_swift_decls.swift | 13 ++++++ test/Unsafe/unsafe.swift | 27 +++++++++--- 10 files changed, 209 insertions(+), 24 deletions(-) create mode 100644 test/Unsafe/Inputs/unsafe_swift_decls.swift diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 148505bf44d64..2fa99782f7c88 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -7974,6 +7974,9 @@ ERROR(override_safe_withunsafe,none, ERROR(witness_unsafe,none, "unsafe %0 %1 cannot satisfy safe requirement", (DescriptiveDeclKind, DeclName)) +ERROR(type_witness_unsafe,none, + "unsafe type %0 cannot satisfy safe associated type %1", + (Type, DeclName)) ERROR(unchecked_conformance_is_unsafe,none, "@unchecked conformance involves unsafe code", ()) ERROR(unowned_unsafe_is_unsafe,none, @@ -7983,6 +7986,11 @@ ERROR(nonisolated_unsafe_is_unsafe,none, ERROR(reference_to_unsafe_decl,none, "%select{reference|call}0 to unsafe %kindbase1", (bool, const ValueDecl *)) +ERROR(reference_to_unsafe_typed_decl,none, + "%select{reference|call}0 to %kindbase1 involves unsafe type %2", + (bool, const ValueDecl *, Type)) +NOTE(unsafe_decl_here,none, + "unsafe %kindbase0 declared here", (const ValueDecl *)) #define UNDEFINE_DIAGNOSTIC_MACROS #include "DefineDiagnosticMacros.h" diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h index 4677bdf6f239f..e4c970bac80e3 100644 --- a/include/swift/AST/Types.h +++ b/include/swift/AST/Types.h @@ -185,7 +185,10 @@ class RecursiveTypeProperties { /// Contains a PackArchetypeType. Also implies HasPrimaryArchetype. HasPackArchetype = 0x20000, - Last_Property = HasPackArchetype + /// Whether this type contains an unsafe type. + IsUnsafe = 0x040000, + + Last_Property = IsUnsafe }; enum { BitWidth = countBitsUsed(Property::Last_Property) }; @@ -266,6 +269,8 @@ class RecursiveTypeProperties { bool hasPackArchetype() const { return Bits & HasPackArchetype; } + bool isUnsafe() const { return Bits & IsUnsafe; } + /// Does a type with these properties structurally contain a /// parameterized existential type? bool hasParameterizedExistential() const { @@ -431,12 +436,12 @@ class alignas(1 << TypeAlignInBits) TypeBase NumProtocols : 16 ); - SWIFT_INLINE_BITFIELD_FULL(TypeVariableType, TypeBase, 7+29, + SWIFT_INLINE_BITFIELD_FULL(TypeVariableType, TypeBase, 7+28, /// Type variable options. Options : 7, : NumPadBits, /// The unique number assigned to this type variable. - ID : 29 + ID : 28 ); SWIFT_INLINE_BITFIELD_FULL(ErrorUnionType, TypeBase, 32, @@ -709,6 +714,11 @@ class alignas(1 << TypeAlignInBits) TypeBase return getRecursiveProperties().hasPackArchetype(); } + /// Whether the type contains an @unsafe type in it anywhere. + bool isUnsafe() const { + return getRecursiveProperties().isUnsafe(); + } + /// Determine whether the type involves a primary, pack or local archetype. /// /// FIXME: Replace all remaining callers with a more precise check. @@ -6637,7 +6647,8 @@ class PrimaryArchetypeType final : public ArchetypeType, GenericEnvironment *GenericEnv, Type InterfaceType, ArrayRef ConformsTo, - Type Superclass, LayoutConstraint Layout); + Type Superclass, LayoutConstraint Layout, + RecursiveTypeProperties Properties); }; BEGIN_CAN_TYPE_WRAPPER(PrimaryArchetypeType, ArchetypeType) END_CAN_TYPE_WRAPPER(PrimaryArchetypeType, ArchetypeType) @@ -6918,7 +6929,8 @@ class PackArchetypeType final private: PackArchetypeType(const ASTContext &Ctx, GenericEnvironment *GenericEnv, Type InterfaceType, ArrayRef ConformsTo, - Type Superclass, LayoutConstraint Layout, PackShape Shape); + Type Superclass, LayoutConstraint Layout, PackShape Shape, + RecursiveTypeProperties properties); }; BEGIN_CAN_TYPE_WRAPPER(PackArchetypeType, ArchetypeType) END_CAN_TYPE_WRAPPER(PackArchetypeType, ArchetypeType) diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index c091fbea9b606..80c3f3206f437 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -3897,7 +3897,9 @@ get(GenericTypeDecl *TheDecl, Type Parent, const ASTContext &C) { UnboundGenericType::Profile(ID, TheDecl, Parent); void *InsertPos = nullptr; RecursiveTypeProperties properties; + if (TheDecl->isUnsafe()) properties |= RecursiveTypeProperties::IsUnsafe; if (Parent) properties |= Parent->getRecursiveProperties(); + auto arena = getArena(properties); if (auto unbound = C.getImpl().getArena(arena).UnboundGenericTypes @@ -3948,6 +3950,7 @@ BoundGenericType *BoundGenericType::get(NominalTypeDecl *TheDecl, llvm::FoldingSetNodeID ID; BoundGenericType::Profile(ID, TheDecl, Parent, GenericArgs); RecursiveTypeProperties properties; + if (TheDecl->isUnsafe()) properties |= RecursiveTypeProperties::IsUnsafe; if (Parent) properties |= Parent->getRecursiveProperties(); for (Type Arg : GenericArgs) { properties |= Arg->getRecursiveProperties(); @@ -4029,6 +4032,7 @@ EnumType::EnumType(EnumDecl *TheDecl, Type Parent, const ASTContext &C, EnumType *EnumType::get(EnumDecl *D, Type Parent, const ASTContext &C) { RecursiveTypeProperties properties; + if (D->isUnsafe()) properties |= RecursiveTypeProperties::IsUnsafe; if (Parent) properties |= Parent->getRecursiveProperties(); auto arena = getArena(properties); @@ -4045,6 +4049,7 @@ StructType::StructType(StructDecl *TheDecl, Type Parent, const ASTContext &C, StructType *StructType::get(StructDecl *D, Type Parent, const ASTContext &C) { RecursiveTypeProperties properties; + if (D->isUnsafe()) properties |= RecursiveTypeProperties::IsUnsafe; if (Parent) properties |= Parent->getRecursiveProperties(); auto arena = getArena(properties); @@ -4061,6 +4066,7 @@ ClassType::ClassType(ClassDecl *TheDecl, Type Parent, const ASTContext &C, ClassType *ClassType::get(ClassDecl *D, Type Parent, const ASTContext &C) { RecursiveTypeProperties properties; + if (D->isUnsafe()) properties |= RecursiveTypeProperties::IsUnsafe; if (Parent) properties |= Parent->getRecursiveProperties(); auto arena = getArena(properties); @@ -4325,20 +4331,39 @@ isAnyFunctionTypeCanonical(ArrayRef params, // always materializable. static RecursiveTypeProperties getGenericFunctionRecursiveProperties(ArrayRef params, - Type result) { - static_assert(RecursiveTypeProperties::BitWidth == 18, + Type result, Type globalActor, + Type thrownError) { + static_assert(RecursiveTypeProperties::BitWidth == 19, "revisit this if you add new recursive type properties"); RecursiveTypeProperties properties; for (auto param : params) { if (param.getPlainType()->getRecursiveProperties().hasError()) properties |= RecursiveTypeProperties::HasError; + if (param.getPlainType()->getRecursiveProperties().isUnsafe()) + properties |= RecursiveTypeProperties::IsUnsafe; } if (result->getRecursiveProperties().hasDynamicSelf()) properties |= RecursiveTypeProperties::HasDynamicSelf; if (result->getRecursiveProperties().hasError()) properties |= RecursiveTypeProperties::HasError; + if (result->getRecursiveProperties().isUnsafe()) + properties |= RecursiveTypeProperties::IsUnsafe; + + if (globalActor) { + if (globalActor->getRecursiveProperties().hasError()) + properties |= RecursiveTypeProperties::HasError; + if (globalActor->getRecursiveProperties().isUnsafe()) + properties |= RecursiveTypeProperties::IsUnsafe; + } + + if (thrownError) { + if (thrownError->getRecursiveProperties().hasError()) + properties |= RecursiveTypeProperties::HasError; + if (thrownError->getRecursiveProperties().isUnsafe()) + properties |= RecursiveTypeProperties::IsUnsafe; + } return properties; } @@ -4671,7 +4696,8 @@ GenericFunctionType *GenericFunctionType::get(GenericSignature sig, hasLifetimeDependenceInfo ? numLifetimeDependencies : 0); void *mem = ctx.Allocate(allocSize, alignof(GenericFunctionType)); - auto properties = getGenericFunctionRecursiveProperties(params, result); + auto properties = getGenericFunctionRecursiveProperties( + params, result, globalActor, thrownError); auto funcTy = new (mem) GenericFunctionType(sig, params, result, info, isCanonical ? &ctx : nullptr, properties); @@ -5044,7 +5070,7 @@ CanSILFunctionType SILFunctionType::get( void *mem = ctx.Allocate(bytes, alignof(SILFunctionType)); RecursiveTypeProperties properties; - static_assert(RecursiveTypeProperties::BitWidth == 18, + static_assert(RecursiveTypeProperties::BitWidth == 19, "revisit this if you add new recursive type properties"); for (auto ¶m : params) properties |= param.getInterfaceType()->getRecursiveProperties(); @@ -5132,6 +5158,7 @@ OptionalType *OptionalType::get(Type base) { ProtocolType *ProtocolType::get(ProtocolDecl *D, Type Parent, const ASTContext &C) { RecursiveTypeProperties properties; + if (D->isUnsafe()) properties |= RecursiveTypeProperties::IsUnsafe; if (Parent) properties |= Parent->getRecursiveProperties(); auto arena = getArena(properties); diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index 4da2d40182aeb..7d16017d03b9d 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -3489,13 +3489,31 @@ std::string ArchetypeType::getFullName() const { return InterfaceType.getString(); } +/// Determine the recursive type properties for an archetype. +static RecursiveTypeProperties archetypeProperties( + RecursiveTypeProperties properties, + ArrayRef conformsTo, + Type superclass +) { + for (auto proto : conformsTo) { + if (proto->isUnsafe()) { + properties |= RecursiveTypeProperties::IsUnsafe; + break; + } + } + + if (superclass) properties |= superclass->getRecursiveProperties(); + + return properties; +} + PrimaryArchetypeType::PrimaryArchetypeType(const ASTContext &Ctx, GenericEnvironment *GenericEnv, Type InterfaceType, ArrayRef ConformsTo, - Type Superclass, LayoutConstraint Layout) - : ArchetypeType(TypeKind::PrimaryArchetype, Ctx, - RecursiveTypeProperties::HasPrimaryArchetype, + Type Superclass, LayoutConstraint Layout, + RecursiveTypeProperties Properties) + : ArchetypeType(TypeKind::PrimaryArchetype, Ctx, Properties, InterfaceType, ConformsTo, Superclass, Layout, GenericEnv) { assert(!InterfaceType->isParameterPack()); @@ -3514,6 +3532,11 @@ PrimaryArchetypeType::getNew(const ASTContext &Ctx, // Gather the set of protocol declarations to which this archetype conforms. ProtocolType::canonicalizeProtocols(ConformsTo); + RecursiveTypeProperties Properties = archetypeProperties( + RecursiveTypeProperties::HasPrimaryArchetype, + ConformsTo, Superclass); + assert(!Properties.hasTypeVariable()); + auto arena = AllocationArena::Permanent; void *mem = Ctx.Allocate( PrimaryArchetypeType::totalSizeToAlloc( @@ -3521,7 +3544,8 @@ PrimaryArchetypeType::getNew(const ASTContext &Ctx, alignof(PrimaryArchetypeType), arena); return CanPrimaryArchetypeType(::new (mem) PrimaryArchetypeType( - Ctx, GenericEnv, InterfaceType, ConformsTo, Superclass, Layout)); + Ctx, GenericEnv, InterfaceType, ConformsTo, Superclass, Layout, + Properties)); } OpaqueTypeArchetypeType::OpaqueTypeArchetypeType( @@ -3573,11 +3597,10 @@ UUID OpenedArchetypeType::getOpenedExistentialID() const { PackArchetypeType::PackArchetypeType( const ASTContext &Ctx, GenericEnvironment *GenericEnv, Type InterfaceType, ArrayRef ConformsTo, Type Superclass, - LayoutConstraint Layout, PackShape Shape) - : ArchetypeType(TypeKind::PackArchetype, Ctx, - RecursiveTypeProperties::HasPrimaryArchetype | - RecursiveTypeProperties::HasPackArchetype, - InterfaceType, ConformsTo, Superclass, Layout, GenericEnv) { + LayoutConstraint Layout, PackShape Shape, + RecursiveTypeProperties Properties +) : ArchetypeType(TypeKind::PackArchetype, Ctx, Properties, + InterfaceType, ConformsTo, Superclass, Layout, GenericEnv) { assert(InterfaceType->isParameterPack()); *getTrailingObjects() = Shape; } @@ -3594,6 +3617,12 @@ PackArchetypeType::get(const ASTContext &Ctx, // Gather the set of protocol declarations to which this archetype conforms. ProtocolType::canonicalizeProtocols(ConformsTo); + RecursiveTypeProperties properties = archetypeProperties( + (RecursiveTypeProperties::HasPrimaryArchetype | + RecursiveTypeProperties::HasPackArchetype), + ConformsTo, Superclass); + assert(!properties.hasTypeVariable()); + auto arena = AllocationArena::Permanent; void *mem = Ctx.Allocate(PackArchetypeType::totalSizeToAllocisUnsafe() && type->isUnsafe()) { + SourceLoc loc = typeDecl->getLoc(); + if (loc.isInvalid()) + loc = conformance->getLoc(); + diagnoseUnsafeType(ctx, + loc, + type, + [&](Type specificType) { + ctx.Diags.diagnose( + loc, diag::type_witness_unsafe, specificType, assocType->getName()); + assocType->diagnose(diag::decl_declared_here, assocType); + }); + } + // Record the type witness. conformance->setTypeWitness(assocType, type, typeDecl); diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index cea6aaf7c957b..7f3ce46d947cf 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -18,6 +18,7 @@ #include "MiscDiagnostics.h" #include "TypeCheckConcurrency.h" #include "TypeCheckObjC.h" +#include "TypeCheckType.h" #include "TypeChecker.h" #include "swift/AST/ASTWalker.h" #include "swift/AST/ClangModuleLoader.h" @@ -3882,6 +3883,27 @@ bool ExprAvailabilityWalker::diagnoseDeclRefAvailability( if (diagnoseDeclAvailability(D, R, call, Where, Flags)) return true; + // If the declaration itself is "safe" but we don't disallow unsafe uses, + // check whether it traffics in unsafe types. + ASTContext &ctx = D->getASTContext(); + if (ctx.LangOpts.hasFeature(Feature::DisallowUnsafe) && !D->isUnsafe()) { + auto type = D->getInterfaceType(); + if (auto subs = declRef.getSubstitutions()) + type = type.subst(subs); + if (type->isUnsafe()) { + if (diagnoseUnsafeType( + ctx, R.Start, type, + [&](Type specificType) { + ctx.Diags.diagnose( + R.Start, diag::reference_to_unsafe_typed_decl, + call != nullptr && !isa(D), D, + specificType); + D->diagnose(diag::decl_declared_here, D); + })) + return true; + } + } + if (R.isValid()) { if (diagnoseSubstitutionMapAvailability(R.Start, declRef.getSubstitutions(), Where)) { diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index 3f1e84a691160..eb836a1f19c59 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -6421,3 +6421,35 @@ Type ExplicitCaughtTypeRequest::evaluate( llvm_unreachable("Unhandled catch node"); } + +bool swift::diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type, + llvm::function_ref diagnose) { + if (!ctx.LangOpts.hasFeature(Feature::DisallowUnsafe)) + return false; + + if (!type->isUnsafe()) + return false; + + // Look for a specific @unsafe nominal type. + Type specificType; + type.findIf([&specificType](Type type) { + if (auto typeDecl = type->getAnyNominal()) { + if (typeDecl->isUnsafe()) { + specificType = type; + return true; + } + } + + return false; + }); + + diagnose(specificType ? specificType : type); + + if (specificType) { + if (auto specificTypeDecl = specificType->getAnyNominal()) { + specificTypeDecl->diagnose(diag::unsafe_decl_here, specificTypeDecl); + } + } + + return true; +} diff --git a/lib/Sema/TypeCheckType.h b/lib/Sema/TypeCheckType.h index 40415a0614b56..bc3f02a4f76bc 100644 --- a/lib/Sema/TypeCheckType.h +++ b/lib/Sema/TypeCheckType.h @@ -686,6 +686,14 @@ bool diagnoseMissingOwnership(ParamSpecifier ownership, TypeRepr *repr, Type ty, const TypeResolution &resolution); +/// If the given type involves an unsafe type, diagnose it by calling the +/// diagnose function with the most specific unsafe type that can be provided. +/// +/// \returns \c true if the type was unsafe and we are diagnosing unsafe types +/// here. +bool diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type, + llvm::function_ref diagnose); + } // end namespace swift #endif /* SWIFT_SEMA_TYPE_CHECK_TYPE_H */ diff --git a/test/Unsafe/Inputs/unsafe_swift_decls.swift b/test/Unsafe/Inputs/unsafe_swift_decls.swift new file mode 100644 index 0000000000000..7c92bd6203f9e --- /dev/null +++ b/test/Unsafe/Inputs/unsafe_swift_decls.swift @@ -0,0 +1,13 @@ +@unsafe public struct PointerType { } // expected-note{{'PointerType' declared here}} + +public func getPointers() -> [PointerType] { [] } + +public struct HasAPointerType { + public typealias Ptr = PointerType +} + +public protocol Ptrable { + associatedtype Ptr +} + +extension HasAPointerType: Ptrable { } diff --git a/test/Unsafe/unsafe.swift b/test/Unsafe/unsafe.swift index 27e873db87a08..11bc90ce0c2a5 100644 --- a/test/Unsafe/unsafe.swift +++ b/test/Unsafe/unsafe.swift @@ -1,7 +1,12 @@ -// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature DisallowUnsafe +// RUN: %empty-directory(%t) +// RUN: %target-swift-frontend -emit-module-path %t/unsafe_swift_decls.swiftmodule %S/Inputs/unsafe_swift_decls.swift -enable-experimental-feature AllowUnsafeAttribute + +// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature DisallowUnsafe -I %t // Make sure everything compiles without error when unsafe code is allowed. -// RUN: %target-swift-frontend -typecheck -enable-experimental-feature AllowUnsafeAttribute %s +// RUN: %target-swift-frontend -typecheck -enable-experimental-feature AllowUnsafeAttribute %s -I %t + +import unsafe_swift_decls // ----------------------------------------------------------------------- // Witness matching @@ -16,6 +21,16 @@ struct XP: P { @unsafe func g() { } } +// ----------------------------------------------------------------------- +// Conformances +// ----------------------------------------------------------------------- + +protocol Ptrable2 { + associatedtype Ptr // expected-note{{'Ptr' declared here}} +} + +extension HasAPointerType: Ptrable2 { } // expected-error{{unsafe type 'HasAPointerType.Ptr' (aka 'PointerType') cannot satisfy safe associated type 'Ptr'}} + // ----------------------------------------------------------------------- // Overrides // ----------------------------------------------------------------------- @@ -40,7 +55,7 @@ struct SuperHolder { // ----------------------------------------------------------------------- // Inheritance of @unsafe // ----------------------------------------------------------------------- -@unsafe class UnsafeSuper { // expected-note 2{{'UnsafeSuper' declared here}} +@unsafe class UnsafeSuper { // expected-note 3{{'UnsafeSuper' declared here}} func f() { } // expected-note{{'f()' declared here}} }; @@ -52,13 +67,15 @@ class UnsafeSub: UnsafeSuper { } // expected-error{{reference to unsafe class 'U @unsafe func unsafeF() { } // expected-note{{'unsafeF()' declared here}} @unsafe var unsafeVar: Int = 0 // expected-note{{'unsafeVar' declared here}} -@unsafe struct PointerType { } // expected-note{{'PointerType' declared here}} - func testMe( _ pointer: PointerType, // expected-error{{reference to unsafe struct 'PointerType'}} _ unsafeSuper: UnsafeSuper // expected-error{{reference to unsafe class 'UnsafeSuper'}} + // expected-note@-1{{'unsafeSuper' declared here}} ) { unsafeF() // expected-error{{call to unsafe global function 'unsafeF'}} _ = unsafeVar // expected-error{{reference to unsafe var 'unsafeVar'}} unsafeSuper.f() // expected-error{{call to unsafe instance method 'f'}} + // expected-error@-1{{reference to parameter 'unsafeSuper' involves unsafe type 'UnsafeSuper'}} + + _ = getPointers() // expected-error{{call to global function 'getPointers' involves unsafe type 'PointerType'}} } From 8378562e1204fd763476c171fb5a7fb5d8aad2d5 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 23 Jul 2024 09:43:03 -0700 Subject: [PATCH 3/6] Adopt `@unsafe` throughout the standard library Annotate all of the `Unsafe*` types and `unsafe` functions in the standard library (including concurrency, synchronization, etc.) as `@unsafe`. Add a few tests to ensure that we detect uses of these types in clients that have disabled unsafe code. --- stdlib/cmake/modules/SwiftSource.cmake | 1 + stdlib/public/Concurrency/CMakeLists.txt | 2 +- .../public/Concurrency/PartialAsyncTask.swift | 3 +++ stdlib/public/Concurrency/Task.swift | 1 + stdlib/public/Synchronization/Mutex/Mutex.swift | 3 +++ stdlib/public/Volatile/Volatile.swift | 1 + stdlib/public/core/Builtin.swift | 5 +++++ stdlib/public/core/CTypes.swift | 1 + stdlib/public/core/Misc.swift | 1 + stdlib/public/core/Optional.swift | 1 + stdlib/public/core/Unmanaged.swift | 1 + .../public/core/UnsafeBufferPointer.swift.gyb | 1 + stdlib/public/core/UnsafePointer.swift | 2 ++ .../core/UnsafeRawBufferPointer.swift.gyb | 1 + stdlib/public/core/UnsafeRawPointer.swift | 2 ++ test/Unsafe/Inputs/unsafe_decls.h | 2 ++ test/Unsafe/unsafe_imports.swift | 4 ++++ test/Unsafe/unsafe_stdlib.swift | 17 +++++++++++++++++ 18 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 test/Unsafe/unsafe_stdlib.swift diff --git a/stdlib/cmake/modules/SwiftSource.cmake b/stdlib/cmake/modules/SwiftSource.cmake index 7dcac4c57d85b..1a1986904e228 100644 --- a/stdlib/cmake/modules/SwiftSource.cmake +++ b/stdlib/cmake/modules/SwiftSource.cmake @@ -628,6 +628,7 @@ function(_compile_swift_files list(APPEND swift_flags "-enable-experimental-feature" "NoncopyableGenerics2") list(APPEND swift_flags "-enable-experimental-feature" "SuppressedAssociatedTypes") list(APPEND swift_flags "-enable-experimental-feature" "SE427NoInferenceOnExtension") + list(APPEND swift_flags "-enable-experimental-feature" "AllowUnsafeAttribute") if(SWIFT_ENABLE_EXPERIMENTAL_NONESCAPABLE_TYPES) list(APPEND swift_flags "-enable-experimental-feature" "NonescapableTypes") diff --git a/stdlib/public/Concurrency/CMakeLists.txt b/stdlib/public/Concurrency/CMakeLists.txt index 30e2300b42f5c..4d13ee095d671 100644 --- a/stdlib/public/Concurrency/CMakeLists.txt +++ b/stdlib/public/Concurrency/CMakeLists.txt @@ -64,7 +64,7 @@ endif() list(APPEND SWIFT_RUNTIME_CONCURRENCY_SWIFT_FLAGS "-enable-experimental-feature" "IsolatedAny" -) + ) list(APPEND SWIFT_RUNTIME_CONCURRENCY_C_FLAGS "-D__STDC_WANT_LIB_EXT1__=1") diff --git a/stdlib/public/Concurrency/PartialAsyncTask.swift b/stdlib/public/Concurrency/PartialAsyncTask.swift index 552c5b09c8675..eb35d6ec5cddd 100644 --- a/stdlib/public/Concurrency/PartialAsyncTask.swift +++ b/stdlib/public/Concurrency/PartialAsyncTask.swift @@ -480,6 +480,7 @@ extension JobPriority: Comparable { /// without making other changes. @available(SwiftStdlib 5.1, *) @frozen +@unsafe public struct UnsafeContinuation: Sendable { @usableFromInline internal var context: Builtin.RawUnsafeContinuation @@ -683,6 +684,7 @@ internal func _resumeUnsafeThrowingContinuationWithError( /// - SeeAlso: `withCheckedThrowingContinuation(function:_:)` @available(SwiftStdlib 5.1, *) @_alwaysEmitIntoClient +@unsafe public func withUnsafeContinuation( isolation: isolated (any Actor)? = #isolation, _ fn: (UnsafeContinuation) -> Void @@ -719,6 +721,7 @@ public func withUnsafeContinuation( /// - SeeAlso: `withCheckedThrowingContinuation(function:_:)` @available(SwiftStdlib 5.1, *) @_alwaysEmitIntoClient +@unsafe public func withUnsafeThrowingContinuation( isolation: isolated (any Actor)? = #isolation, _ fn: (UnsafeContinuation) -> Void diff --git a/stdlib/public/Concurrency/Task.swift b/stdlib/public/Concurrency/Task.swift index 2aa563dd54dc4..0a326ef8d9737 100644 --- a/stdlib/public/Concurrency/Task.swift +++ b/stdlib/public/Concurrency/Task.swift @@ -1033,6 +1033,7 @@ public func withUnsafeCurrentTask(body: (UnsafeCurrentTask?) async throws -> /// [concurrency]: https://docs.swift.org/swift-book/LanguageGuide/Concurrency.html /// [tspl]: https://docs.swift.org/swift-book/ @available(SwiftStdlib 5.1, *) +@unsafe public struct UnsafeCurrentTask { internal let _task: Builtin.NativeObject diff --git a/stdlib/public/Synchronization/Mutex/Mutex.swift b/stdlib/public/Synchronization/Mutex/Mutex.swift index 800b22ed87aee..8b46810cf1fb8 100644 --- a/stdlib/public/Synchronization/Mutex/Mutex.swift +++ b/stdlib/public/Synchronization/Mutex/Mutex.swift @@ -175,6 +175,7 @@ extension _MutexHandle { @available(SwiftStdlib 6.0, *) @_alwaysEmitIntoClient @_transparent + @unsafe public borrowing func unsafeLock() { _lock() } @@ -182,6 +183,7 @@ extension _MutexHandle { @available(SwiftStdlib 6.0, *) @_alwaysEmitIntoClient @_transparent + @unsafe public borrowing func unsafeTryLock() -> Bool { _tryLock() } @@ -189,6 +191,7 @@ extension _MutexHandle { @available(SwiftStdlib 6.0, *) @_alwaysEmitIntoClient @_transparent + @unsafe public borrowing func unsafeUnlock() { _unlock() } diff --git a/stdlib/public/Volatile/Volatile.swift b/stdlib/public/Volatile/Volatile.swift index 1ce38f835568c..d3ce545f17ddb 100644 --- a/stdlib/public/Volatile/Volatile.swift +++ b/stdlib/public/Volatile/Volatile.swift @@ -29,6 +29,7 @@ public struct VolatileMappedRegister { let _rawPointer: Builtin.RawPointer @_transparent + @unsafe public init(unsafeBitPattern: UInt) { self._rawPointer = Builtin.inttoptr_Word(unsafeBitPattern._builtinWordValue) } diff --git a/stdlib/public/core/Builtin.swift b/stdlib/public/core/Builtin.swift index 731b77b9fa40e..fe0c6196e24fa 100644 --- a/stdlib/public/core/Builtin.swift +++ b/stdlib/public/core/Builtin.swift @@ -91,6 +91,7 @@ func _canBeClass(_: T.Type) -> Int8 { /// Returns: A new instance of type `U`, cast from `x`. @inlinable // unsafe-performance @_transparent +@unsafe public func unsafeBitCast(_ x: T, to type: U.Type) -> U { _precondition(MemoryLayout.size == MemoryLayout.size, "Can't unsafeBitCast between types of different sizes") @@ -242,6 +243,7 @@ internal func _isClassOrObjCExistential(_ x: T.Type) -> Bool { /// be either a class or a class protocol. Either T, U, or both may be /// optional references. @_transparent +@unsafe public func _unsafeReferenceCast(_ x: T, to: U.Type) -> U { return Builtin.castReference(x) } @@ -265,12 +267,14 @@ public func _unsafeReferenceCast(_ x: T, to: U.Type) -> U { /// - type: The type `T` to which `x` is cast. /// - Returns: The instance `x`, cast to type `T`. @_transparent +@unsafe public func unsafeDowncast(_ x: AnyObject, to type: T.Type) -> T { _debugPrecondition(x is T, "invalid unsafeDowncast") return Builtin.castReference(x) } @_transparent +@unsafe public func _unsafeUncheckedDowncast(_ x: AnyObject, to type: T.Type) -> T { _internalInvariant(x is T, "invalid unsafeDowncast") return Builtin.castReference(x) @@ -331,6 +335,7 @@ public func _onFastPath() { // Optimizer hint that the condition is true. The condition is unchecked. // The builtin acts as an opaque instruction with side-effects. @usableFromInline @_transparent +@unsafe func _uncheckedUnsafeAssume(_ condition: Bool) { _ = Builtin.assume_Int1(condition._value) } diff --git a/stdlib/public/core/CTypes.swift b/stdlib/public/core/CTypes.swift index 35a0721ab3ce6..b45bd8a7fce34 100644 --- a/stdlib/public/core/CTypes.swift +++ b/stdlib/public/core/CTypes.swift @@ -150,6 +150,7 @@ public typealias CBool = Bool /// Opaque pointers are used to represent C pointers to types that /// cannot be represented in Swift, such as incomplete struct types. @frozen +@unsafe public struct OpaquePointer { @usableFromInline internal var _rawValue: Builtin.RawPointer diff --git a/stdlib/public/core/Misc.swift b/stdlib/public/core/Misc.swift index 60c0e9da1489d..4ee2994ee3257 100644 --- a/stdlib/public/core/Misc.swift +++ b/stdlib/public/core/Misc.swift @@ -155,6 +155,7 @@ public func _getTypeByMangledNameInContext( /// Prevents performance diagnostics in the passed closure. @_alwaysEmitIntoClient @_semantics("no_performance_analysis") +@unsafe public func _unsafePerformance(_ c: () -> T) -> T { return c() } diff --git a/stdlib/public/core/Optional.swift b/stdlib/public/core/Optional.swift index 9ec72fcd67dab..99ad22595f6dc 100644 --- a/stdlib/public/core/Optional.swift +++ b/stdlib/public/core/Optional.swift @@ -338,6 +338,7 @@ extension Optional { /// will never be equal to `nil` and only after you've tried using the /// postfix `!` operator. @inlinable + @unsafe public var unsafelyUnwrapped: Wrapped { @inline(__always) get { diff --git a/stdlib/public/core/Unmanaged.swift b/stdlib/public/core/Unmanaged.swift index 8ce562cd98913..58b6d6af7fc9c 100644 --- a/stdlib/public/core/Unmanaged.swift +++ b/stdlib/public/core/Unmanaged.swift @@ -15,6 +15,7 @@ /// When you use this type, you become partially responsible for /// keeping the object alive. @frozen +@unsafe public struct Unmanaged { @usableFromInline internal unowned(unsafe) var _value: Instance diff --git a/stdlib/public/core/UnsafeBufferPointer.swift.gyb b/stdlib/public/core/UnsafeBufferPointer.swift.gyb index f677f561cadc8..62a10459e4888 100644 --- a/stdlib/public/core/UnsafeBufferPointer.swift.gyb +++ b/stdlib/public/core/UnsafeBufferPointer.swift.gyb @@ -30,6 +30,7 @@ /// collection with an `${Self}` instance copies the instances out of the /// referenced memory and into the new collection. @frozen // unsafe-performance +@unsafe public struct Unsafe${Mutable}BufferPointer: Copyable { @usableFromInline diff --git a/stdlib/public/core/UnsafePointer.swift b/stdlib/public/core/UnsafePointer.swift index 2cafd1766419f..dbda0f2dd0ac7 100644 --- a/stdlib/public/core/UnsafePointer.swift +++ b/stdlib/public/core/UnsafePointer.swift @@ -204,6 +204,7 @@ /// let numberPointer = UnsafePointer(&number) /// // Accessing 'numberPointer' is undefined behavior. @frozen // unsafe-performance +@unsafe public struct UnsafePointer: Copyable { /// The underlying raw (untyped) pointer. @@ -655,6 +656,7 @@ extension UnsafePointer where Pointee: ~Copyable { /// let numberPointer = UnsafeMutablePointer(&number) /// // Accessing 'numberPointer' is undefined behavior. @frozen // unsafe-performance +@unsafe public struct UnsafeMutablePointer: Copyable { /// The underlying raw (untyped) pointer. @_preInverseGenerics diff --git a/stdlib/public/core/UnsafeRawBufferPointer.swift.gyb b/stdlib/public/core/UnsafeRawBufferPointer.swift.gyb index bd3650b6466ca..4230b980bd0c6 100644 --- a/stdlib/public/core/UnsafeRawBufferPointer.swift.gyb +++ b/stdlib/public/core/UnsafeRawBufferPointer.swift.gyb @@ -95,6 +95,7 @@ /// destBytes[0..'}} } diff --git a/test/Unsafe/unsafe_stdlib.swift b/test/Unsafe/unsafe_stdlib.swift new file mode 100644 index 0000000000000..4396f4b384320 --- /dev/null +++ b/test/Unsafe/unsafe_stdlib.swift @@ -0,0 +1,17 @@ +// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature DisallowUnsafe + +// Make sure everything compiles without error when unsafe code is allowed. +// RUN: %target-swift-frontend -typecheck -enable-experimental-feature AllowUnsafeAttribute %s + +func test( + x: OpaquePointer, // expected-error{{reference to unsafe struct 'OpaquePointer'}} + other: UnsafeMutablePointer // expected-error{{reference to unsafe generic struct 'UnsafeMutablePointer'}} +) { + var array = [1, 2, 3] + // expected-error@+1{{call to instance method 'withUnsafeBufferPointer' involves unsafe type 'UnsafeBufferPointer'}} + array.withUnsafeBufferPointer{ buffer in // expected-note{{'buffer' declared here}} + print(buffer) // expected-error{{reference to parameter 'buffer' involves unsafe type 'UnsafeBufferPointer'}} + } + array.append(4) + _ = array +} From 7e831f248b1872896dcf8dd62d8e8fca71aa145a Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Wed, 24 Jul 2024 07:50:01 -0700 Subject: [PATCH 4/6] Update API test for addition of @unsafe --- .../stability-stdlib-source-x86_64.swift.expected | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/api-digester/Outputs/stability-stdlib-source-x86_64.swift.expected b/test/api-digester/Outputs/stability-stdlib-source-x86_64.swift.expected index 50e86c5a1b695..788bdd2934e73 100644 --- a/test/api-digester/Outputs/stability-stdlib-source-x86_64.swift.expected +++ b/test/api-digester/Outputs/stability-stdlib-source-x86_64.swift.expected @@ -3,3 +3,18 @@ // NOTE: Most differences from the baseline are common across all architectures // should go into stability-stdlib-source-base.swift.expected. Only capture // x86_64-specific differences from the baseline here. + +// Adoption of @unsafe +Func unsafeBitCast(_:to:) is now with @unsafe +Func unsafeDowncast(_:to:) is now with @unsafe +Struct OpaquePointer is now with @unsafe +Struct Unmanaged is now with @unsafe +Struct UnsafeBufferPointer is now with @unsafe +Struct UnsafeMutableBufferPointer is now with @unsafe +Struct UnsafeMutablePointer is now with @unsafe +Struct UnsafeMutableRawBufferPointer is now with @unsafe +Struct UnsafeMutableRawPointer is now with @unsafe +Struct UnsafePointer is now with @unsafe +Struct UnsafeRawBufferPointer is now with @unsafe +Struct UnsafeRawPointer is now with @unsafe +Var Optional.unsafelyUnwrapped is now with @unsafe From 571859f294bd125ec602a84455d30409864d41a6 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Wed, 14 Aug 2024 21:36:02 -0700 Subject: [PATCH 5/6] Move "unsafe" diagnostics from errors to warnings Warnings fit better with the approach we're going for, and can be escalated to errors by `-warnings-as-errors` for clients that need it. --- include/swift/AST/DiagnosticsSema.def | 16 ++++++------- include/swift/Basic/Features.def | 4 ++-- lib/AST/FeatureSet.cpp | 2 +- lib/Sema/AssociatedTypeInference.cpp | 2 +- lib/Sema/TypeCheckAttr.cpp | 4 ++-- lib/Sema/TypeCheckAvailability.cpp | 33 ++++++++++++--------------- lib/Sema/TypeCheckDeclOverride.cpp | 2 +- lib/Sema/TypeCheckProtocol.cpp | 4 ++-- lib/Sema/TypeCheckType.cpp | 12 ++++------ lib/Sema/TypeCheckType.h | 5 +--- test/Unsafe/unsafe.swift | 24 +++++++++---------- test/Unsafe/unsafe_concurrency.swift | 6 ++--- test/Unsafe/unsafe_imports.swift | 8 +++---- test/Unsafe/unsafe_stdlib.swift | 12 +++++----- 14 files changed, 63 insertions(+), 71 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 2fa99782f7c88..fa1299b5d68c5 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -7969,24 +7969,24 @@ NOTE(sending_function_result_with_sending_param_note, none, //------------------------------------------------------------------------------ ERROR(unsafe_attr_disabled,none, "attribute requires '-enable-experimental-feature AllowUnsafeAttribute'", ()) -ERROR(override_safe_withunsafe,none, +WARNING(override_safe_withunsafe,none, "override of safe %0 with unsafe %0", (DescriptiveDeclKind)) -ERROR(witness_unsafe,none, +WARNING(witness_unsafe,none, "unsafe %0 %1 cannot satisfy safe requirement", (DescriptiveDeclKind, DeclName)) -ERROR(type_witness_unsafe,none, +WARNING(type_witness_unsafe,none, "unsafe type %0 cannot satisfy safe associated type %1", (Type, DeclName)) -ERROR(unchecked_conformance_is_unsafe,none, +WARNING(unchecked_conformance_is_unsafe,none, "@unchecked conformance involves unsafe code", ()) -ERROR(unowned_unsafe_is_unsafe,none, +WARNING(unowned_unsafe_is_unsafe,none, "unowned(unsafe) involves unsafe code", ()) -ERROR(nonisolated_unsafe_is_unsafe,none, +WARNING(nonisolated_unsafe_is_unsafe,none, "nonisolated(unsafe) involves unsafe code", ()) -ERROR(reference_to_unsafe_decl,none, +WARNING(reference_to_unsafe_decl,none, "%select{reference|call}0 to unsafe %kindbase1", (bool, const ValueDecl *)) -ERROR(reference_to_unsafe_typed_decl,none, +WARNING(reference_to_unsafe_typed_decl,none, "%select{reference|call}0 to %kindbase1 involves unsafe type %2", (bool, const ValueDecl *, Type)) NOTE(unsafe_decl_here,none, diff --git a/include/swift/Basic/Features.def b/include/swift/Basic/Features.def index abd590365d3f3..be3a851521687 100644 --- a/include/swift/Basic/Features.def +++ b/include/swift/Basic/Features.def @@ -405,8 +405,8 @@ EXPERIMENTAL_FEATURE(TrailingComma, false) /// Allow the @unsafe attribute. SUPPRESSIBLE_EXPERIMENTAL_FEATURE(AllowUnsafeAttribute, true) -/// Disallow use of unsafe code. -EXPERIMENTAL_FEATURE(DisallowUnsafe, true) +/// Warn on use of unsafe constructs. +EXPERIMENTAL_FEATURE(WarnUnsafe, true) #undef EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE #undef EXPERIMENTAL_FEATURE diff --git a/lib/AST/FeatureSet.cpp b/lib/AST/FeatureSet.cpp index 6a32f0dada088..5f232958b4673 100644 --- a/lib/AST/FeatureSet.cpp +++ b/lib/AST/FeatureSet.cpp @@ -230,7 +230,7 @@ static bool usesFeatureAllowUnsafeAttribute(Decl *decl) { return decl->getAttrs().hasAttribute(); } -UNINTERESTING_FEATURE(DisallowUnsafe) +UNINTERESTING_FEATURE(WarnUnsafe) // ---------------------------------------------------------------------------- // MARK: - FeatureSet diff --git a/lib/Sema/AssociatedTypeInference.cpp b/lib/Sema/AssociatedTypeInference.cpp index 47e91179ccad5..9fc5e01761362 100644 --- a/lib/Sema/AssociatedTypeInference.cpp +++ b/lib/Sema/AssociatedTypeInference.cpp @@ -346,7 +346,7 @@ static void recordTypeWitness(NormalProtocolConformance *conformance, } // If we're disallowing unsafe code, check for an unsafe type witness. - if (ctx.LangOpts.hasFeature(Feature::DisallowUnsafe) && + if (ctx.LangOpts.hasFeature(Feature::WarnUnsafe) && !assocType->isUnsafe() && type->isUnsafe()) { SourceLoc loc = typeDecl->getLoc(); if (loc.isInvalid()) diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 93bd73fbb93df..5120892ab1378 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -4900,7 +4900,7 @@ Type TypeChecker::checkReferenceOwnershipAttr(VarDecl *var, Type type, // unowned(unsafe) is unsafe (duh). if (ownershipKind == ReferenceOwnership::Unmanaged && - ctx.LangOpts.hasFeature(Feature::DisallowUnsafe)) { + ctx.LangOpts.hasFeature(Feature::WarnUnsafe)) { Diags.diagnose(attr->getLocation(), diag::unowned_unsafe_is_unsafe); } @@ -7100,7 +7100,7 @@ void AttributeChecker::visitNonisolatedAttr(NonisolatedAttr *attr) { // nonisolated(unsafe) is unsafe, but only under strict concurrency. if (attr->isUnsafe() && - Ctx.LangOpts.hasFeature(Feature::DisallowUnsafe) && + Ctx.LangOpts.hasFeature(Feature::WarnUnsafe) && Ctx.LangOpts.StrictConcurrencyLevel == StrictConcurrency::Complete) Ctx.Diags.diagnose(attr->getLocation(), diag::nonisolated_unsafe_is_unsafe); diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index 7f3ce46d947cf..cbc242a64324e 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -3886,21 +3886,20 @@ bool ExprAvailabilityWalker::diagnoseDeclRefAvailability( // If the declaration itself is "safe" but we don't disallow unsafe uses, // check whether it traffics in unsafe types. ASTContext &ctx = D->getASTContext(); - if (ctx.LangOpts.hasFeature(Feature::DisallowUnsafe) && !D->isUnsafe()) { + if (ctx.LangOpts.hasFeature(Feature::WarnUnsafe) && !D->isUnsafe()) { auto type = D->getInterfaceType(); if (auto subs = declRef.getSubstitutions()) type = type.subst(subs); if (type->isUnsafe()) { - if (diagnoseUnsafeType( - ctx, R.Start, type, - [&](Type specificType) { - ctx.Diags.diagnose( - R.Start, diag::reference_to_unsafe_typed_decl, - call != nullptr && !isa(D), D, - specificType); - D->diagnose(diag::decl_declared_here, D); - })) - return true; + diagnoseUnsafeType( + ctx, R.Start, type, + [&](Type specificType) { + ctx.Diags.diagnose( + R.Start, diag::reference_to_unsafe_typed_decl, + call != nullptr && !isa(D), D, + specificType); + D->diagnose(diag::decl_declared_here, D); + }); } } @@ -3966,21 +3965,20 @@ diagnoseDeclAsyncAvailability(const ValueDecl *D, SourceRange R, } /// Diagnose uses of unsafe declarations. -static bool +static void diagnoseDeclUnsafe(const ValueDecl *D, SourceRange R, const Expr *call, const ExportContext &Where) { ASTContext &ctx = D->getASTContext(); - if (!ctx.LangOpts.hasFeature(Feature::DisallowUnsafe)) - return false; + if (!ctx.LangOpts.hasFeature(Feature::WarnUnsafe)) + return; if (!D->isUnsafe()) - return false; + return; SourceLoc diagLoc = call ? call->getLoc() : R.Start; ctx.Diags .diagnose(diagLoc, diag::reference_to_unsafe_decl, call != nullptr, D); D->diagnose(diag::decl_declared_here, D); - return true; } /// Diagnose uses of unavailable declarations. Returns true if a diagnostic @@ -4019,8 +4017,7 @@ bool swift::diagnoseDeclAvailability(const ValueDecl *D, SourceRange R, if (diagnoseDeclAsyncAvailability(D, R, call, Where)) return true; - if (diagnoseDeclUnsafe(D, R, call, Where)) - return true; + diagnoseDeclUnsafe(D, R, call, Where); // Make sure not to diagnose an accessor's deprecation if we already // complained about the property/subscript. diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp index 3cf598f34abbe..f032448690ad9 100644 --- a/lib/Sema/TypeCheckDeclOverride.cpp +++ b/lib/Sema/TypeCheckDeclOverride.cpp @@ -1733,7 +1733,7 @@ namespace { void visitObjCAttr(ObjCAttr *attr) {} void visitUnsafeAttr(UnsafeAttr *attr) { - if (!Base->getASTContext().LangOpts.hasFeature(Feature::DisallowUnsafe)) + if (!Base->getASTContext().LangOpts.hasFeature(Feature::WarnUnsafe)) return; if (Override->isUnsafe() && !Base->isUnsafe()) { diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index 3ad8422e251b1..1ad929014fe6b 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -2430,7 +2430,7 @@ checkIndividualConformance(NormalProtocolConformance *conformance) { // @unchecked conformances are considered unsafe in strict concurrency mode. if (conformance->isUnchecked() && - Context.LangOpts.hasFeature(Feature::DisallowUnsafe) && + Context.LangOpts.hasFeature(Feature::WarnUnsafe) && Context.LangOpts.StrictConcurrencyLevel == StrictConcurrency::Complete) { Context.Diags.diagnose(ComplainLoc, diag::unchecked_conformance_is_unsafe); } @@ -5032,7 +5032,7 @@ void ConformanceChecker::resolveValueWitnesses() { // If we're disallowing unsafe code, check for an unsafe witness to a // safe requirement. - if (C.LangOpts.hasFeature(Feature::DisallowUnsafe) && + if (C.LangOpts.hasFeature(Feature::WarnUnsafe) && witness->isUnsafe() && !requirement->isUnsafe()) { witness->diagnose(diag::witness_unsafe, witness->getDescriptiveKind(), diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index eb836a1f19c59..f67bcd3a0f3d3 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -6422,13 +6422,13 @@ Type ExplicitCaughtTypeRequest::evaluate( llvm_unreachable("Unhandled catch node"); } -bool swift::diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type, +void swift::diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type, llvm::function_ref diagnose) { - if (!ctx.LangOpts.hasFeature(Feature::DisallowUnsafe)) - return false; + if (!ctx.LangOpts.hasFeature(Feature::WarnUnsafe)) + return; if (!type->isUnsafe()) - return false; + return; // Look for a specific @unsafe nominal type. Type specificType; @@ -6436,7 +6436,7 @@ bool swift::diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type, if (auto typeDecl = type->getAnyNominal()) { if (typeDecl->isUnsafe()) { specificType = type; - return true; + return false; } } @@ -6450,6 +6450,4 @@ bool swift::diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type, specificTypeDecl->diagnose(diag::unsafe_decl_here, specificTypeDecl); } } - - return true; } diff --git a/lib/Sema/TypeCheckType.h b/lib/Sema/TypeCheckType.h index bc3f02a4f76bc..b56d126d520fc 100644 --- a/lib/Sema/TypeCheckType.h +++ b/lib/Sema/TypeCheckType.h @@ -688,10 +688,7 @@ bool diagnoseMissingOwnership(ParamSpecifier ownership, /// If the given type involves an unsafe type, diagnose it by calling the /// diagnose function with the most specific unsafe type that can be provided. -/// -/// \returns \c true if the type was unsafe and we are diagnosing unsafe types -/// here. -bool diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type, +void diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type, llvm::function_ref diagnose); } // end namespace swift diff --git a/test/Unsafe/unsafe.swift b/test/Unsafe/unsafe.swift index 11bc90ce0c2a5..bc6df9e218d66 100644 --- a/test/Unsafe/unsafe.swift +++ b/test/Unsafe/unsafe.swift @@ -1,7 +1,7 @@ // RUN: %empty-directory(%t) // RUN: %target-swift-frontend -emit-module-path %t/unsafe_swift_decls.swiftmodule %S/Inputs/unsafe_swift_decls.swift -enable-experimental-feature AllowUnsafeAttribute -// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature DisallowUnsafe -I %t +// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature WarnUnsafe -I %t // Make sure everything compiles without error when unsafe code is allowed. // RUN: %target-swift-frontend -typecheck -enable-experimental-feature AllowUnsafeAttribute %s -I %t @@ -17,7 +17,7 @@ protocol P { } struct XP: P { - @unsafe func f() { } // expected-error{{unsafe instance method 'f()' cannot satisfy safe requirement}} + @unsafe func f() { } // expected-warning{{unsafe instance method 'f()' cannot satisfy safe requirement}} @unsafe func g() { } } @@ -29,7 +29,7 @@ protocol Ptrable2 { associatedtype Ptr // expected-note{{'Ptr' declared here}} } -extension HasAPointerType: Ptrable2 { } // expected-error{{unsafe type 'HasAPointerType.Ptr' (aka 'PointerType') cannot satisfy safe associated type 'Ptr'}} +extension HasAPointerType: Ptrable2 { } // expected-warning{{unsafe type 'HasAPointerType.Ptr' (aka 'PointerType') cannot satisfy safe associated type 'Ptr'}} // ----------------------------------------------------------------------- // Overrides @@ -49,7 +49,7 @@ class Sub: Super { // ----------------------------------------------------------------------- struct SuperHolder { unowned var s1: Super - unowned(unsafe) var s2: Super // expected-error{{unowned(unsafe) involves unsafe code}} + unowned(unsafe) var s2: Super // expected-warning{{unowned(unsafe) involves unsafe code}} } // ----------------------------------------------------------------------- @@ -59,7 +59,7 @@ struct SuperHolder { func f() { } // expected-note{{'f()' declared here}} }; -class UnsafeSub: UnsafeSuper { } // expected-error{{reference to unsafe class 'UnsafeSuper'}} +class UnsafeSub: UnsafeSuper { } // expected-warning{{reference to unsafe class 'UnsafeSuper'}} // ----------------------------------------------------------------------- // Declaration references @@ -68,14 +68,14 @@ class UnsafeSub: UnsafeSuper { } // expected-error{{reference to unsafe class 'U @unsafe var unsafeVar: Int = 0 // expected-note{{'unsafeVar' declared here}} func testMe( - _ pointer: PointerType, // expected-error{{reference to unsafe struct 'PointerType'}} - _ unsafeSuper: UnsafeSuper // expected-error{{reference to unsafe class 'UnsafeSuper'}} + _ pointer: PointerType, // expected-warning{{reference to unsafe struct 'PointerType'}} + _ unsafeSuper: UnsafeSuper // expected-warning{{reference to unsafe class 'UnsafeSuper'}} // expected-note@-1{{'unsafeSuper' declared here}} ) { - unsafeF() // expected-error{{call to unsafe global function 'unsafeF'}} - _ = unsafeVar // expected-error{{reference to unsafe var 'unsafeVar'}} - unsafeSuper.f() // expected-error{{call to unsafe instance method 'f'}} - // expected-error@-1{{reference to parameter 'unsafeSuper' involves unsafe type 'UnsafeSuper'}} + unsafeF() // expected-warning{{call to unsafe global function 'unsafeF'}} + _ = unsafeVar // expected-warning{{reference to unsafe var 'unsafeVar'}} + unsafeSuper.f() // expected-warning{{call to unsafe instance method 'f'}} + // expected-warning@-1{{reference to parameter 'unsafeSuper' involves unsafe type 'UnsafeSuper'}} - _ = getPointers() // expected-error{{call to global function 'getPointers' involves unsafe type 'PointerType'}} + _ = getPointers() // expected-warning{{call to global function 'getPointers' involves unsafe type 'PointerType'}} } diff --git a/test/Unsafe/unsafe_concurrency.swift b/test/Unsafe/unsafe_concurrency.swift index 823394fb3c16c..9d43ff670ebcc 100644 --- a/test/Unsafe/unsafe_concurrency.swift +++ b/test/Unsafe/unsafe_concurrency.swift @@ -1,15 +1,15 @@ -// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature DisallowUnsafe -enable-experimental-feature StrictConcurrency +// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature WarnUnsafe -enable-experimental-feature StrictConcurrency // REQUIRES: concurrency -// expected-error@+1{{@unchecked conformance involves unsafe code}} +// expected-warning@+1{{@unchecked conformance involves unsafe code}} class C: @unchecked Sendable { var counter: Int = 0 } @available(SwiftStdlib 5.1, *) func f() async { - // expected-error@+1{{nonisolated(unsafe) involves unsafe code}} + // expected-warning@+1{{nonisolated(unsafe) involves unsafe code}} nonisolated(unsafe) var counter = 0 Task.detached { counter += 1 diff --git a/test/Unsafe/unsafe_imports.swift b/test/Unsafe/unsafe_imports.swift index 3002cc614e76b..be34cdee86b3e 100644 --- a/test/Unsafe/unsafe_imports.swift +++ b/test/Unsafe/unsafe_imports.swift @@ -1,11 +1,11 @@ -// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature DisallowUnsafe -I %S/Inputs +// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature WarnUnsafe -I %S/Inputs import unsafe_decls -func testUnsafe(_ ut: UnsafeType) { // expected-error{{reference to unsafe struct 'UnsafeType'}} - unsafe_c_function() // expected-error{{call to unsafe global function 'unsafe_c_function'}} +func testUnsafe(_ ut: UnsafeType) { // expected-warning{{reference to unsafe struct 'UnsafeType'}} + unsafe_c_function() // expected-warning{{call to unsafe global function 'unsafe_c_function'}} var array: [CInt] = [1, 2, 3, 4, 5] print_ints(&array, CInt(array.count)) - // expected-error@-1{{call to global function 'print_ints' involves unsafe type 'UnsafeMutablePointer'}} + // expected-warning@-1{{call to global function 'print_ints' involves unsafe type 'UnsafeMutablePointer'}} } diff --git a/test/Unsafe/unsafe_stdlib.swift b/test/Unsafe/unsafe_stdlib.swift index 4396f4b384320..52248bd7aca62 100644 --- a/test/Unsafe/unsafe_stdlib.swift +++ b/test/Unsafe/unsafe_stdlib.swift @@ -1,16 +1,16 @@ -// RUN: %target-typecheck-verify-swift -enable-experimental-feature AllowUnsafeAttribute -enable-experimental-feature DisallowUnsafe +// RUN: %target-typecheck-verify-swift -enable-experimental-feature WarnUnsafe // Make sure everything compiles without error when unsafe code is allowed. -// RUN: %target-swift-frontend -typecheck -enable-experimental-feature AllowUnsafeAttribute %s +// RUN: %target-swift-frontend -typecheck -enable-experimental-feature AllowUnsafeAttribute -warnings-as-errors %s func test( - x: OpaquePointer, // expected-error{{reference to unsafe struct 'OpaquePointer'}} - other: UnsafeMutablePointer // expected-error{{reference to unsafe generic struct 'UnsafeMutablePointer'}} + x: OpaquePointer, // expected-warning{{reference to unsafe struct 'OpaquePointer'}} + other: UnsafeMutablePointer // expected-warning{{reference to unsafe generic struct 'UnsafeMutablePointer'}} ) { var array = [1, 2, 3] - // expected-error@+1{{call to instance method 'withUnsafeBufferPointer' involves unsafe type 'UnsafeBufferPointer'}} + // expected-warning@+1{{call to instance method 'withUnsafeBufferPointer' involves unsafe type 'UnsafeBufferPointer'}} array.withUnsafeBufferPointer{ buffer in // expected-note{{'buffer' declared here}} - print(buffer) // expected-error{{reference to parameter 'buffer' involves unsafe type 'UnsafeBufferPointer'}} + print(buffer) // expected-warning{{reference to parameter 'buffer' involves unsafe type 'UnsafeBufferPointer'}} } array.append(4) _ = array From 5412a8a04cf6e338442796be3fa357bff78dc90f Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Mon, 19 Aug 2024 22:14:09 -0700 Subject: [PATCH 6/6] Move baseline updates over to the architecture-independent file --- .../stability-stdlib-source-base.swift.expected | 15 +++++++++++++++ .../stability-stdlib-source-x86_64.swift.expected | 15 --------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/test/api-digester/Outputs/stability-stdlib-source-base.swift.expected b/test/api-digester/Outputs/stability-stdlib-source-base.swift.expected index a5a6e34b3e500..1cddd7b8b13f8 100644 --- a/test/api-digester/Outputs/stability-stdlib-source-base.swift.expected +++ b/test/api-digester/Outputs/stability-stdlib-source-base.swift.expected @@ -360,3 +360,18 @@ Func ContiguousArray.withUnsafeBufferPointer(_:) is now without @rethrows Func ContiguousArray.withUnsafeMutableBufferPointer(_:) has generic signature change from to Func ContiguousArray.withUnsafeMutableBufferPointer(_:) has parameter 0 type change from (inout Swift.UnsafeMutableBufferPointer) throws -> R to (inout Swift.UnsafeMutableBufferPointer) throws(E) -> R Func ContiguousArray.withUnsafeMutableBufferPointer(_:) is now without @rethrows + +// Adoption of @unsafe +Func unsafeBitCast(_:to:) is now with @unsafe +Func unsafeDowncast(_:to:) is now with @unsafe +Struct OpaquePointer is now with @unsafe +Struct Unmanaged is now with @unsafe +Struct UnsafeBufferPointer is now with @unsafe +Struct UnsafeMutableBufferPointer is now with @unsafe +Struct UnsafeMutablePointer is now with @unsafe +Struct UnsafeMutableRawBufferPointer is now with @unsafe +Struct UnsafeMutableRawPointer is now with @unsafe +Struct UnsafePointer is now with @unsafe +Struct UnsafeRawBufferPointer is now with @unsafe +Struct UnsafeRawPointer is now with @unsafe +Var Optional.unsafelyUnwrapped is now with @unsafe diff --git a/test/api-digester/Outputs/stability-stdlib-source-x86_64.swift.expected b/test/api-digester/Outputs/stability-stdlib-source-x86_64.swift.expected index 788bdd2934e73..50e86c5a1b695 100644 --- a/test/api-digester/Outputs/stability-stdlib-source-x86_64.swift.expected +++ b/test/api-digester/Outputs/stability-stdlib-source-x86_64.swift.expected @@ -3,18 +3,3 @@ // NOTE: Most differences from the baseline are common across all architectures // should go into stability-stdlib-source-base.swift.expected. Only capture // x86_64-specific differences from the baseline here. - -// Adoption of @unsafe -Func unsafeBitCast(_:to:) is now with @unsafe -Func unsafeDowncast(_:to:) is now with @unsafe -Struct OpaquePointer is now with @unsafe -Struct Unmanaged is now with @unsafe -Struct UnsafeBufferPointer is now with @unsafe -Struct UnsafeMutableBufferPointer is now with @unsafe -Struct UnsafeMutablePointer is now with @unsafe -Struct UnsafeMutableRawBufferPointer is now with @unsafe -Struct UnsafeMutableRawPointer is now with @unsafe -Struct UnsafePointer is now with @unsafe -Struct UnsafeRawBufferPointer is now with @unsafe -Struct UnsafeRawPointer is now with @unsafe -Var Optional.unsafelyUnwrapped is now with @unsafe