Skip to content

[SE-0280] Enum cases as protocol witnesses #28916

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Mar 28, 2020
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
8 changes: 6 additions & 2 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1881,8 +1881,9 @@ ERROR(requirement_restricts_self,none,
"'Self'",
(DescriptiveDeclKind, DeclName, StringRef, unsigned, StringRef))
ERROR(witness_argument_name_mismatch,none,
"%select{method|initializer}0 %1 has different argument labels from those "
"required by protocol %2 (%3)", (bool, DeclName, Type, DeclName))
"%0 %1 has different argument labels "
"from those required by protocol %2 (%3)",
(DescriptiveDeclKind, DeclName, Type, DeclName))
ERROR(witness_initializer_not_required,none,
"initializer requirement %0 can only be satisfied by a 'required' "
"initializer in%select{| the definition of}1 non-final class %2",
Expand Down Expand Up @@ -2108,6 +2109,9 @@ NOTE(protocol_witness_throws_conflict,none,
"candidate throws, but protocol does not allow it", ())
NOTE(protocol_witness_not_objc,none,
"candidate is explicitly '@nonobjc'", ())
NOTE(protocol_witness_enum_case_payload, none,
"candidate is an enum case with associated values, "
"but protocol does not allow it", ())

NOTE(protocol_witness_type,none,
"possibly intended match", ())
Expand Down
4 changes: 4 additions & 0 deletions include/swift/SILOptimizer/Utils/InstOptUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,10 @@ bool calleesAreStaticallyKnowable(SILModule &module, SILDeclRef decl);
/// be reached by calling the function represented by Decl?
bool calleesAreStaticallyKnowable(SILModule &module, AbstractFunctionDecl *afd);

/// Do we have enough information to determine all callees that could
/// be reached by calling the function represented by Decl?
bool calleesAreStaticallyKnowable(SILModule &module, EnumElementDecl *eed);

// Attempt to get the instance for , whose static type is the same as
// its exact dynamic type, returning a null SILValue() if we cannot find it.
// The information that a static type is the same as the exact dynamic,
Expand Down
2 changes: 1 addition & 1 deletion lib/SIL/SILDeclRef.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ SILLinkage SILDeclRef::getLinkage(ForDefinition_t forDefinition) const {
limit = Limit::OnDemand;
}
}

auto effectiveAccess = d->getEffectiveAccess();

// Private setter implementations for an internal storage declaration should
Expand Down
29 changes: 27 additions & 2 deletions lib/SILGen/SILGenType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -387,11 +387,25 @@ template<typename T> class SILGenWitnessTable : public SILWitnessVisitor<T> {

public:
void addMethod(SILDeclRef requirementRef) {
auto reqAccessor = dyn_cast<AccessorDecl>(requirementRef.getDecl());
auto reqDecl = requirementRef.getDecl();

// Static functions can be witnessed by enum cases with payload
if (!(isa<AccessorDecl>(reqDecl) || isa<ConstructorDecl>(reqDecl))) {
auto FD = cast<FuncDecl>(reqDecl);
if (auto witness = asDerived().getWitness(FD)) {
if (auto EED = dyn_cast<EnumElementDecl>(witness.getDecl())) {
return addMethodImplementation(
requirementRef, SILDeclRef(EED, SILDeclRef::Kind::EnumElement),
witness);
}
}
}

auto reqAccessor = dyn_cast<AccessorDecl>(reqDecl);

// If it's not an accessor, just look for the witness.
if (!reqAccessor) {
if (auto witness = asDerived().getWitness(requirementRef.getDecl())) {
if (auto witness = asDerived().getWitness(reqDecl)) {
return addMethodImplementation(
requirementRef, requirementRef.withDecl(witness.getDecl()),
witness);
Expand All @@ -406,6 +420,13 @@ template<typename T> class SILGenWitnessTable : public SILWitnessVisitor<T> {
if (!witness)
return asDerived().addMissingMethod(requirementRef);

// Static properties can be witnessed by enum cases without payload
if (auto EED = dyn_cast<EnumElementDecl>(witness.getDecl())) {
return addMethodImplementation(
requirementRef, SILDeclRef(EED, SILDeclRef::Kind::EnumElement),
witness);
}

auto witnessStorage = cast<AbstractStorageDecl>(witness.getDecl());
if (reqAccessor->isSetter() && !witnessStorage->supportsMutation())
return asDerived().addMissingMethod(requirementRef);
Expand Down Expand Up @@ -566,6 +587,10 @@ class SILGenConformance : public SILGenWitnessTable<SILGenConformance> {
witnessLinkage = SILLinkage::Shared;
}

if (isa<EnumElementDecl>(witnessRef.getDecl())) {
assert(witnessRef.isEnumElement() && "Witness decl, but different kind?");
}

SILFunction *witnessFn = SGM.emitProtocolWitness(
ProtocolConformanceRef(Conformance), witnessLinkage, witnessSerialized,
requirementRef, witnessRef, isFree, witness);
Expand Down
40 changes: 40 additions & 0 deletions lib/SILOptimizer/Utils/InstOptUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1797,6 +1797,11 @@ bool swift::calleesAreStaticallyKnowable(SILModule &module, SILDeclRef decl) {
if (decl.isForeign)
return false;

if (decl.isEnumElement()) {
return calleesAreStaticallyKnowable(module,
cast<EnumElementDecl>(decl.getDecl()));
}

auto *afd = decl.getAbstractFunctionDecl();
assert(afd && "Expected abstract function decl!");
return calleesAreStaticallyKnowable(module, afd);
Expand Down Expand Up @@ -1845,6 +1850,41 @@ bool swift::calleesAreStaticallyKnowable(SILModule &module,
llvm_unreachable("Unhandled access level in switch.");
}

/// Are the callees that could be called through Decl statically
/// knowable based on the Decl and the compilation mode?
// FIXME: Merge this with calleesAreStaticallyKnowable above
bool swift::calleesAreStaticallyKnowable(SILModule &module,
EnumElementDecl *eed) {
const DeclContext *assocDC = module.getAssociatedContext();
if (!assocDC)
return false;

// Only handle members defined within the SILModule's associated context.
if (!eed->isChildContextOf(assocDC))
return false;

if (eed->isDynamic()) {
return false;
}

if (!eed->hasAccess())
return false;

// Only consider 'private' members, unless we are in whole-module compilation.
switch (eed->getEffectiveAccess()) {
case AccessLevel::Open:
return false;
case AccessLevel::Public:
case AccessLevel::Internal:
return module.isWholeModule();
case AccessLevel::FilePrivate:
case AccessLevel::Private:
return true;
}

llvm_unreachable("Unhandled access level in switch.");
}

Optional<FindLocalApplySitesResult>
swift::findLocalApplySites(FunctionRefBaseInst *fri) {
SmallVector<Operand *, 32> worklist(fri->use_begin(), fri->use_end());
Expand Down
51 changes: 40 additions & 11 deletions lib/Sema/TypeCheckProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -479,8 +479,20 @@ swift::matchWitness(
assert(!req->isInvalid() && "Cannot have an invalid requirement here");

/// Make sure the witness is of the same kind as the requirement.
if (req->getKind() != witness->getKind())
return RequirementMatch(witness, MatchKind::KindConflict);
if (req->getKind() != witness->getKind()) {
// An enum case can witness:
// 1. A static get-only property requirement, as long as the property's
// type is `Self` or it matches the type of the enum explicitly.
// 2. A static function requirement, if the enum case has a payload
// and the payload types and labels match the function and the
// function returns `Self` or the type of the enum.
//
// If there are any discrepencies, we'll diagnose it later. For now,
// let's assume the match is valid.
if (!((isa<VarDecl>(req) || isa<FuncDecl>(req)) &&
isa<EnumElementDecl>(witness)))
return RequirementMatch(witness, MatchKind::KindConflict);
}

// If we're currently validating the witness, bail out.
if (witness->isRecursiveValidation())
Expand All @@ -497,7 +509,8 @@ swift::matchWitness(
// Perform basic matching of the requirement and witness.
bool decomposeFunctionType = false;
bool ignoreReturnType = false;
if (auto funcReq = dyn_cast<FuncDecl>(req)) {
if (isa<FuncDecl>(req) && isa<FuncDecl>(witness)) {
auto funcReq = cast<FuncDecl>(req);
auto funcWitness = cast<FuncDecl>(witness);

// Either both must be 'static' or neither.
Expand Down Expand Up @@ -559,6 +572,18 @@ swift::matchWitness(
} else if (isa<ConstructorDecl>(witness)) {
decomposeFunctionType = true;
ignoreReturnType = true;
} else if (isa<EnumElementDecl>(witness)) {
auto enumCase = cast<EnumElementDecl>(witness);
if (enumCase->hasAssociatedValues() && isa<VarDecl>(req))
return RequirementMatch(witness, MatchKind::EnumCaseWithAssociatedValues);
auto isValid = isa<VarDecl>(req) || isa<FuncDecl>(req);
if (!isValid)
return RequirementMatch(witness, MatchKind::KindConflict);
if (!cast<ValueDecl>(req)->isStatic())
return RequirementMatch(witness, MatchKind::StaticNonStaticConflict);
if (isa<VarDecl>(req) &&
cast<VarDecl>(req)->getParsedAccessor(AccessorKind::Set))
return RequirementMatch(witness, MatchKind::SettableConflict);
}

// If the requirement is @objc, the witness must not be marked with @nonobjc.
Expand Down Expand Up @@ -2177,7 +2202,8 @@ diagnoseMatch(ModuleDecl *module, NormalProtocolConformance *conformance,
if (match.Kind != MatchKind::RenamedMatch &&
!match.Witness->getAttrs().hasAttribute<ImplementsAttr>() &&
match.Witness->getFullName() &&
req->getFullName() != match.Witness->getFullName())
req->getFullName() != match.Witness->getFullName() &&
!isa<EnumElementDecl>(match.Witness))
return;

// Form a string describing the associated type deductions.
Expand Down Expand Up @@ -2229,7 +2255,7 @@ diagnoseMatch(ModuleDecl *module, NormalProtocolConformance *conformance,
break;

case MatchKind::TypeConflict: {
if (!isa<TypeDecl>(req)) {
if (!isa<TypeDecl>(req) && !isa<EnumElementDecl>(match.Witness)) {
computeFixitsForOverridenDeclaration(match.Witness, req, [&](bool){
return diags.diagnose(match.Witness,
diag::protocol_witness_type_conflict,
Expand Down Expand Up @@ -2273,6 +2299,8 @@ diagnoseMatch(ModuleDecl *module, NormalProtocolConformance *conformance,
auto witness = match.Witness;
auto diag = diags.diagnose(witness, diag::protocol_witness_static_conflict,
!req->isInstanceMember());
if (isa<EnumElementDecl>(witness))
break;
if (req->isInstanceMember()) {
SourceLoc loc;
if (auto FD = dyn_cast<FuncDecl>(witness)) {
Expand Down Expand Up @@ -2398,6 +2426,9 @@ diagnoseMatch(ModuleDecl *module, NormalProtocolConformance *conformance,
}
break;
}
case MatchKind::EnumCaseWithAssociatedValues:
diags.diagnose(match.Witness, diag::protocol_witness_enum_case_payload);
break;
}
}

Expand Down Expand Up @@ -3320,12 +3351,10 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
auto &diags = proto->getASTContext().Diags;
{
SourceLoc diagLoc = getLocForDiagnosingWitness(conformance,witness);
auto diag = diags.diagnose(diagLoc,
diag::witness_argument_name_mismatch,
isa<ConstructorDecl>(witness),
witness->getFullName(),
proto->getDeclaredType(),
requirement->getFullName());
auto diag = diags.diagnose(
diagLoc, diag::witness_argument_name_mismatch,
witness->getDescriptiveKind(), witness->getFullName(),
proto->getDeclaredType(), requirement->getFullName());
if (diagLoc == witness->getLoc()) {
fixDeclarationName(diag, witness, requirement->getFullName());
} else {
Expand Down
6 changes: 6 additions & 0 deletions lib/Sema/TypeCheckProtocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,10 @@ enum class MatchKind : uint8_t {

/// The witness is missing a `@differentiable` attribute from the requirement.
MissingDifferentiableAttr,

/// The witness did not match because it is an enum case with
/// associated values.
EnumCaseWithAssociatedValues,
};

/// Describes the kind of optional adjustment performed when
Expand Down Expand Up @@ -437,6 +441,7 @@ struct RequirementMatch {
case MatchKind::ThrowsConflict:
case MatchKind::NonObjC:
case MatchKind::MissingDifferentiableAttr:
case MatchKind::EnumCaseWithAssociatedValues:
return false;
}

Expand Down Expand Up @@ -467,6 +472,7 @@ struct RequirementMatch {
case MatchKind::ThrowsConflict:
case MatchKind::NonObjC:
case MatchKind::MissingDifferentiableAttr:
case MatchKind::EnumCaseWithAssociatedValues:
return false;
}

Expand Down
32 changes: 17 additions & 15 deletions lib/TBDGen/TBDGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -494,21 +494,23 @@ void TBDGenVisitor::addConformances(DeclContext *DC) {
}
};

rootConformance->forEachValueWitness(
[&](ValueDecl *valueReq, Witness witness) {
auto witnessDecl = witness.getDecl();
if (isa<AbstractFunctionDecl>(valueReq)) {
addSymbolIfNecessary(valueReq, witnessDecl);
} else if (auto *storage = dyn_cast<AbstractStorageDecl>(valueReq)) {
auto witnessStorage = cast<AbstractStorageDecl>(witnessDecl);
storage->visitOpaqueAccessors([&](AccessorDecl *reqtAccessor) {
auto witnessAccessor =
witnessStorage->getSynthesizedAccessor(
reqtAccessor->getAccessorKind());
addSymbolIfNecessary(reqtAccessor, witnessAccessor);
});
}
});
rootConformance->forEachValueWitness([&](ValueDecl *valueReq,
Witness witness) {
auto witnessDecl = witness.getDecl();
if (isa<AbstractFunctionDecl>(valueReq)) {
addSymbolIfNecessary(valueReq, witnessDecl);
} else if (auto *storage = dyn_cast<AbstractStorageDecl>(valueReq)) {
if (auto witnessStorage = dyn_cast<AbstractStorageDecl>(witnessDecl)) {
storage->visitOpaqueAccessors([&](AccessorDecl *reqtAccessor) {
auto witnessAccessor = witnessStorage->getSynthesizedAccessor(
reqtAccessor->getAccessorKind());
addSymbolIfNecessary(reqtAccessor, witnessAccessor);
});
} else if (isa<EnumElementDecl>(witnessDecl)) {
addSymbolIfNecessary(valueReq, witnessDecl);
Copy link
Contributor

Choose a reason for hiding this comment

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

In practice, enum constructors never have public linkage so they don't need to be visited by TBDGen. However in case this ever changes, there's one less spot to update...

}
}
});
}
}

Expand Down
49 changes: 49 additions & 0 deletions test/SILGen/protocol_enum_witness.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// RUN: %target-swift-emit-silgen %s | %FileCheck %s

protocol Foo {
static var button: Self { get }
}

enum Bar: Foo {
case button
}

protocol AnotherFoo {
static func bar(arg: Int) -> Self
}

enum AnotherBar: AnotherFoo {
case bar(arg: Int)
}

// CHECK-LABEL: sil private [transparent] [thunk] [ossa] @$s21protocol_enum_witness3BarOAA3FooA2aDP6buttonxvgZTW : $@convention(witness_method: Foo) (@thick Bar.Type) -> @out Bar {
// CHECK: bb0([[BAR:%.*]] : $*Bar, [[BAR_TYPE:%.*]] : $@thick Bar.Type):
// CHECK-NEXT: [[META_TYPE:%.*]] = metatype $@thin Bar.Type
// CHECK: [[REF:%.*]] = function_ref @$s21protocol_enum_witness3BarO6buttonyA2CmF : $@convention(method) (@thin Bar.Type) -> Bar
// CHECK-NEXT: [[RESULT:%.*]] = apply [[REF]]([[META_TYPE]]) : $@convention(method) (@thin Bar.Type) -> Bar
// CHECK-NEXT: store [[RESULT]] to [trivial] [[BAR]] : $*Bar
// CHECK-NEXT: [[TUPLE:%.*]] = tuple ()
// CHECK-NEXT: return [[TUPLE]] : $()
// CHECK-END: }

// CHECK-LABEL: sil hidden [transparent] [ossa] @$s21protocol_enum_witness3BarO6buttonyA2CmF : $@convention(method) (@thin Bar.Type) -> Bar {
// CHECK: bb0({{%.*}} : $@thin Bar.Type):
// CHECK-NEXT: [[CASE:%.*]] = enum $Bar, #Bar.button!enumelt
// CHECK-NEXT: return [[CASE]] : $Bar
// CHECK-END: }

// CHECK-LABEL: sil private [transparent] [thunk] [ossa] @$s21protocol_enum_witness10AnotherBarOAA0D3FooA2aDP3bar3argxSi_tFZTW : $@convention(witness_method: AnotherFoo) (Int, @thick AnotherBar.Type) -> @out AnotherBar {
// CHECK: bb0([[ANOTHER_BAR:%.*]] : $*AnotherBar, [[INT_ARG:%.*]] : $Int, [[ANOTHER_BAR_TYPE:%.*]] : $@thick AnotherBar.Type):
// CHECK-NEXT: [[META_TYPE:%.*]] = metatype $@thin AnotherBar.Type
// CHECK: [[REF:%.*]] = function_ref @$s21protocol_enum_witness10AnotherBarO3baryACSi_tcACmF : $@convention(method) (Int, @thin AnotherBar.Type) -> AnotherBar
// CHECK-NEXT: [[RESULT:%.*]] = apply [[REF]]([[INT_ARG]], [[META_TYPE]]) : $@convention(method) (Int, @thin AnotherBar.Type) -> AnotherBar
// CHECK-NEXT: store [[RESULT]] to [trivial] [[ANOTHER_BAR]] : $*AnotherBar
// CHECK-NEXT: [[TUPLE:%.*]] = tuple ()
// CHECK-NEXT: return [[TUPLE]] : $()
// CHECK-END: }

// CHECK-LABEL: sil_witness_table hidden Bar: Foo module protocol_enum_witness {
// CHECK: method #Foo.button!getter: <Self where Self : Foo> (Self.Type) -> () -> Self : @$s21protocol_enum_witness3BarOAA3FooA2aDP6buttonxvgZTW

// CHECK-LABEL: sil_witness_table hidden AnotherBar: AnotherFoo module protocol_enum_witness {
// CHECK: method #AnotherFoo.bar: <Self where Self : AnotherFoo> (Self.Type) -> (Int) -> Self : @$s21protocol_enum_witness10AnotherBarOAA0D3FooA2aDP3bar3argxSi_tFZTW
Loading