Skip to content

[NFC] Remove some dead variadic tuple code #33059

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.

Sign up for GitHub

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 7 commits into from
Jul 23, 2020
Merged

[NFC] Remove some dead variadic tuple code #33059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8c9d0a1
[ASTDemangler] Remove 'isVariadic' parameter from 'createTupleType'
theblixguy Jul 22, 2020
d4e667e
[TypeDecoder] Remove 'variadic' argument from 'createTupleType' call
theblixguy Jul 22, 2020
b4aa326
[Reflection] Remove support for variadic from TupleTypeRef
theblixguy Jul 22, 2020
899ff07
[Remote] Remove 'variadic' argument from 'createTupleType' call
theblixguy Jul 22, 2020
81760d3
[Runtime] Remove 'variadic' parameter from createTupleType in Metadat…
theblixguy Jul 22, 2020
576645c
[Test] Remove variadic tuple typeref tests
theblixguy Jul 22, 2020
60339e5
[Reflection] Restore accidentally deleted code from 'visitFunctionTyp…
theblixguy Jul 22, 2020
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
3 changes: 1 addition & 2 deletions include/swift/AST/ASTDemangler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ class ASTBuilder {
Type createBoundGenericType(GenericTypeDecl *decl, ArrayRef<Type> args,
Type parent);

Type createTupleType(ArrayRef<Type> eltTypes, StringRef labels,
bool isVariadic);
Type createTupleType(ArrayRef<Type> eltTypes, StringRef labels);

Type createFunctionType(ArrayRef<Demangle::FunctionParam<Type>> params,
Type output, FunctionTypeFlags flags);
Expand Down
3 changes: 1 addition & 2 deletions include/swift/Demangling/TypeDecoder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -711,7 +711,6 @@ class TypeDecoder {
case NodeKind::Tuple: {
llvm::SmallVector<BuiltType, 8> elements;
std::string labels;
bool variadic = false;
for (auto &element : *Node) {
if (element->getKind() != NodeKind::TupleElement)
return BuiltType();
Expand Down Expand Up @@ -744,7 +743,7 @@ class TypeDecoder {

elements.push_back(elementType);
}
return Builder.createTupleType(elements, std::move(labels), variadic);
return Builder.createTupleType(elements, std::move(labels));
}
case NodeKind::TupleElement:
if (Node->getNumChildren() < 1)
Expand Down
20 changes: 5 additions & 15 deletions include/swift/Reflection/TypeRef.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,38 +302,28 @@ class BoundGenericTypeRef final : public TypeRef, public NominalTypeTrait {

class TupleTypeRef final : public TypeRef {
std::vector<const TypeRef *> Elements;
bool Variadic;

static TypeRefID Profile(const std::vector<const TypeRef *> &Elements,
bool Variadic) {
static TypeRefID Profile(const std::vector<const TypeRef *> &Elements) {
TypeRefID ID;
for (auto Element : Elements)
ID.addPointer(Element);

ID.addInteger(static_cast<uint32_t>(Variadic));
return ID;
}

public:
TupleTypeRef(std::vector<const TypeRef *> Elements, bool Variadic=false)
: TypeRef(TypeRefKind::Tuple), Elements(std::move(Elements)),
Variadic(Variadic) {}
TupleTypeRef(std::vector<const TypeRef *> Elements)
: TypeRef(TypeRefKind::Tuple), Elements(std::move(Elements)) {}

template <typename Allocator>
static const TupleTypeRef *create(Allocator &A,
std::vector<const TypeRef *> Elements,
bool Variadic = false) {
FIND_OR_CREATE_TYPEREF(A, TupleTypeRef, Elements, Variadic);
std::vector<const TypeRef *> Elements) {
FIND_OR_CREATE_TYPEREF(A, TupleTypeRef, Elements);
}

const std::vector<const TypeRef *> &getElements() const {
return Elements;
};

bool isVariadic() const {
return Variadic;
}

static bool classof(const TypeRef *TR) {
return TR->getKind() == TypeRefKind::Tuple;
}
Expand Down
6 changes: 3 additions & 3 deletions include/swift/Reflection/TypeRefBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,10 +404,10 @@ class TypeRefBuilder {
}

const TupleTypeRef *createTupleType(llvm::ArrayRef<const TypeRef *> elements,
std::string &&labels, bool isVariadic) {
std::string &&labels) {
// FIXME: Add uniqueness checks in TupleTypeRef::Profile and
// unittests/Reflection/TypeRef.cpp if using labels for identity.
return TupleTypeRef::create(*this, elements, isVariadic);
return TupleTypeRef::create(*this, elements);
}

const FunctionTypeRef *createFunctionType(
Expand Down Expand Up @@ -445,7 +445,7 @@ class TypeRefBuilder {
break;
}

auto result = createTupleType({}, "", false);
auto result = createTupleType({}, "");
return FunctionTypeRef::create(*this, {}, result, funcFlags);
}

Expand Down
4 changes: 2 additions & 2 deletions include/swift/Remote/MetadataReader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -759,8 +759,8 @@ class MetadataReader {
!Reader->readString(RemoteAddress(tupleMeta->Labels), labels))
return BuiltType();

auto BuiltTuple = Builder.createTupleType(elementTypes, std::move(labels),
/*variadic*/ false);
auto BuiltTuple =
Builder.createTupleType(elementTypes, std::move(labels));
TypeCache[MetadataAddress] = BuiltTuple;
return BuiltTuple;
}
Expand Down
7 changes: 1 addition & 6 deletions lib/AST/ASTDemangler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,12 +323,7 @@ Type ASTBuilder::createBoundGenericType(GenericTypeDecl *decl,
return aliasDecl->getDeclaredInterfaceType().subst(subMap);
}

Type ASTBuilder::createTupleType(ArrayRef<Type> eltTypes,
StringRef labels,
bool isVariadic) {
// Just bail out on variadic tuples for now.
if (isVariadic) return Type();

Type ASTBuilder::createTupleType(ArrayRef<Type> eltTypes, StringRef labels) {
SmallVector<TupleTypeElt, 4> elements;
elements.reserve(eltTypes.size());
for (auto eltType : eltTypes) {
Expand Down
6 changes: 0 additions & 6 deletions stdlib/public/Reflection/TypeRef.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,12 +458,6 @@ class DemanglingForTypeRef

Demangle::NodePointer visitTupleTypeRef(const TupleTypeRef *T) {
auto tuple = Dem.createNode(Node::Kind::Tuple);
if (T->isVariadic()) {
auto tupleElt = Dem.createNode(Node::Kind::TupleElement);
tupleElt->addChild(Dem.createNode(Node::Kind::VariadicMarker), Dem);
tuple->addChild(tupleElt, Dem);
return tuple;
}

for (auto element : T->getElements()) {
auto tupleElt = Dem.createNode(Node::Kind::TupleElement);
Expand Down
3 changes: 1 addition & 2 deletions stdlib/public/runtime/MetadataLookup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1397,8 +1397,7 @@ class DecodedMetadataBuilder {
}

BuiltType createTupleType(llvm::ArrayRef<BuiltType> elements,
std::string labels, bool variadic) const {
// TODO: 'variadic' should no longer exist
std::string labels) const {
auto flags = TupleTypeFlags().withNumElements(elements.size());
if (!labels.empty())
flags = flags.withNonConstantLabels(true);
Expand Down
23 changes: 8 additions & 15 deletions unittests/Reflection/TypeRef.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,36 +88,29 @@ TEST(TypeRefTest, UniqueTupleTypeRef) {
auto N2 = Builder.createNominalType(XYZ, nullptr);

std::vector<const TypeRef *> Void;
auto Void1 = Builder.createTupleType(Void, "", false);
auto Void2 = Builder.createTupleType(Void, "", false);
auto Void1 = Builder.createTupleType(Void, "");
auto Void2 = Builder.createTupleType(Void, "");

EXPECT_EQ(Void1, Void2);

std::vector<const TypeRef *> Elements1 { N1, N2 };
std::vector<const TypeRef *> Elements2 { N1, N2, N2 };

auto T1 = Builder.createTupleType(Elements1, "", false);
auto T2 = Builder.createTupleType(Elements1, "", false);
auto T3 = Builder.createTupleType(Elements2, "", false);
auto T1 = Builder.createTupleType(Elements1, "");
auto T2 = Builder.createTupleType(Elements1, "");
auto T3 = Builder.createTupleType(Elements2, "");

EXPECT_EQ(T1, T2);
EXPECT_NE(T2, T3);
EXPECT_NE(T1, Void1);

auto T4 = Builder.createTupleType(Elements1, "", true);
auto T5 = Builder.createTupleType(Elements1, "", true);
auto T6 = Builder.createTupleType(Elements1, "", false);

EXPECT_EQ(T4, T5);
EXPECT_NE(T5, T6);
}

TEST(TypeRefTest, UniqueFunctionTypeRef) {

TypeRefBuilder Builder(TypeRefBuilder::ForTesting);

std::vector<const TypeRef *> Void;
auto VoidResult = Builder.createTupleType(Void, "", false);
auto VoidResult = Builder.createTupleType(Void, "");
Param Param1 = Builder.createNominalType(ABC, nullptr);
Param Param2 = Builder.createNominalType(XYZ, nullptr);

Expand All @@ -126,7 +119,7 @@ TEST(TypeRefTest, UniqueFunctionTypeRef) {
std::vector<Param> Parameters2{Param1, Param1};

auto Result =
Builder.createTupleType({Param1.getType(), Param2.getType()}, "", false);
Builder.createTupleType({Param1.getType(), Param2.getType()}, "");

auto F1 =
Builder.createFunctionType(Parameters1, Result, FunctionTypeFlags());
Expand Down Expand Up @@ -449,7 +442,7 @@ TEST(TypeRefTest, DeriveSubstitutions) {
auto Nominal = Builder.createBoundGenericType(NominalName, NominalArgs,
/*parent*/ nullptr);

auto Result = Builder.createTupleType({GTP00, GTP01}, "", false);
auto Result = Builder.createTupleType({GTP00, GTP01}, "");
auto Func =
Builder.createFunctionType({Nominal}, Result, FunctionTypeFlags());

Expand Down