Skip to content

[TypeChecker/SILGen] Dynamic enforcement of witness/objc isolation with @preconcurrency attribute #70867

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/swift/AST/ASTBridging.h
Original file line number Diff line number Diff line change
Expand Up @@ -1161,6 +1161,7 @@ enum ENUM_EXTENSIBILITY_ATTR(closed) BridgedTypeAttrKind : size_t {
BridgedTypeAttrKind_Sendable,
BridgedTypeAttrKind_retroactive,
BridgedTypeAttrKind_unchecked,
BridgedTypeAttrKind_preconcurrency,
BridgedTypeAttrKind__local,
BridgedTypeAttrKind__noMetadata,
BridgedTypeAttrKind__opaqueReturnTypeOf,
Expand Down
3 changes: 2 additions & 1 deletion include/swift/AST/ASTContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -1282,7 +1282,8 @@ class ASTContext final {
SourceLoc loc,
DeclContext *dc,
ProtocolConformanceState state,
bool isUnchecked);
bool isUnchecked,
bool isPreconcurrency);

/// Produce a self-conformance for the given protocol.
SelfProtocolConformance *
Expand Down
1 change: 1 addition & 0 deletions include/swift/AST/Attr.def
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ TYPE_ATTR(async)
TYPE_ATTR(Sendable)
TYPE_ATTR(retroactive)
TYPE_ATTR(unchecked)
TYPE_ATTR(preconcurrency)
TYPE_ATTR(_local)
TYPE_ATTR(_noMetadata)
TYPE_ATTR(_opaqueReturnTypeOf)
Expand Down
10 changes: 7 additions & 3 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1604,11 +1604,15 @@ struct InheritedEntry : public TypeLoc {
/// Whether there was an @retroactive attribute.
bool isRetroactive = false;

/// Whether there was an @preconcurrency attribute.
bool isPreconcurrency = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be bitfields?


InheritedEntry(const TypeLoc &typeLoc);

InheritedEntry(const TypeLoc &typeLoc, bool isUnchecked, bool isRetroactive)
: TypeLoc(typeLoc), isUnchecked(isUnchecked), isRetroactive(isRetroactive) {
}
InheritedEntry(const TypeLoc &typeLoc, bool isUnchecked, bool isRetroactive,
bool isPreconcurrency)
: TypeLoc(typeLoc), isUnchecked(isUnchecked),
isRetroactive(isRetroactive), isPreconcurrency(isPreconcurrency) {}
};

/// A wrapper for the collection of inherited types for either a `TypeDecl` or
Expand Down
9 changes: 9 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -5626,6 +5626,15 @@ ERROR(unchecked_not_inheritance_clause,none,
ERROR(unchecked_not_existential,none,
"'unchecked' attribute cannot apply to non-protocol type %0", (Type))

WARNING(preconcurrency_conformance_not_used,none,
"@preconcurrency attribute on conformance to %0 has no effect", (Type))
ERROR(preconcurrency_not_inheritance_clause,none,
"'preconcurrency' attribute only applies in inheritance clauses", ())
ERROR(preconcurrency_not_existential,none,
"'preconcurrency' attribute cannot apply to non-protocol type %0", (Type))
ERROR(preconcurrency_attr_disabled,none,
"attribute requires '-enable-experimental-feature PreconcurrencyConformances'", ())

ERROR(redundant_any_in_existential,none,
"redundant 'any' in type %0",
(Type))
Expand Down
11 changes: 7 additions & 4 deletions include/swift/AST/NameLookup.h
Original file line number Diff line number Diff line change
Expand Up @@ -586,12 +586,15 @@ struct InheritedNominalEntry : Located<NominalTypeDecl *> {
/// The location of the "unchecked" attribute, if present.
SourceLoc uncheckedLoc;

/// The location of the "preconcurrency" attribute if present.
SourceLoc preconcurrencyLoc;

InheritedNominalEntry() { }

InheritedNominalEntry(
NominalTypeDecl *item, SourceLoc loc,
SourceLoc uncheckedLoc
) : Located(item, loc), uncheckedLoc(uncheckedLoc) { }
InheritedNominalEntry(NominalTypeDecl *item, SourceLoc loc,
SourceLoc uncheckedLoc, SourceLoc preconcurrencyLoc)
: Located(item, loc), uncheckedLoc(uncheckedLoc),
preconcurrencyLoc(preconcurrencyLoc) {}
};

/// Retrieve the set of nominal type declarations that are directly
Expand Down
28 changes: 18 additions & 10 deletions include/swift/AST/ProtocolConformance.h
Original file line number Diff line number Diff line change
Expand Up @@ -444,11 +444,11 @@ class NormalProtocolConformance : public RootProtocolConformance,

// Flag bits used in ContextAndBits.
enum {
/// The conformance is invalid.
InvalidFlag = 0x01,

/// The conformance was labeled with @unchecked.
UncheckedFlag = 0x02,
UncheckedFlag = 0x01,

/// The conformance was labeled with @preconcurrency.
PreconcurrencyFlag = 0x02,

/// We have allocated the AssociatedConformances array (but not necessarily
/// populated any of its elements).
Expand All @@ -458,10 +458,13 @@ class NormalProtocolConformance : public RootProtocolConformance,
/// The declaration context containing the ExtensionDecl or
/// NominalTypeDecl that declared the conformance.
///
/// Also stores the "invalid", "unchecked" and "has computed associated
/// Also stores the "unchecked", "preconcurrency" and "has computed associated
/// conformances" bits.
llvm::PointerIntPair<DeclContext *, 3, unsigned> ContextAndBits;

/// Indicates whether the conformance is invalid.
bool Invalid : 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can combine the bits from ContextAndBits and SourceKindAndImplyingConformance with 'Invalid' into a single bitfield

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed this offline with Slava and I'm going to follow-up with bitfield changes in a separate PR.


/// The reason that this conformance exists.
///
/// Either Explicit (e.g. 'struct Foo: Protocol {}' or 'extension Foo:
Expand Down Expand Up @@ -501,12 +504,14 @@ class NormalProtocolConformance : public RootProtocolConformance,
public:
NormalProtocolConformance(Type conformingType, ProtocolDecl *protocol,
SourceLoc loc, DeclContext *dc,
ProtocolConformanceState state,
bool isUnchecked)
ProtocolConformanceState state, bool isUnchecked,
bool isPreconcurrency)
: RootProtocolConformance(ProtocolConformanceKind::Normal,
conformingType),
ProtocolAndState(protocol, state), Loc(loc),
ContextAndBits(dc, isUnchecked ? UncheckedFlag : 0) {
ContextAndBits(dc, ((isUnchecked ? UncheckedFlag : 0) |
(isPreconcurrency ? PreconcurrencyFlag : 0))),
Invalid(false) {
assert(!conformingType->hasArchetype() &&
"ProtocolConformances should store interface types");
}
Expand Down Expand Up @@ -543,12 +548,12 @@ class NormalProtocolConformance : public RootProtocolConformance,

/// Determine whether this conformance is invalid.
bool isInvalid() const {
return ContextAndBits.getInt() & InvalidFlag;
return Invalid;
}

/// Mark this conformance as invalid.
void setInvalid() {
ContextAndBits.setInt(ContextAndBits.getInt() | InvalidFlag);
Invalid = true;
}

/// Whether this is an "unchecked" conformance.
Expand All @@ -563,6 +568,9 @@ class NormalProtocolConformance : public RootProtocolConformance,
ContextAndBits.setInt(ContextAndBits.getInt() | UncheckedFlag);
}

/// Whether this is an preconcurrency conformance.
bool isPreconcurrency() const;

/// Determine whether we've lazily computed the associated conformance array
/// already.
bool hasComputedAssociatedConformances() const {
Expand Down
3 changes: 3 additions & 0 deletions include/swift/Basic/Features.def
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,9 @@ EXPERIMENTAL_FEATURE(GroupActorErrors, true)
// Allow for the 'transferring' keyword to be applied to arguments and results.
EXPERIMENTAL_FEATURE(TransferringArgsAndResults, true)

// Enable `@preconcurrency` attribute on protocol conformances.
EXPERIMENTAL_FEATURE(PreconcurrencyConformances, false)

#undef EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE
#undef EXPERIMENTAL_FEATURE
#undef UPCOMING_FEATURE
Expand Down
10 changes: 5 additions & 5 deletions lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2490,7 +2490,8 @@ ASTContext::getNormalConformance(Type conformingType,
SourceLoc loc,
DeclContext *dc,
ProtocolConformanceState state,
bool isUnchecked) {
bool isUnchecked,
bool isPreconcurrency) {
assert(dc->isTypeContext());

llvm::FoldingSetNodeID id;
Expand All @@ -2504,10 +2505,9 @@ ASTContext::getNormalConformance(Type conformingType,
return result;

// Build a new normal protocol conformance.
auto result
= new (*this, AllocationArena::Permanent)
NormalProtocolConformance(
conformingType, protocol, loc, dc, state,isUnchecked);
auto result = new (*this, AllocationArena::Permanent)
NormalProtocolConformance(conformingType, protocol, loc, dc, state,
isUnchecked, isPreconcurrency);
normalConformances.InsertNode(result, insertPos);

return result;
Expand Down
29 changes: 28 additions & 1 deletion lib/AST/ASTPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2732,6 +2732,8 @@ void PrintAST::printInherited(const Decl *decl) {
if (inherited.isRetroactive &&
!llvm::is_contained(Options.ExcludeAttrList, TAK_retroactive))
Printer << "@retroactive ";
if (inherited.isPreconcurrency)
Printer << "@preconcurrency ";

printTypeLoc(inherited);
}, [&]() {
Expand Down Expand Up @@ -3906,6 +3908,30 @@ static bool usesFeatureBitwiseCopyable(Decl *decl) { return false; }

static bool usesFeatureTransferringArgsAndResults(Decl *decl) { return false; }

static bool usesFeaturePreconcurrencyConformances(Decl *decl) {
auto usesPreconcurrencyConformance = [&](const InheritedTypes &inherited) {
return llvm::any_of(
inherited.getEntries(),
[](const InheritedEntry &entry) { return entry.isPreconcurrency; });
};

if (auto *T = dyn_cast<TypeDecl>(decl))
return usesPreconcurrencyConformance(T->getInherited());

if (auto *E = dyn_cast<ExtensionDecl>(decl)) {
// If type has `@preconcurrency` conformance(s) all of its
// extensions have to be guarded by the flag too.
if (auto *T = dyn_cast<TypeDecl>(E->getExtendedNominal())) {
if (usesPreconcurrencyConformance(T->getInherited()))
return true;
}

return usesPreconcurrencyConformance(E->getInherited());
}

return false;
}

/// Suppress the printing of a particular feature.
static void suppressingFeature(PrintOptions &options, Feature feature,
llvm::function_ref<void()> action) {
Expand Down Expand Up @@ -8308,7 +8334,8 @@ swift::getInheritedForPrinting(

Results.push_back({TypeLoc::withoutLoc(proto->getDeclaredInterfaceType()),
isUnchecked,
/*isRetroactive=*/false});
/*isRetroactive=*/false,
/*isPreconcurrency=*/false});
}
}

Expand Down
42 changes: 25 additions & 17 deletions lib/AST/ConformanceLookupTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,15 +137,19 @@ void ConformanceLookupTable::destroy() {

namespace {
struct ConformanceConstructionInfo : public Located<ProtocolDecl *> {
/// The location of the "unchecked" attribute, if this
/// The location of the "unchecked" attribute, if present.
const SourceLoc uncheckedLoc;

/// The location of the "preconcurrency" attribute if present.
const SourceLoc preconcurrencyLoc;

ConformanceConstructionInfo() { }

ConformanceConstructionInfo(
ProtocolDecl *item, SourceLoc loc,
SourceLoc uncheckedLoc
) : Located(item, loc), uncheckedLoc(uncheckedLoc) { }
ConformanceConstructionInfo(ProtocolDecl *item, SourceLoc loc,
SourceLoc uncheckedLoc,
SourceLoc preconcurrencyLoc)
: Located(item, loc), uncheckedLoc(uncheckedLoc),
preconcurrencyLoc(preconcurrencyLoc) {}
};
}

Expand Down Expand Up @@ -200,15 +204,17 @@ void ConformanceLookupTable::forEachInStage(ConformanceStage stage,
loader.first->loadAllConformances(next, loader.second, conformances);
loadAllConformances(next, conformances);
for (auto conf : conformances) {
protocols.push_back({conf->getProtocol(), SourceLoc(), SourceLoc()});
protocols.push_back(
{conf->getProtocol(), SourceLoc(), SourceLoc(), SourceLoc()});
}
} else if (next->getParentSourceFile() ||
next->getParentModule()->isBuiltinModule()) {
bool anyObject = false;
for (const auto &found :
getDirectlyInheritedNominalTypeDecls(next, anyObject)) {
if (auto proto = dyn_cast<ProtocolDecl>(found.Item))
protocols.push_back({proto, found.Loc, found.uncheckedLoc});
protocols.push_back(
{proto, found.Loc, found.uncheckedLoc, found.preconcurrencyLoc});
}
}

Expand Down Expand Up @@ -294,15 +300,15 @@ void ConformanceLookupTable::updateLookupTable(NominalTypeDecl *nominal,
addMacroGeneratedProtocols(
nominal, ConformanceSource::forUnexpandedMacro(nominal));
},
[&](ExtensionDecl *ext,
ArrayRef<ConformanceConstructionInfo> protos) {
[&](ExtensionDecl *ext, ArrayRef<ConformanceConstructionInfo> protos) {
// The extension decl may not be validated, so we can't use
// its inherited protocols directly.
auto source = ConformanceSource::forExplicit(ext);
for (auto locAndProto : protos)
addProtocol(
locAndProto.Item, locAndProto.Loc,
source.withUncheckedLoc(locAndProto.uncheckedLoc));
locAndProto.Item, locAndProto.Loc,
source.withUncheckedLoc(locAndProto.uncheckedLoc)
.withPreconcurrencyLoc(locAndProto.preconcurrencyLoc));
});
break;

Expand Down Expand Up @@ -495,8 +501,9 @@ void ConformanceLookupTable::addInheritedProtocols(
for (const auto &found :
getDirectlyInheritedNominalTypeDecls(decl, anyObject)) {
if (auto proto = dyn_cast<ProtocolDecl>(found.Item)) {
addProtocol(
proto, found.Loc, source.withUncheckedLoc(found.uncheckedLoc));
addProtocol(proto, found.Loc,
source.withUncheckedLoc(found.uncheckedLoc)
.withPreconcurrencyLoc(found.preconcurrencyLoc));
}
}
}
Expand Down Expand Up @@ -953,10 +960,11 @@ ConformanceLookupTable::getConformance(NominalTypeDecl *nominal,
}

// Create or find the normal conformance.
auto normalConf =
ctx.getNormalConformance(conformingType, protocol, conformanceLoc,
conformingDC, ProtocolConformanceState::Incomplete,
entry->Source.getUncheckedLoc().isValid());
auto normalConf = ctx.getNormalConformance(
conformingType, protocol, conformanceLoc, conformingDC,
ProtocolConformanceState::Incomplete,
entry->Source.getUncheckedLoc().isValid(),
entry->Source.getPreconcurrencyLoc().isValid());
// Invalid code may cause the getConformance call below to loop, so break
// the infinite recursion by setting this eagerly to shortcircuit with the
// early return at the start of this function.
Expand Down
16 changes: 16 additions & 0 deletions lib/AST/ConformanceLookupTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ class ConformanceLookupTable : public ASTAllocated<ConformanceLookupTable> {
/// The location of the "unchecked" attribute, if there is one.
SourceLoc uncheckedLoc;

/// The location of the "preconcurrency" attribute, if there is one.
SourceLoc preconcurrencyLoc;

ConformanceSource(void *ptr, ConformanceEntryKind kind)
: Storage(ptr), Kind(kind) { }

Expand Down Expand Up @@ -141,6 +144,15 @@ class ConformanceLookupTable : public ASTAllocated<ConformanceLookupTable> {
return result;
}

/// Return a new conformance source with the given location of
/// "@preconcurrency".
ConformanceSource withPreconcurrencyLoc(SourceLoc preconcurrencyLoc) {
ConformanceSource result(*this);
if (preconcurrencyLoc.isValid())
result.preconcurrencyLoc = preconcurrencyLoc;
return result;
}

/// Retrieve the kind of conformance formed from this source.
ConformanceEntryKind getKind() const { return Kind; }

Expand Down Expand Up @@ -184,6 +196,10 @@ class ConformanceLookupTable : public ASTAllocated<ConformanceLookupTable> {
return uncheckedLoc;
}

SourceLoc getPreconcurrencyLoc() const {
return preconcurrencyLoc;
}

/// For an inherited conformance, retrieve the class declaration
/// for the inheriting class.
ClassDecl *getInheritingClass() const {
Expand Down
1 change: 1 addition & 0 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1592,6 +1592,7 @@ InheritedEntry::InheritedEntry(const TypeLoc &typeLoc)
if (auto typeRepr = typeLoc.getTypeRepr()) {
isUnchecked = typeRepr->findAttrLoc(TAK_unchecked).isValid();
isRetroactive = typeRepr->findAttrLoc(TAK_retroactive).isValid();
isPreconcurrency = typeRepr->findAttrLoc(TAK_preconcurrency).isValid();
}
}

Expand Down
Loading