Skip to content

[Sema] Allow PlaceholderType Originator to be TypeRepr not just PlaceholderTypeRepr #75463

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

Closed
wants to merge 1 commit into from
Closed

[Sema] Allow PlaceholderType Originator to be TypeRepr not just PlaceholderTypeRepr #75463

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
2 changes: 1 addition & 1 deletion include/swift/AST/Types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7291,7 +7291,7 @@ class PlaceholderType : public TypeBase {
// recursive property logic in PlaceholderType::get.
using Originator =
llvm::PointerUnion<TypeVariableType *, DependentMemberType *, VarDecl *,
ErrorExpr *, PlaceholderTypeRepr *>;
ErrorExpr *, TypeRepr *>;

Originator O;

Expand Down
6 changes: 3 additions & 3 deletions include/swift/Sema/ConstraintLocator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -985,13 +985,13 @@ class LocatorPathElt::ConformanceRequirement final
};

class LocatorPathElt::PlaceholderType final
: public StoredPointerElement<PlaceholderTypeRepr> {
: public StoredPointerElement<TypeRepr> {
public:
PlaceholderType(PlaceholderTypeRepr *placeholderRepr)
PlaceholderType(TypeRepr *placeholderRepr)
: StoredPointerElement(PathElementKind::PlaceholderType,
placeholderRepr) {}

PlaceholderTypeRepr *getPlaceholderRepr() const { return getStoredPointer(); }
TypeRepr *getPlaceholderRepr() const { return getStoredPointer(); }

static bool classof(const LocatorPathElt *elt) {
return elt->getKind() == ConstraintLocator::PlaceholderType;
Expand Down
4 changes: 2 additions & 2 deletions lib/AST/ASTDumper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3989,8 +3989,8 @@ namespace {
printFlag("error_expr");
} else if (auto *DMT = originator.dyn_cast<DependentMemberType *>()) {
printRec(DMT, "dependent_member_type");
} else if (originator.is<PlaceholderTypeRepr *>()) {
printFlag("placeholder_type_repr");
} else if (originator.is<TypeRepr *>()) {
printFlag("type_repr");
} else {
assert(false && "unknown originator");
}
Expand Down
4 changes: 2 additions & 2 deletions lib/AST/ASTPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6021,8 +6021,8 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
Printer << "error_expr";
} else if (auto *DMT = originator.dyn_cast<DependentMemberType *>()) {
visit(DMT);
} else if (originator.is<PlaceholderTypeRepr *>()) {
Printer << "placeholder_type_repr";
} else if (originator.is<TypeRepr *>()) {
Printer << "type_repr";
} else {
assert(false && "unknown originator");
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/CSDiagnostics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9338,7 +9338,7 @@ bool InvalidMemberReferenceWithinInitAccessor::diagnoseAsError() {
}

bool ConcreteTypeSpecialization::diagnoseAsError() {
emitDiagnostic(diag::not_a_generic_type, resolveType(ConcreteType));
emitDiagnostic(diag::not_a_generic_type, ConcreteType);
return true;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/CSDiagnostics.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3116,7 +3116,7 @@ class ConcreteTypeSpecialization final : public FailureDiagnostic {
ConcreteTypeSpecialization(const Solution &solution, Type concreteTy,
ConstraintLocator *locator)
: FailureDiagnostic(solution, locator),
ConcreteType(concreteTy) {}
ConcreteType(resolveType(concreteTy)) {}

bool diagnoseAsError() override;
};
Expand Down
9 changes: 4 additions & 5 deletions lib/Sema/CSGen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1924,8 +1924,7 @@ namespace {
OpenPackElementType(CS, locator, elementEnv));
if (result->hasError()) {
auto &ctxt = CS.getASTContext();
auto *repr = new (ctxt) PlaceholderTypeRepr(specializationArg->getLoc());
result = PlaceholderType::get(ctxt, repr);
result = PlaceholderType::get(ctxt, specializationArg);
ctxt.Diags.diagnose(lAngleLoc,
diag::while_parsing_as_left_angle_bracket);
}
Expand Down Expand Up @@ -4806,11 +4805,11 @@ bool ConstraintSystem::generateConstraints(
// If we have a placeholder originating from a PlaceholderTypeRepr,
// tack that on to the locator.
if (auto *placeholderTy = ty->getAs<PlaceholderType>())
if (auto *placeholderRepr = placeholderTy->getOriginator()
.dyn_cast<PlaceholderTypeRepr *>())
if (auto *typeRepr = placeholderTy->getOriginator()
.dyn_cast<TypeRepr *>())
return getConstraintLocator(
convertTypeLocator,
LocatorPathElt::PlaceholderType(placeholderRepr));
LocatorPathElt::PlaceholderType(typeRepr));
return convertTypeLocator;
};

Expand Down
22 changes: 11 additions & 11 deletions lib/Sema/ConstraintSystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1071,17 +1071,17 @@ Type ConstraintSystem::replaceInferableTypesWithTypeVars(
return openUnboundGenericType(unbound->getDecl(), unbound->getParent(),
locator, /*isTypeResolution=*/false);
} else if (auto *placeholderTy = type->getAs<PlaceholderType>()) {
if (auto *placeholderRepr = placeholderTy->getOriginator()
.dyn_cast<PlaceholderTypeRepr *>()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This change needs an additional check for PlaceholderTypeRepr as is we only want to open _ type reprs and not the invalid reprs.


return createTypeVariable(
getConstraintLocator(
locator, LocatorPathElt::PlaceholderType(placeholderRepr)),
TVO_CanBindToNoEscape | TVO_PrefersSubtypeBinding |
TVO_CanBindToHole);
}

if (auto *var = placeholderTy->getOriginator().dyn_cast<VarDecl *>()) {
if (auto *typeRepr =
placeholderTy->getOriginator().dyn_cast<TypeRepr *>()) {
if (isa<PlaceholderTypeRepr>(typeRepr)) {
return createTypeVariable(
getConstraintLocator(locator,
LocatorPathElt::PlaceholderType(typeRepr)),
TVO_CanBindToNoEscape | TVO_PrefersSubtypeBinding |
TVO_CanBindToHole);
}
} else if (auto *var =
placeholderTy->getOriginator().dyn_cast<VarDecl *>()) {
if (var->getName().hasDollarPrefix()) {
auto *repr =
new (type->getASTContext()) PlaceholderTypeRepr(var->getLoc());
Expand Down
3 changes: 2 additions & 1 deletion lib/Sema/TypeCheckDeclPrimary.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1172,7 +1172,8 @@ void TypeChecker::notePlaceholderReplacementTypes(Type writtenType,
}

if (auto *origRepr =
placeholder->getOriginator().dyn_cast<PlaceholderTypeRepr *>()) {
placeholder->getOriginator().dyn_cast<TypeRepr *>()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add an assert here that that makes sure that origRepr was PlaceholderTypeRepr.

assert(isa<PlaceholderTypeRepr>(origRepr));
t1->getASTContext()
.Diags
.diagnose(origRepr->getLoc(),
Expand Down