Skip to content

Deterministically order class members for emission #32581

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
7 changes: 4 additions & 3 deletions include/swift/AST/ASTMangler.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ class ASTMangler : public Mangler {
SymbolKind SKind = SymbolKind::Default);

std::string mangleDestructorEntity(const DestructorDecl *decl,
bool isDeallocating, SymbolKind SKind);
bool isDeallocating,
SymbolKind SKind = SymbolKind::Default);

std::string mangleConstructorEntity(const ConstructorDecl *ctor,
bool isAllocating,
Expand All @@ -120,11 +121,11 @@ class ASTMangler : public Mangler {

std::string mangleDefaultArgumentEntity(const DeclContext *func,
unsigned index,
SymbolKind SKind);
SymbolKind SKind = SymbolKind::Default);

std::string mangleInitializerEntity(const VarDecl *var, SymbolKind SKind);
std::string mangleBackingInitializerEntity(const VarDecl *var,
SymbolKind SKind);
SymbolKind SKind = SymbolKind::Default);

std::string mangleNominalType(const NominalTypeDecl *decl);

Expand Down
15 changes: 2 additions & 13 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -535,17 +535,14 @@ class alignas(1 << DeclAlignInBits) Decl {
NumRequirementsInSignature : 16
);

SWIFT_INLINE_BITFIELD(ClassDecl, NominalTypeDecl, 1+1+2+1+1+1+1+1+1,
SWIFT_INLINE_BITFIELD(ClassDecl, NominalTypeDecl, 1+1+2+1+1+1+1+1,
/// Whether this class inherits its superclass's convenience initializers.
InheritsSuperclassInits : 1,
ComputedInheritsSuperclassInits : 1,

/// \see ClassDecl::ForeignKind
RawForeignKind : 2,

/// \see ClassDecl::getEmittedMembers()
HasForcedEmittedMembers : 1,

HasMissingDesignatedInitializers : 1,
ComputedHasMissingDesignatedInitializers : 1,

Expand Down Expand Up @@ -3856,14 +3853,6 @@ class ClassDecl final : public NominalTypeDecl {
llvm::PointerIntPair<Type, 1, bool> SuperclassType;
} LazySemanticInfo;

bool hasForcedEmittedMembers() const {
return Bits.ClassDecl.HasForcedEmittedMembers;
}

void setHasForcedEmittedMembers() {
Bits.ClassDecl.HasForcedEmittedMembers = true;
}

Optional<bool> getCachedInheritsSuperclassInitializers() const {
if (Bits.ClassDecl.ComputedInheritsSuperclassInits)
return Bits.ClassDecl.InheritsSuperclassInits;
Expand Down Expand Up @@ -4089,7 +4078,7 @@ class ClassDecl final : public NominalTypeDecl {

/// Get all the members of this class, synthesizing any implicit members
/// that appear in the vtable if needed.
DeclRange getEmittedMembers() const;
ArrayRef<Decl *> getEmittedMembers() const;

// Implement isa/cast/dyncast/etc.
static bool classof(const Decl *D) {
Expand Down
9 changes: 3 additions & 6 deletions include/swift/AST/TypeCheckRequests.h
Original file line number Diff line number Diff line change
Expand Up @@ -1064,23 +1064,20 @@ class SynthesizeAccessorRequest :

class EmittedMembersRequest :
public SimpleRequest<EmittedMembersRequest,
DeclRange(ClassDecl *),
RequestFlags::SeparatelyCached> {
ArrayRef<Decl *>(ClassDecl *),
RequestFlags::Cached> {
public:
using SimpleRequest::SimpleRequest;

private:
friend SimpleRequest;

// Evaluation.
DeclRange
ArrayRef<Decl *>
evaluate(Evaluator &evaluator, ClassDecl *classDecl) const;

public:
// Separate caching.
bool isCached() const { return true; }
Optional<DeclRange> getCachedResult() const;
void cacheResult(DeclRange value) const;
};

class IsImplicitlyUnwrappedOptionalRequest :
Expand Down
4 changes: 2 additions & 2 deletions include/swift/AST/TypeCheckerTypeIDZone.def
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ SWIFT_REQUEST(TypeChecker, TypeEraserHasViableInitRequest,
SWIFT_REQUEST(TypeChecker, DynamicallyReplacedDeclRequest,
ValueDecl *(ValueDecl *),
Cached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, EmittedMembersRequest, DeclRange(ClassDecl *),
SeparatelyCached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, EmittedMembersRequest, ArrayRef<Decl *>(ClassDecl *),
Cached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, EnumRawValuesRequest,
evaluator::SideEffect (EnumDecl *, TypeResolutionStage),
SeparatelyCached, NoLocationInfo)
Expand Down
12 changes: 9 additions & 3 deletions include/swift/Demangling/ManglingUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,8 @@ void mangleIdentifier(Mangler &M, StringRef ident) {
// Mangle the sub-string up to the next word substitution (or to the end
// of the identifier - that's why we added the dummy-word).
// The first thing: we add the encoded sub-string length.
bool first = true;
M.Buffer << (Repl.StringPos - Pos);
assert(!isDigit(ident[Pos]) &&
"first char of sub-string may not be a digit");
do {
// Update the start position of new added words, so that they refer to
// the begin of the whole mangled Buffer.
Expand All @@ -203,9 +202,16 @@ void mangleIdentifier(Mangler &M, StringRef ident) {
M.Words[WordsInBuffer].start = M.getBufferStr().size();
WordsInBuffer++;
}
// Error recovery. We sometimes need to mangle identifiers coming
// from invalid code.
if (first && isDigit(ident[Pos]))
M.Buffer << 'X';
// Add a literal character of the sub-string.
M.Buffer << ident[Pos];
else
M.Buffer << ident[Pos];

Pos++;
first = false;
} while (Pos < Repl.StringPos);
}
// Is it a "real" word substitution (and not the dummy-word)?
Expand Down
71 changes: 1 addition & 70 deletions include/swift/SIL/SILVTableVisitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,55 +18,11 @@
#ifndef SWIFT_SIL_SILVTABLEVISITOR_H
#define SWIFT_SIL_SILVTABLEVISITOR_H

#include <string>

#include "swift/AST/Decl.h"
#include "swift/AST/Types.h"
#include "swift/AST/ASTMangler.h"

namespace swift {

// Utility class for deterministically ordering vtable entries for
// synthesized methods.
struct SortedFuncList {
using Entry = std::pair<std::string, AbstractFunctionDecl *>;
SmallVector<Entry, 2> elts;
bool sorted = false;

void add(AbstractFunctionDecl *afd) {
Mangle::ASTMangler mangler;
std::string mangledName;
if (auto *cd = dyn_cast<ConstructorDecl>(afd))
mangledName = mangler.mangleConstructorEntity(cd, 0);
else
mangledName = mangler.mangleEntity(afd);

elts.push_back(std::make_pair(mangledName, afd));
}

bool empty() { return elts.empty(); }

void sort() {
assert(!sorted);
sorted = true;
std::sort(elts.begin(),
elts.end(),
[](const Entry &lhs, const Entry &rhs) -> bool {
return lhs.first < rhs.first;
});
}

decltype(elts)::const_iterator begin() const {
assert(sorted);
return elts.begin();
}

decltype(elts)::const_iterator end() const {
assert(sorted);
return elts.end();
}
};

/// A CRTP class for visiting virtually-dispatched methods of a class.
///
/// You must override these two methods in your subclass:
Expand Down Expand Up @@ -192,33 +148,8 @@ template <class T> class SILVTableVisitor {
if (!theClass->hasKnownSwiftImplementation())
return;

// Note that while vtable order is not ABI, we still want it to be
// consistent between translation units.
//
// So, sort synthesized members by their mangled name, since they
// are added lazily during type checking, with the remaining ones
// forced at the end.
SortedFuncList synthesizedMembers;

for (auto member : theClass->getEmittedMembers()) {
if (auto *afd = dyn_cast<AbstractFunctionDecl>(member)) {
if (afd->isSynthesized()) {
synthesizedMembers.add(afd);
continue;
}
}

for (auto member : theClass->getEmittedMembers())
maybeAddMember(member);
}

if (synthesizedMembers.empty())
return;

synthesizedMembers.sort();

for (const auto &pair : synthesizedMembers) {
maybeAddMember(pair.second);
}
}
};

Expand Down
2 changes: 1 addition & 1 deletion lib/AST/ASTMangler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2741,7 +2741,7 @@ void ASTMangler::appendConstructorEntity(const ConstructorDecl *ctor,
}

void ASTMangler::appendDestructorEntity(const DestructorDecl *dtor,
bool isDeallocating) {
bool isDeallocating) {
appendContextOf(dtor);
appendOperator(isDeallocating ? "fD" : "fd");
}
Expand Down
8 changes: 6 additions & 2 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4285,11 +4285,11 @@ DestructorDecl *ClassDecl::getDestructor() const {
nullptr);
}

DeclRange ClassDecl::getEmittedMembers() const {
ArrayRef<Decl *> ClassDecl::getEmittedMembers() const {
ASTContext &ctx = getASTContext();
return evaluateOrDefault(ctx.evaluator,
EmittedMembersRequest{const_cast<ClassDecl *>(this)},
getMembers());
ArrayRef<Decl *>());
}

/// Synthesizer callback for an empty implicit function body.
Expand Down Expand Up @@ -4318,6 +4318,10 @@ GetDestructorRequest::evaluate(Evaluator &evaluator, ClassDecl *CD) const {
if (ctx.LangOpts.EnableObjCInterop)
CD->recordObjCMethod(DD, DD->getObjCSelector());

// Mark it as synthesized to make its location in getEmittedMembers()
// deterministic.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice "why" comment!

DD->setSynthesized(true);

return DD;
}

Expand Down
17 changes: 0 additions & 17 deletions lib/AST/TypeCheckRequests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -783,23 +783,6 @@ void SynthesizeAccessorRequest::cacheResult(AccessorDecl *accessor) const {
storage->setSynthesizedAccessor(kind, accessor);
}

//----------------------------------------------------------------------------//
// EmittedMembersRequest computation.
//----------------------------------------------------------------------------//

Optional<DeclRange>
EmittedMembersRequest::getCachedResult() const {
auto *classDecl = std::get<0>(getStorage());
if (classDecl->hasForcedEmittedMembers())
return classDecl->getMembers();
return None;
}

void EmittedMembersRequest::cacheResult(DeclRange result) const {
auto *classDecl = std::get<0>(getStorage());
classDecl->setHasForcedEmittedMembers();
}

//----------------------------------------------------------------------------//
// IsImplicitlyUnwrappedOptionalRequest computation.
//----------------------------------------------------------------------------//
Expand Down
Loading