Skip to content

AST: Remove LiteralExpr::shallowClone() #26118

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 1 commit into from
Jul 13, 2019
Merged

AST: Remove LiteralExpr::shallowClone() #26118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 0 additions & 6 deletions include/swift/AST/Expr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,12 +599,6 @@ class CodeCompletionExpr : public Expr {
class LiteralExpr : public Expr {
public:
LiteralExpr(ExprKind Kind, bool Implicit) : Expr(Kind, Implicit) {}

// Make an exact copy of this one AST node.
LiteralExpr *
shallowClone(ASTContext &Ctx,
llvm::function_ref<void(Expr *, Type)> setType,
llvm::function_ref<Type(const Expr *)> getType) const;

static bool classof(const Expr *E) {
return E->getKind() >= ExprKind::First_LiteralExpr &&
Expand Down
97 changes: 0 additions & 97 deletions lib/AST/Expr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,103 +773,6 @@ llvm::DenseMap<Expr *, unsigned> Expr::getPreorderIndexMap() {
// Support methods for Exprs.
//===----------------------------------------------------------------------===//

static LiteralExpr *
shallowCloneImpl(const NilLiteralExpr *E, ASTContext &Ctx,
llvm::function_ref<Type(const Expr *)> getType) {
return new (Ctx) NilLiteralExpr(E->getLoc());
}

static LiteralExpr *
shallowCloneImpl(const IntegerLiteralExpr *E, ASTContext &Ctx,
llvm::function_ref<Type(const Expr *)> getType) {
auto res = new (Ctx) IntegerLiteralExpr(E->getDigitsText(),
E->getSourceRange().End);
if (E->isNegative())
res->setNegative(E->getSourceRange().Start);
return res;
}

static LiteralExpr *
shallowCloneImpl(const FloatLiteralExpr *E, ASTContext &Ctx,
llvm::function_ref<Type(const Expr *)> getType) {
auto res = new (Ctx) FloatLiteralExpr(E->getDigitsText(),
E->getSourceRange().End);
if (E->isNegative())
res->setNegative(E->getSourceRange().Start);
return res;
}
static LiteralExpr *
shallowCloneImpl(const BooleanLiteralExpr *E, ASTContext &Ctx,
llvm::function_ref<Type(const Expr *)> getType) {
return new (Ctx) BooleanLiteralExpr(E->getValue(), E->getLoc());
}
static LiteralExpr *
shallowCloneImpl(const StringLiteralExpr *E, ASTContext &Ctx,
llvm::function_ref<Type(const Expr *)> getType) {
auto res = new (Ctx) StringLiteralExpr(E->getValue(), E->getSourceRange());
res->setEncoding(E->getEncoding());
return res;
}

static LiteralExpr *
shallowCloneImpl(const InterpolatedStringLiteralExpr *E, ASTContext &Ctx,
llvm::function_ref<Type(const Expr *)> getType) {
auto res = new (Ctx) InterpolatedStringLiteralExpr(E->getLoc(),
E->getTrailingQuoteLoc(),
E->getLiteralCapacity(),
E->getInterpolationCount(),
E->getAppendingExpr());
res->setSemanticExpr(E->getSemanticExpr());
return res;
}

static LiteralExpr *
shallowCloneImpl(const MagicIdentifierLiteralExpr *E, ASTContext &Ctx,
llvm::function_ref<Type(const Expr *)> getType) {
auto res = new (Ctx) MagicIdentifierLiteralExpr(E->getKind(),
E->getSourceRange().End);
if (res->isString())
res->setStringEncoding(E->getStringEncoding());
return res;
}

static LiteralExpr *
shallowCloneImpl(const ObjectLiteralExpr *E, ASTContext &Ctx,
llvm::function_ref<Type(const Expr *)> getType) {
auto res =
ObjectLiteralExpr::create(Ctx, E->getStartLoc(), E->getLiteralKind(),
E->getArg(), E->isImplicit(), getType);
return res;
}

// Make an exact copy of this AST node.
LiteralExpr *LiteralExpr::shallowClone(
ASTContext &Ctx, llvm::function_ref<void(Expr *, Type)> setType,
llvm::function_ref<Type(const Expr *)> getType) const {
LiteralExpr *Result = nullptr;
switch (getKind()) {
default: llvm_unreachable("Unknown literal type!");
#define DISPATCH_CLONE(KIND) \
case ExprKind::KIND: \
Result = shallowCloneImpl(cast<KIND##Expr>(this), Ctx, getType); \
break;

DISPATCH_CLONE(NilLiteral)
DISPATCH_CLONE(IntegerLiteral)
DISPATCH_CLONE(FloatLiteral)
DISPATCH_CLONE(BooleanLiteral)
DISPATCH_CLONE(StringLiteral)
DISPATCH_CLONE(InterpolatedStringLiteral)
DISPATCH_CLONE(ObjectLiteral)
DISPATCH_CLONE(MagicIdentifierLiteral)
#undef DISPATCH_CLONE
}

setType(Result, getType(this));
Result->setImplicit(isImplicit());
return Result;
}

IntegerLiteralExpr * IntegerLiteralExpr::createFromUnsigned(ASTContext &C, unsigned value) {
llvm::SmallString<8> Scratch;
llvm::APInt(sizeof(unsigned)*8, value).toString(Scratch, 10, /*signed*/ false);
Expand Down
22 changes: 2 additions & 20 deletions lib/Sema/CSApply.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6685,26 +6685,9 @@ Expr *ExprRewriter::convertLiteralInPlace(Expr *literal,
DeclName builtinLiteralFuncName,
Diag<> brokenProtocolDiag,
Diag<> brokenBuiltinProtocolDiag) {
auto &tc = cs.getTypeChecker();
Copy link
Contributor

Choose a reason for hiding this comment

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

Too much deleting here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

... or not enough deleting further down! See updated version :)


auto getType = [&](const Expr *E) -> Type {
return cs.getType(E);
};

auto setType = [&](Expr *E, Type Ty) {
cs.setType(E, Ty);
};

// If coercing a literal to an unresolved type, we don't try to look up the
// witness members, just do it.
if (type->is<UnresolvedType>()) {
// Instead of updating the literal expr in place, allocate a new node. This
// avoids issues where Builtin types end up on expr nodes and pollute
// diagnostics.
literal = cast<LiteralExpr>(literal)->shallowClone(tc.Context, setType,
getType);

// The literal expression has this type.
cs.setType(literal, type);
return literal;
}
Expand Down Expand Up @@ -6753,9 +6736,8 @@ Expr *ExprRewriter::convertLiteralInPlace(Expr *literal,
// Dig out the literal type and perform a builtin literal conversion to it.
if (!literalType.empty()) {
// Extract the literal type.
Type builtinLiteralType = tc.getWitnessType(type, protocol, *conformance,
literalType,
brokenProtocolDiag);
Type builtinLiteralType =
conformance->getTypeWitnessByName(type, literalType);
if (!builtinLiteralType)
return nullptr;

Expand Down
2 changes: 1 addition & 1 deletion test/Constraints/add_with_nil.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

func test(_ x: Int) -> Int {
return x + nil
// expected-error@-1 {{cannot convert value of type 'Int' to expected argument type '_.Stride'}}
// expected-error@-1 {{type of expression is ambiguous without more context}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xedin I noticed this diagnostic did change. Are you OK with this? The test case was originally added when a crash was fixed. I don't think the old diagnostic actually gave you any useful information (the real problem is there's no suitable overload of + -- as far as I can see the appearance of Stride in the old message is an emergent behavior of CSDiag and CalleeCandidateDiagnostics)

}