Skip to content

Store the conforming type within an abstract ProtocolConformanceRef #80225

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
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
2 changes: 1 addition & 1 deletion include/swift/AST/ASTBridgingImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ BridgedASTType BridgedConformance::getType() const {
}

BridgedDeclObj BridgedConformance::getRequirement() const {
return {unbridged().getRequirement()};
return {unbridged().getProtocol()};
}

BridgedConformance BridgedConformance::getGenericConformance() const {
Expand Down
69 changes: 55 additions & 14 deletions include/swift/AST/ProtocolConformanceRef.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,24 @@
#include "llvm/ADT/STLExtras.h"
#include <optional>

namespace swift {
class AbstractConformance;
}

namespace llvm {
class raw_ostream;
class raw_ostream;

template <>
struct PointerLikeTypeTraits<swift::AbstractConformance *> {
public:
static inline void *getAsVoidPointer(swift::AbstractConformance *ptr) {
return ptr;
}
static inline swift::AbstractConformance *getFromVoidPointer(void *ptr) {
return (swift::AbstractConformance *)ptr;
}
enum { NumLowBitsAvailable = swift::TypeAlignInBits };
};
}

namespace swift {
Expand All @@ -53,22 +69,27 @@ enum class EffectKind : uint8_t;
/// ProtocolConformanceRef allows the efficient recovery of the protocol
/// even when the conformance is abstract.
class ProtocolConformanceRef {
using UnionType = llvm::PointerUnion<ProtocolDecl *,
public:
using UnionType = llvm::PointerUnion<AbstractConformance *,
ProtocolConformance *,
PackConformance *>;

private:
UnionType Union;

explicit ProtocolConformanceRef(UnionType value) : Union(value) {}

public:
ProtocolConformanceRef() : Union() {}
ProtocolConformanceRef(std::nullptr_t) : Union() {}

/// Create an abstract protocol conformance reference.
explicit ProtocolConformanceRef(ProtocolDecl *proto) : Union(proto) {
assert(proto != nullptr &&
explicit ProtocolConformanceRef(AbstractConformance *abstract)
: Union(abstract) {
assert(abstract != nullptr &&
"cannot construct ProtocolConformanceRef with null");
}

public:
ProtocolConformanceRef() : Union() {}

/// Create a concrete protocol conformance reference.
explicit ProtocolConformanceRef(ProtocolConformance *conf) : Union(conf) {
assert(conf != nullptr &&
Expand All @@ -95,7 +116,7 @@ class ProtocolConformanceRef {
explicit operator bool() const { return !isInvalid(); }

/// Create an abstract conformance for a type parameter or archetype.
static ProtocolConformanceRef forAbstract(Type subjectType,
static ProtocolConformanceRef forAbstract(Type conformingType,
ProtocolDecl *protocol);

bool isConcrete() const {
Expand All @@ -115,12 +136,11 @@ class ProtocolConformanceRef {
}

bool isAbstract() const {
return !isInvalid() && Union.is<ProtocolDecl*>();
return !isInvalid() && Union.is<AbstractConformance*>();
}

ProtocolDecl *getAbstract() const {
ASSERT(isAbstract());
return Union.get<ProtocolDecl*>();
AbstractConformance *getAbstract() const {
return Union.get<AbstractConformance *>();
}

/// Determine whether this conformance (or a conformance it depends on)
Expand Down Expand Up @@ -149,7 +169,7 @@ class ProtocolConformanceRef {
/// The given `body` will be called on each isolated conformance. If it ever
/// returns `true`, this function will abort the search and return `true`.
bool forEachIsolatedConformance(
llvm::function_ref<bool(ProtocolConformance*)> body
llvm::function_ref<bool(ProtocolConformanceRef)> body
) const;

using OpaqueValue = void*;
Expand All @@ -158,8 +178,11 @@ class ProtocolConformanceRef {
return ProtocolConformanceRef(UnionType::getFromOpaqueValue(value));
}

/// Retrieve the conforming type.
Type getType() const;

/// Return the protocol requirement.
ProtocolDecl *getRequirement() const;
ProtocolDecl *getProtocol() const;

/// Apply a substitution to the conforming type.
ProtocolConformanceRef subst(Type origType, SubstitutionMap subMap,
Expand Down Expand Up @@ -240,4 +263,22 @@ SourceLoc extractNearestSourceLoc(const ProtocolConformanceRef conformanceRef);

} // end namespace swift

namespace llvm {
class raw_ostream;

template <>
struct PointerLikeTypeTraits<swift::ProtocolConformanceRef>
: PointerLikeTypeTraits<swift::ProtocolConformanceRef::UnionType>
{
public:
static inline void *getAsVoidPointer(swift::ProtocolConformanceRef ref) {
return ref.getOpaqueValue();
}
static inline swift::ProtocolConformanceRef getFromVoidPointer(void *ptr) {
return swift::ProtocolConformanceRef::getFromOpaqueValue(ptr);
}
};

}

#endif // LLVM_SWIFT_AST_PROTOCOLCONFORMANCEREF_H
2 changes: 1 addition & 1 deletion include/swift/AST/Requirement.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ class Requirement {
CheckRequirementResult checkRequirement(
SmallVectorImpl<Requirement> &subReqs,
bool allowMissing = false,
SmallVectorImpl<ProtocolConformance *> *isolatedConformances = nullptr
SmallVectorImpl<ProtocolConformanceRef> *isolatedConformances = nullptr
) const;

/// Determines if this substituted requirement can ever be satisfied,
Expand Down
2 changes: 1 addition & 1 deletion lib/APIDigester/ModuleAnalyzerNodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1498,7 +1498,7 @@ SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, ImportDecl *ID):
}

SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, ProtocolConformanceRef Conform):
SDKNodeInitInfo(Ctx, Conform.getRequirement()) {
SDKNodeInitInfo(Ctx, Conform.getProtocol()) {
// The conformance can be conditional. The generic signature keeps track of
// the requirements.
if (Conform.isConcrete()) {
Expand Down
36 changes: 35 additions & 1 deletion lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
//===----------------------------------------------------------------------===//

#include "swift/AST/ASTContext.h"
#include "AbstractConformance.h"
#include "ClangTypeConverter.h"
#include "ForeignRepresentationInfo.h"
#include "SubstitutionMapStorage.h"
Expand Down Expand Up @@ -596,6 +597,9 @@ struct ASTContext::Implementation {
/// The set of substitution maps (uniqued by their storage).
llvm::FoldingSet<SubstitutionMap::Storage> SubstitutionMaps;

/// The set of abstract conformances (uniqued by their storage).
llvm::FoldingSet<AbstractConformance> AbstractConformances;

~Arena() {
for (auto &conformance : SpecializedConformances)
conformance.~SpecializedProtocolConformance();
Expand Down Expand Up @@ -3379,6 +3383,7 @@ void ASTContext::Implementation::Arena::dump(llvm::raw_ostream &os) const {
SIZE_AND_BYTES(BuiltinConformances);
SIZE(PackConformances);
SIZE(SubstitutionMaps);
SIZE(AbstractConformances);

#undef SIZE
#undef SIZE_AND_BYTES
Expand Down Expand Up @@ -5104,7 +5109,7 @@ void SILFunctionType::Profile(
invocationSubs.profile(id);
id.AddBoolean((bool)conformance);
if (conformance)
id.AddPointer(conformance.getRequirement());
id.AddPointer(conformance.getProtocol());
}

SILFunctionType::SILFunctionType(
Expand Down Expand Up @@ -5739,6 +5744,35 @@ SubstitutionMap::Storage *SubstitutionMap::Storage::get(
return result;
}

ProtocolConformanceRef ProtocolConformanceRef::forAbstract(
Type conformingType, ProtocolDecl *proto) {
ASTContext &ctx = proto->getASTContext();

// Figure out which arena this should go in.
RecursiveTypeProperties properties;
if (conformingType)
Copy link
Contributor

Choose a reason for hiding this comment

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

Where are the remaining callers that pass a null Type?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know of any, but I want the assertions in place before I take this out.

properties |= conformingType->getRecursiveProperties();
auto arena = getArena(properties);

// Profile the substitution map.
llvm::FoldingSetNodeID id;
AbstractConformance::Profile(id, conformingType, proto);

// Did we already record this abstract conformance?
void *insertPos;
auto &abstractConformances =
ctx.getImpl().getArena(arena).AbstractConformances;
if (auto result = abstractConformances.FindNodeOrInsertPos(id, insertPos))
return ProtocolConformanceRef(result);

// Allocate and record this abstract conformance.
auto mem = ctx.Allocate(sizeof(AbstractConformance),
alignof(AbstractConformance), arena);
auto result = new (mem) AbstractConformance(conformingType, proto);
abstractConformances.InsertNode(result, insertPos);
return ProtocolConformanceRef(result);
}

const AvailabilityContext::Storage *AvailabilityContext::Storage::get(
const AvailabilityRange &platformRange, bool isDeprecated,
llvm::ArrayRef<DomainInfo> domainInfos, const ASTContext &ctx) {
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/ASTDumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5519,7 +5519,7 @@ class PrintConformance : public PrintBase {
assert(conformance.isAbstract());

printHead("abstract_conformance", ASTNodeColor, label);
printReferencedDeclField(conformance.getAbstract(),
printReferencedDeclField(conformance.getProtocol(),
Label::always("protocol"));
printFoot();
}
Expand Down
12 changes: 7 additions & 5 deletions lib/AST/ASTMangler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2170,7 +2170,7 @@ void ASTMangler::appendRetroactiveConformances(SubstitutionMap subMap,
if (conformance.isInvalid())
continue;

if (conformance.getRequirement()->isMarkerProtocol())
if (conformance.getProtocol()->isMarkerProtocol())
continue;

SWIFT_DEFER {
Expand Down Expand Up @@ -4528,7 +4528,7 @@ void ASTMangler::appendAnyProtocolConformance(
// If we have a conformance to a marker protocol but we aren't allowed to
// emit marker protocols, skip it.
if (!AllowMarkerProtocols &&
conformance.getRequirement()->isMarkerProtocol())
conformance.getProtocol()->isMarkerProtocol())
return;

// While all invertible protocols are marker protocols, do not mangle them
Expand All @@ -4537,15 +4537,17 @@ void ASTMangler::appendAnyProtocolConformance(
// but we *might* have let that slip by for the other cases below, so the
// early-exits are highly conservative.
const bool forInvertible =
conformance.getRequirement()->getInvertibleProtocolKind().has_value();
conformance.getProtocol()->getInvertibleProtocolKind().has_value();

if (conformingType->isTypeParameter()) {
assert(genericSig && "Need a generic signature to resolve conformance");
if (forInvertible)
return;

// FIXME: conformingType parameter should no longer be needed, because
// its in conformance.
auto path = genericSig->getConformancePath(conformingType,
conformance.getAbstract());
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add a FIXME here to remind ourselves that the conformingType parameter to this function is no longer necessary

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure.

conformance.getProtocol());
appendDependentProtocolConformance(path, genericSig);
} else if (auto opaqueType = conformingType->getAs<OpaqueTypeArchetypeType>()) {
if (forInvertible)
Expand All @@ -4556,7 +4558,7 @@ void ASTMangler::appendAnyProtocolConformance(
ConformancePath conformancePath =
opaqueSignature->getConformancePath(
opaqueType->getInterfaceType(),
conformance.getAbstract());
conformance.getProtocol());

// Append the conformance path with the signature of the opaque type.
appendDependentProtocolConformance(conformancePath, opaqueSignature);
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/ASTPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6634,7 +6634,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
case SILFunctionType::Representation::WitnessMethod:
Printer << "witness_method: ";
printTypeDeclName(
witnessMethodConformance.getRequirement()->getDeclaredType()
witnessMethodConformance.getProtocol()->getDeclaredType()
->castTo<ProtocolType>());
break;
case SILFunctionType::Representation::Closure:
Expand Down
6 changes: 3 additions & 3 deletions lib/AST/ASTVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1785,7 +1785,7 @@ class Verifier : public ASTWalker {
for (auto proto : erasedLayout.getProtocols()) {
if (std::find_if(conformances.begin(), conformances.end(),
[&](ProtocolConformanceRef ref) -> bool {
return ref.getRequirement() == proto->getDecl();
return ref.getProtocol() == proto->getDecl();
})
== conformances.end()) {
Out << "ErasureExpr is missing conformance for required protocol\n";
Expand Down Expand Up @@ -1819,7 +1819,7 @@ class Verifier : public ASTWalker {
anyHashableDecl->getDeclaredInterfaceType(),
"AnyHashableErasureExpr and the standard AnyHashable type");

if (E->getConformance().getRequirement() != hashableDecl) {
if (E->getConformance().getProtocol() != hashableDecl) {
Out << "conformance on AnyHashableErasureExpr was not for Hashable\n";
E->getConformance().dump(Out);
abort();
Expand Down Expand Up @@ -2811,7 +2811,7 @@ class Verifier : public ASTWalker {
if (!type->is<ArchetypeType>() && !type->isAnyExistentialType()) {
Out << "type " << type
<< " should not have an abstract conformance to "
<< conformance.getRequirement()->getName();
<< conformance.getProtocol()->getName();
abort();
}

Expand Down
51 changes: 51 additions & 0 deletions lib/AST/AbstractConformance.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
//===--- AbstractConformance.h - Abstract conformance storage ---*- C++ -*-===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//
//
// This file defines the AbstractConformance class, which stores an
// abstract conformance that is stashed in a ProtocolConformanceRef.
//
//===----------------------------------------------------------------------===//
#ifndef SWIFT_AST_ABSTRACT_CONFORMANCE_H
#define SWIFT_AST_ABSTRACT_CONFORMANCE_H

#include "swift/AST/Type.h"
#include "llvm/ADT/FoldingSet.h"

namespace swift {
class AbstractConformance final : public llvm::FoldingSetNode {
Type conformingType;
ProtocolDecl *requirement;

public:
AbstractConformance(Type conformingType, ProtocolDecl *requirement)
: conformingType(conformingType), requirement(requirement) { }

Type getType() const { return conformingType; }
ProtocolDecl *getProtocol() const { return requirement; }

void Profile(llvm::FoldingSetNodeID &id) const {
Profile(id, getType(), getProtocol());
}

/// Profile the substitution map storage, for use with LLVM's FoldingSet.
static void Profile(llvm::FoldingSetNodeID &id,
Type conformingType,
ProtocolDecl *requirement) {
id.AddPointer(conformingType.getPointer());
id.AddPointer(requirement);
}
};

}

#endif // SWIFT_AST_ABSTRACT_CONFORMANCE_H

2 changes: 1 addition & 1 deletion lib/AST/Effects.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ void swift::simple_display(llvm::raw_ostream &out,

bool ProtocolConformanceRef::hasEffect(EffectKind kind) const {
if (!isConcrete()) { return kind != EffectKind::Unsafe; }
return evaluateOrDefault(getRequirement()->getASTContext().evaluator,
return evaluateOrDefault(getProtocol()->getASTContext().evaluator,
ConformanceHasEffectRequest{kind, getConcrete()},
true);
}
Loading