Skip to content

Automatic conversion of mutable pointers to immutable pointers. #37214

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

Closed
wants to merge 9 commits into from
Closed
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
13 changes: 13 additions & 0 deletions include/swift/AST/ASTContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,20 @@ class ASTContext final {
llvm::BumpPtrAllocator &
getAllocator(AllocationArena arena = AllocationArena::Permanent) const;

/// init(implicit:) constructors declaring implicit conversions.
/// indexed by nominal of toType then nominal of fromType.
llvm::DenseMap<NominalTypeDecl *, llvm::DenseMap<NominalTypeDecl *,
SmallVector<ConstructorDecl *, 2>>> ImplicitConversionMap;

public:
llvm::DenseMap<NominalTypeDecl *, SmallVector<ConstructorDecl *, 2>>
*implicitConversionsTo(NominalTypeDecl *toNominal, bool create) {
auto constructorsByFromType = ImplicitConversionMap.find(toNominal);
if (constructorsByFromType != ImplicitConversionMap.end())
return &constructorsByFromType->getSecond();
return create ? &ImplicitConversionMap[toNominal] : nullptr;
}

/// Allocate - Allocate memory from the ASTContext bump pointer.
void *Allocate(unsigned long bytes, unsigned alignment,
AllocationArena arena = AllocationArena::Permanent) const {
Expand Down
8 changes: 8 additions & 0 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -3155,11 +3155,19 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext {
Bits.NominalTypeDecl.IsComputingSemanticMembers = false;
}

bool implicitConversionsComputed = false;

friend class ProtocolType;

public:
using GenericTypeDecl::getASTContext;

bool setConversionsComputed() {
bool wasComputed = implicitConversionsComputed;
implicitConversionsComputed = true;
return wasComputed;
}

SourceRange getBraces() const { return Braces; }

void setBraces(SourceRange braces) { Braces = braces; }
Expand Down
1 change: 1 addition & 0 deletions include/swift/AST/KnownIdentifiers.def
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ IDENTIFIER(identifier)
IDENTIFIER(_distributedActorRemoteInitialize)
IDENTIFIER(_distributedActorDestroy)
IDENTIFIER(__isRemoteActor)
IDENTIFIER(implicit)

#undef IDENTIFIER
#undef IDENTIFIER_
Expand Down
2 changes: 2 additions & 0 deletions include/swift/Sema/Constraint.h
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,8 @@ enum class ConversionRestrictionKind {
/// Implicit conversion from a value of CGFloat type to a value of Double type
/// via an implicit Double initializer call passing a CGFloat value.
CGFloatToDouble,
/// Implicit conversion where an init(implicit:) initializer is available.
ImplicitConversion,
};

/// Specifies whether a given conversion requires the creation of a temporary
Expand Down
3 changes: 3 additions & 0 deletions include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -4243,6 +4243,9 @@ class ConstraintSystem {
SmallVectorImpl<RestrictionOrFix> &conversionsOrFixes,
ConstraintLocatorBuilder locator);

/// Is an implicit conversion available from fromType to toType.
ConstructorDecl *implicitConversionAvailable(Type fromType, Type toType);

/// Subroutine of \c matchTypes(), which matches up two tuple types.
///
/// \returns the result of performing the tuple-to-tuple conversion.
Expand Down
4 changes: 4 additions & 0 deletions lib/AST/ASTVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1136,6 +1136,10 @@ class Verifier : public ASTWalker {
}

void verifyChecked(TupleExpr *E) {
if (!E->getType()->is<TupleType>()) {
E->getType()->dump();
return;
}
const TupleType *exprTy = E->getType()->castTo<TupleType>();
for_each(exprTy->getElements().begin(), exprTy->getElements().end(),
E->getElements().begin(),
Expand Down
1 change: 1 addition & 0 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3839,6 +3839,7 @@ void NominalTypeDecl::addExtension(ExtensionDecl *extension) {
LastExtension->NextExtension.setPointer(extension);
LastExtension = extension;

implicitConversionsComputed = false;
addedExtension(extension);
}

Expand Down
7 changes: 3 additions & 4 deletions lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,7 @@ bool TypeBase::isStdlibType() {

bool TypeBase::isCGFloatType() {
auto *NTD = getAnyNominal();
if (!NTD)
if (!NTD || !NTD->getName().is("CGFloat"))
return false;

auto *DC = NTD->getDeclContext();
Expand All @@ -828,9 +828,8 @@ bool TypeBase::isCGFloatType() {
auto *module = DC->getParentModule();
// On macOS `CGFloat` is part of a `CoreGraphics` module,
// but on Linux it could be found in `Foundation`.
return (module->getName().is("CoreGraphics") ||
module->getName().is("Foundation")) &&
NTD->getName().is("CGFloat");
return module->getName().is("CoreGraphics") ||
module->getName().is("Foundation");
}

bool TypeBase::isKnownStdlibCollectionType() {
Expand Down
29 changes: 26 additions & 3 deletions lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5606,6 +5606,9 @@ static unsigned getOptionalEvaluationDepth(Expr *expr, Expr *target) {
expr = open->getExistentialValue();

// Otherwise, look through implicit conversions.
} else if (auto call = dyn_cast<CallExpr>(expr)) {
// expr->dump();
return depth;
} else {
expr = cast<ImplicitConversionExpr>(expr)->getSubExpr();
}
Expand Down Expand Up @@ -6633,6 +6636,10 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
if (toType->hasUnresolvedType())
break;

if (cs.implicitConversionAvailable(fromType->lookThroughAllOptionalTypes(),
toType->lookThroughAllOptionalTypes()))
break;

// HACK: Fix problem related to Swift 4 mode (with assertions),
// since Swift 4 mode allows passing arguments with extra parens
// to parameters which don't expect them, it should be supported
Expand Down Expand Up @@ -6853,8 +6860,16 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
}

case ConversionRestrictionKind::CGFloatToDouble:
case ConversionRestrictionKind::DoubleToCGFloat: {
case ConversionRestrictionKind::DoubleToCGFloat:
case ConversionRestrictionKind::ImplicitConversion: {
auto conversionKind = knownRestriction->second;
ConstructorDecl *implicitConstructor = nullptr;
Identifier label;
if (conversionKind == ConversionRestrictionKind::ImplicitConversion) {
// fromType->dump(); toType->dump();
implicitConstructor = cs.implicitConversionAvailable(fromType, toType);
label = cs.getASTContext().Id_implicit;
}

auto *argExpr = locator.trySimplifyToExpr();
assert(argExpr);
Expand All @@ -6865,7 +6880,7 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
auto *implicitInit =
CallExpr::createImplicit(ctx, TypeExpr::createImplicit(toType, ctx),
/*args=*/{argExpr},
/*argLabels=*/{Identifier()});
/*argLabels=*/{label});

cs.cacheExprTypes(implicitInit->getFn());
cs.setType(argExpr, fromType);
Expand Down Expand Up @@ -6894,7 +6909,15 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
ConstraintLocator::ApplyFunction,
ConstraintLocator::ConstructorMember}));

solution.overloadChoices.insert({memberLoc, overload});
if (implicitConstructor) { // Hack overload to constructor for implicits
SelectedOverload implicitOverload = {
OverloadChoice(toType, implicitConstructor,
overload.choice.getFunctionRefKind()),
overload.openedFullType, overload.openedType, overload.boundType};
solution.overloadChoices.insert({memberLoc, implicitOverload});
}
else
solution.overloadChoices.insert({memberLoc, overload});
}

// Record the implicit call's parameter bindings and match direction.
Expand Down
1 change: 1 addition & 0 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6703,6 +6703,7 @@ void NonEphemeralConversionFailure::emitSuggestionNotes() const {
case ConversionRestrictionKind::ObjCTollFreeBridgeToCF:
case ConversionRestrictionKind::CGFloatToDouble:
case ConversionRestrictionKind::DoubleToCGFloat:
case ConversionRestrictionKind::ImplicitConversion:
llvm_unreachable("Expected an ephemeral conversion!");
}
}
Expand Down
111 changes: 103 additions & 8 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3635,6 +3635,10 @@ static bool repairOutOfOrderArgumentsInBinaryFunction(
return false;
}

/// Switch between restrictions mechanism and original PR using
/// ConversionRestrictionKind::PointerToPointer in repairFailures().
static bool useRestrictions = true;

/// Attempt to repair typing failures and record fixes if needed.
/// \return true if at least some of the failures has been repaired
/// successfully, which allows type matcher to continue.
Expand Down Expand Up @@ -4971,9 +4975,90 @@ bool ConstraintSystem::repairFailures(
break;
}

// Accept mutable pointers in the place of immutables.
if (implicitConversionAvailable(lhs, rhs))
conversionsOrFixes.push_back(
ConversionRestrictionKind::ImplicitConversion);

return !conversionsOrFixes.empty();
}

ConstructorDecl *ConstraintSystem::implicitConversionAvailable(Type fromType, Type toType) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The implementation here would have to be more interesting, that's why we (me and @hborla) were talking about it being complex for the solver. We'd have to add a member constraint to lookup init(implicit:) on the fromType and apply found member with an argument toType to make sure that everything fits together correctly, so instead of doing manual lookup down here you'd have to always record an implicit conversion between types that didn't match but are constrained on Conversion and down in simplifyRestrictedConstraintImpl implement it as member constraint + applicable function constraint.

#if 01 // Determine implicit conversions from init([_] implicit:) constructors
static int called = 0;
static const char *from = 0 ? "\nFROM" : nullptr;
auto trace = [](const char *prefix, Type ty) {
if (!from)
return;
if (prefix == from)
called++;
fprintf(stderr, prefix == from ? "%s %p #%d " : "%s %p ",
prefix, ty.getPointer(), called);
ty->dump();
};
trace(from, fromType);
trace("TO", toType);
if (NominalTypeDecl *toNominal = toType->getAnyNominal()) {
auto &ctx = getASTContext();
if (!toNominal->setConversionsComputed())
for (ExtensionDecl *extension : toNominal->getExtensions()) {
Type extType = extension->getExtendedType()->getCanonicalType();
int arged = 0;
for (Decl *member : extension->getMembers())
if (ConstructorDecl *initDecl = dyn_cast<ConstructorDecl>(member)) {
ParameterList *parameters = initDecl->getParameters();
ParamDecl *param; // could be @implicit instead..
if (parameters->size() == 1 && (param = parameters->get(0)) &&
(param->getArgumentName() == ctx.Id_implicit ||
param->getParameterName() == ctx.Id_implicit)) {
Type argType = param->getType()->getCanonicalType();
if (!arged++)
trace("\nEXT", extType);
trace(" ARG", argType);
(*ctx.implicitConversionsTo(extType->getAnyNominal(),
/*create*/true))[argType->getAnyNominal()].push_back(initDecl);
}
}
}

if (auto *exists = ctx.implicitConversionsTo(toNominal, /*create*/false))
for (ConstructorDecl *initDecl : (*exists)[fromType->getAnyNominal()])
if (ExtensionDecl *ext = dyn_cast<ExtensionDecl>(initDecl->getParent()))
if (Type argType = initDecl->getParameters()->get(0)->getType())
if (Type extType = initDecl->getResultInterfaceType()) {
extType = initDecl->getParent()->mapTypeIntoContext(extType);
// More rigourous check of conversion here...
trace("EXT2", extType);
trace("ARG2", argType);
if (argType->getCanonicalType() == fromType->getCanonicalType() ||
(extType->isUnsafeRawPointer() &&
(fromType->isUnsafeMutablePointer() || fromType->isUnsafePointer()))) {
trace("SELECTED", initDecl->getInterfaceType());
return initDecl;
}
}
}
#else // Original hard coded rules for unsafe pointers.
if (toType->isUnsafeRawPointer() && (fromType->isUnsafeMutableRawPointer() ||
fromType->isUnsafeMutablePointer() || fromType->isUnsafePointer())) {
return true; // Unsafe[Mutable][Raw]Pointer -> UnsafeRawPointer
}

auto firstGenericArgument = [&](Type ty) -> CanType {
return dyn_cast<BoundGenericStructType>(ty->getCanonicalType())
->getGenericArgs()[0]->getCanonicalType();
};

if (fromType->isUnsafeMutablePointer() && toType->isUnsafePointer() &&
firstGenericArgument(fromType) == firstGenericArgument(toType)) {
toType->dump();
fromType->dump();
return true; // UnsafeMutablePointer<Pointee> -> UnsafePointer<Pointee>
}
#endif
return nullptr;
}

ConstraintSystem::TypeMatchResult
ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
TypeMatchOptions flags,
Expand Down Expand Up @@ -5317,10 +5402,12 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
}
}

if (kind >= ConstraintKind::Subtype &&
#if 0
if (0 && kind >= ConstraintKind::Subtype &&
nominal1->getDecl() != nominal2->getDecl() &&
((nominal1->isCGFloatType() || nominal2->isCGFloatType()) &&
(nominal1->isDouble() || nominal2->isDouble()))) {
(nominal1->isDouble() || nominal2->isDouble())) ||
useRestrictions && implicitConversionAvailable(type1, type2)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please extract this into a separate check, no need to try to plug it into the Double/CGFloat. As a side note - if you are working on a more general mechanism then I think we need to remove Double/CGFloat specific logic all together from here because it would be possible to declare a init(implicit: CGFloat) on Double and reverse one of CGFloat and use new infrastructure.

ConstraintLocatorBuilder location{locator};
// Look through all value-to-optional promotions to allow
// conversions like Double -> CGFloat?? and vice versa.
Expand Down Expand Up @@ -5388,16 +5475,20 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
rawElt.getAs<LocatorPathElt::ImplicitConversion>()) {
auto convKind = elt->getConversionKind();
return convKind == ConversionRestrictionKind::DoubleToCGFloat ||
convKind == ConversionRestrictionKind::CGFloatToDouble;
convKind == ConversionRestrictionKind::CGFloatToDouble ||
convKind == ConversionRestrictionKind::ImplicitConversion;
}
return false;
})) {
conversionsOrFixes.push_back(
desugar1->isCGFloatType()
? ConversionRestrictionKind::CGFloatToDouble
: ConversionRestrictionKind::DoubleToCGFloat);
// desugar1->isCGFloatType()
// ? ConversionRestrictionKind::CGFloatToDouble :
// desugar2->isCGFloatType()
// ? ConversionRestrictionKind::DoubleToCGFloat :
ConversionRestrictionKind::ImplicitConversion);
}
}
#endif

break;
}
Expand Down Expand Up @@ -11123,7 +11214,8 @@ ConstraintSystem::simplifyRestrictedConstraintImpl(
}

case ConversionRestrictionKind::DoubleToCGFloat:
case ConversionRestrictionKind::CGFloatToDouble: {
case ConversionRestrictionKind::CGFloatToDouble:
case ConversionRestrictionKind::ImplicitConversion: {
// Prefer CGFloat -> Double over other way araund.
auto impact =
restriction == ConversionRestrictionKind::CGFloatToDouble ? 1 : 10;
Expand Down Expand Up @@ -11170,8 +11262,11 @@ ConstraintSystem::simplifyRestrictedConstraintImpl(
memberTy, DC, FunctionRefKind::DoubleApply,
/*outerAlternatives=*/{}, memberLoc);

Identifier label;
if (restriction == ConversionRestrictionKind::ImplicitConversion)
label = getASTContext().Id_implicit;
addConstraint(ConstraintKind::ApplicableFunction,
FunctionType::get({FunctionType::Param(type1)}, type2),
FunctionType::get({FunctionType::Param(type1, label)}, type2),
memberTy, applicationLoc);

ImplicitValueConversions.push_back(
Expand Down
2 changes: 2 additions & 0 deletions lib/Sema/Constraint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,8 @@ StringRef swift::constraints::getName(ConversionRestrictionKind kind) {
return "[CGFloat-to-Double]";
case ConversionRestrictionKind::DoubleToCGFloat:
return "[Double-to-CGFloat]";
case ConversionRestrictionKind::ImplicitConversion:
return "[Implicit-Conversion]";
}
llvm_unreachable("bad conversion restriction kind");
}
Expand Down
7 changes: 6 additions & 1 deletion lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,9 @@ void ConstraintSystem::assignFixedType(TypeVariableType *typeVar, Type type,
getASTContext(), castToExpr(locator->getAnchor()));
if (!literalProtocol)
continue;
// fprintf(stderr, "HERE ");
// (locator->getAnchor()).dump();
// fprintf(stderr, "\n");

// If the protocol has a default type, check it.
if (auto defaultType = TypeChecker::getDefaultType(literalProtocol, DC)) {
Expand Down Expand Up @@ -452,7 +455,8 @@ ConstraintLocator *ConstraintSystem::getCalleeLocator(
if (auto conversion =
locator->findLast<LocatorPathElt::ImplicitConversion>()) {
if (conversion->is(ConversionRestrictionKind::DoubleToCGFloat) ||
conversion->is(ConversionRestrictionKind::CGFloatToDouble)) {
conversion->is(ConversionRestrictionKind::CGFloatToDouble) ||
conversion->is(ConversionRestrictionKind::ImplicitConversion)) {
return getConstraintLocator(
ASTNode(), {*conversion, ConstraintLocator::ApplyFunction,
ConstraintLocator::ConstructorMember});
Expand Down Expand Up @@ -5130,6 +5134,7 @@ ConstraintSystem::isConversionEphemeral(ConversionRestrictionKind conversion,
case ConversionRestrictionKind::ObjCTollFreeBridgeToCF:
case ConversionRestrictionKind::CGFloatToDouble:
case ConversionRestrictionKind::DoubleToCGFloat:
case ConversionRestrictionKind::ImplicitConversion:
// @_nonEphemeral has no effect on these conversions, so treat them as all
// being non-ephemeral in order to allow their passing to an @_nonEphemeral
// parameter.
Expand Down
4 changes: 4 additions & 0 deletions lib/Sema/TypeCheckCaptures.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,10 @@ class FindCapturedVars : public ASTWalker {
// Approximate this for the purposes of being able to invoke @objc methods
// by considering tuples of ObjC-representable types to not use metadata.
if (auto tuple = dyn_cast<TupleExpr>(E)) {
if (!tuple->getType()->is<TupleType>()) {
tuple->getType()->dump();
return false;
}
for (auto elt : tuple->getType()->castTo<TupleType>()->getElements()) {
if (!elt.getType()->isRepresentableIn(ForeignLanguage::ObjectiveC,
CurDC))
Expand Down
Loading