Skip to content

Commit 34f8670

Browse files
theblixguyxedin
authored andcommitted
[CS] Use fixes to diagnose instance member on type (or vice versa) access (#21830)
This PR migrates instance member on type and type member on instance diagnostics handling to use the new diagnostics framework (fixes) and create more reliable and accurate diagnostics in such scenarios.
1 parent c18ff47 commit 34f8670

16 files changed

+520
-256
lines changed

lib/Sema/CSDiag.cpp

Lines changed: 46 additions & 219 deletions
Original file line numberDiff line numberDiff line change
@@ -511,14 +511,6 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
511511
/// Produce a diagnostic for a general member-lookup failure (irrespective of
512512
/// the exact expression kind).
513513
bool diagnoseGeneralMemberFailure(Constraint *constraint);
514-
515-
/// Diagnose the lookup of a static member or enum element as instance member.
516-
void diagnoseTypeMemberOnInstanceLookup(Type baseObjTy,
517-
Expr *baseExpr,
518-
DeclName memberName,
519-
DeclNameLoc nameLoc,
520-
ValueDecl *member,
521-
SourceLoc loc);
522514

523515
/// Given a result of name lookup that had no viable results, diagnose the
524516
/// unviable ones.
@@ -779,140 +771,6 @@ bool FailureDiagnosis::diagnoseGeneralMemberFailure(Constraint *constraint) {
779771
constraint->getFunctionRefKind(), locator);
780772
}
781773

782-
void FailureDiagnosis::
783-
diagnoseTypeMemberOnInstanceLookup(Type baseObjTy,
784-
Expr *baseExpr,
785-
DeclName memberName,
786-
DeclNameLoc nameLoc,
787-
ValueDecl *member,
788-
SourceLoc loc) {
789-
SourceRange baseRange = baseExpr ? baseExpr->getSourceRange() : SourceRange();
790-
791-
Optional<InFlightDiagnostic> Diag;
792-
793-
// If the base of the lookup is a protocol metatype, suggest
794-
// to replace the metatype with 'Self'
795-
// error saying the lookup cannot be on a protocol metatype
796-
if (auto metatypeTy = baseObjTy->getAs<MetatypeType>()) {
797-
auto instanceTy = metatypeTy->getInstanceType();
798-
799-
// This will only happen if we have an unresolved dot expression
800-
// (.foo) where foo is a protocol member and the contextual type is
801-
// an optional protocol metatype.
802-
if (auto objectTy = instanceTy->getOptionalObjectType()) {
803-
instanceTy = objectTy;
804-
baseObjTy = MetatypeType::get(objectTy);
805-
}
806-
assert(instanceTy->isExistentialType());
807-
808-
// Give a customized message if we're accessing a member type
809-
// of a protocol -- otherwise a diagnostic talking about
810-
// static members doesn't make a whole lot of sense
811-
if (auto TAD = dyn_cast<TypeAliasDecl>(member)) {
812-
Diag.emplace(diagnose(loc,
813-
diag::typealias_outside_of_protocol,
814-
TAD->getName()));
815-
} else if (auto ATD = dyn_cast<AssociatedTypeDecl>(member)) {
816-
Diag.emplace(diagnose(loc,
817-
diag::assoc_type_outside_of_protocol,
818-
ATD->getName()));
819-
} else if (isa<ConstructorDecl>(member)) {
820-
Diag.emplace(diagnose(loc,
821-
diag::construct_protocol_by_name,
822-
instanceTy));
823-
} else {
824-
Diag.emplace(diagnose(loc,
825-
diag::could_not_use_type_member_on_protocol_metatype,
826-
baseObjTy, memberName));
827-
}
828-
829-
Diag->highlight(baseRange).highlight(nameLoc.getSourceRange());
830-
831-
// See through function decl context
832-
if (auto parent = CS.DC->getInnermostTypeContext()) {
833-
// If we are in a protocol extension of 'Proto' and we see
834-
// 'Proto.static', suggest 'Self.static'
835-
if (auto extensionContext = parent->getExtendedProtocolDecl()) {
836-
if (extensionContext->getDeclaredType()->isEqual(instanceTy)) {
837-
Diag->fixItReplace(baseRange, "Self");
838-
}
839-
}
840-
}
841-
842-
return;
843-
}
844-
845-
if (isa<EnumElementDecl>(member))
846-
Diag.emplace(diagnose(loc, diag::could_not_use_enum_element_on_instance,
847-
memberName));
848-
else
849-
Diag.emplace(diagnose(loc, diag::could_not_use_type_member_on_instance,
850-
baseObjTy, memberName));
851-
852-
Diag->highlight(nameLoc.getSourceRange());
853-
854-
// No fix-it if the lookup was qualified
855-
if (baseExpr && !baseExpr->isImplicit())
856-
return;
857-
858-
// Determine the contextual type of the expression
859-
Type contextualType;
860-
for (auto iterateCS = &CS; contextualType.isNull() && iterateCS;
861-
iterateCS = iterateCS->baseCS) {
862-
contextualType = iterateCS->getContextualType();
863-
}
864-
865-
// Try to provide a fix-it that only contains a '.'
866-
if (contextualType) {
867-
if (baseObjTy->isEqual(contextualType)) {
868-
Diag->fixItInsert(loc, ".");
869-
return;
870-
}
871-
}
872-
873-
// Check if the expression is the matching operator ~=, most often used in
874-
// case statements. If so, try to provide a single dot fix-it
875-
const Expr *contextualTypeNode = nullptr;
876-
ConstraintSystem *lastCS = nullptr;
877-
for (auto iterateCS = &CS; iterateCS; iterateCS = iterateCS->baseCS) {
878-
lastCS = iterateCS;
879-
contextualTypeNode = iterateCS->getContextualTypeNode();
880-
}
881-
882-
// The '~=' operator is an overloaded decl ref inside a binaryExpr
883-
if (auto binaryExpr = dyn_cast<BinaryExpr>(contextualTypeNode)) {
884-
if (auto overloadedFn
885-
= dyn_cast<OverloadedDeclRefExpr>(binaryExpr->getFn())) {
886-
if (!overloadedFn->getDecls().empty()) {
887-
// Fetch any declaration to check if the name is '~='
888-
ValueDecl *decl0 = overloadedFn->getDecls()[0];
889-
890-
if (decl0->getBaseName() == decl0->getASTContext().Id_MatchOperator) {
891-
assert(binaryExpr->getArg()->getElements().size() == 2);
892-
893-
// If the rhs of '~=' is the enum type, a single dot suffixes
894-
// since the type can be inferred
895-
Type secondArgType =
896-
lastCS->getType(binaryExpr->getArg()->getElement(1));
897-
if (secondArgType->isEqual(baseObjTy)) {
898-
Diag->fixItInsert(loc, ".");
899-
return;
900-
}
901-
}
902-
}
903-
}
904-
}
905-
906-
// Fall back to a fix-it with a full type qualifier
907-
auto nominal = member->getDeclContext()->getSelfNominalTypeDecl();
908-
SmallString<32> typeName;
909-
llvm::raw_svector_ostream typeNameStream(typeName);
910-
typeNameStream << nominal->getSelfInterfaceType() << ".";
911-
912-
Diag->fixItInsert(loc, typeNameStream.str());
913-
return;
914-
}
915-
916774
/// Given a result of name lookup that had no viable results, diagnose the
917775
/// unviable ones.
918776
void FailureDiagnosis::diagnoseUnviableLookupResults(
@@ -955,80 +813,20 @@ void FailureDiagnosis::diagnoseUnviableLookupResults(
955813
instanceTy, memberName)
956814
.highlight(baseRange).highlight(nameLoc.getSourceRange());
957815
return;
958-
case MemberLookupResult::UR_InstanceMemberOnType: {
959-
// If the base is an implicit self type reference, and we're in a
960-
// an initializer, then the user wrote something like:
961-
//
962-
// class Foo { let x = 1, y = x }
963-
//
964-
// which runs in type context, not instance context, or
965-
//
966-
// class Bar {
967-
// let otherwise = 1 // instance member
968-
// var x: Int
969-
// func init(x: Int =otherwise) { // default parameter
970-
// self.x = x
971-
// }
972-
// }
973-
//
974-
// in which an instance member is used as a default value for a
975-
// parameter.
976-
//
977-
// Produce a tailored diagnostic for these cases since this
978-
// comes up and is otherwise non-obvious what is going on.
979-
if (baseExpr && baseExpr->isImplicit() && isa<Initializer>(CS.DC)) {
980-
auto *TypeDC = CS.DC->getParent();
981-
bool propertyInitializer = true;
982-
// If the parent context is not a type context, we expect it
983-
// to be a defaulted parameter in a function declaration.
984-
if (!TypeDC->isTypeContext()) {
985-
assert(TypeDC->getContextKind() ==
986-
DeclContextKind::AbstractFunctionDecl &&
987-
"Expected function decl context for initializer!");
988-
TypeDC = TypeDC->getParent();
989-
propertyInitializer = false;
990-
}
991-
assert(TypeDC->isTypeContext() && "Expected type decl context!");
992-
993-
if (TypeDC->getSelfNominalTypeDecl() == instanceTy->getAnyNominal()) {
994-
if (propertyInitializer)
995-
CS.TC.diagnose(nameLoc, diag::instance_member_in_initializer,
996-
memberName);
997-
else
998-
CS.TC.diagnose(nameLoc, diag::instance_member_in_default_parameter,
999-
memberName);
1000-
return;
1001-
}
1002-
}
1003-
1004-
// Check whether the instance member is declared on parent context and if so
1005-
// provide more specialized message.
1006-
auto memberTypeContext = member->getDeclContext()->getInnermostTypeContext();
1007-
auto currentTypeContext = CS.DC->getInnermostTypeContext();
1008-
if (memberTypeContext && currentTypeContext &&
1009-
memberTypeContext->getSemanticDepth() <
1010-
currentTypeContext->getSemanticDepth()) {
1011-
diagnose(loc, diag::could_not_use_instance_member_on_type,
1012-
currentTypeContext->getDeclaredInterfaceType(), memberName,
1013-
memberTypeContext->getDeclaredInterfaceType(),
1014-
true)
1015-
.highlight(baseRange).highlight(nameLoc.getSourceRange());
1016-
} else {
1017-
diagnose(loc, diag::could_not_use_instance_member_on_type,
1018-
instanceTy, memberName,
1019-
instanceTy,
1020-
false)
1021-
.highlight(baseRange).highlight(nameLoc.getSourceRange());
1022-
}
816+
case MemberLookupResult::UR_InstanceMemberOnType:
817+
case MemberLookupResult::UR_TypeMemberOnInstance: {
818+
auto locatorKind = isa<SubscriptExpr>(E)
819+
? ConstraintLocator::SubscriptMember
820+
: ConstraintLocator::Member;
821+
AllowTypeOrInstanceMemberFailure failure(
822+
nullptr, CS, baseObjTy, memberName,
823+
CS.getConstraintLocator(E, locatorKind));
824+
auto diagnosed = failure.diagnoseAsError();
825+
assert(diagnosed &&
826+
"Failed to produce missing or extraneous metatype diagnostic");
827+
(void)diagnosed;
1023828
return;
1024829
}
1025-
1026-
case MemberLookupResult::UR_TypeMemberOnInstance:
1027-
diagnoseTypeMemberOnInstanceLookup(baseObjTy, baseExpr,
1028-
memberName, nameLoc,
1029-
member, loc);
1030-
return;
1031-
1032830
case MemberLookupResult::UR_MutatingMemberOnRValue:
1033831
case MemberLookupResult::UR_MutatingGetterOnRValue: {
1034832
auto diagIDsubelt = diag::cannot_pass_rvalue_mutating_subelement;
@@ -1062,9 +860,6 @@ void FailureDiagnosis::diagnoseUnviableLookupResults(
1062860
}
1063861
}
1064862

1065-
// FIXME: Emit candidate set....
1066-
1067-
1068863
// Otherwise, we don't have a specific issue to diagnose. Just say the vague
1069864
// 'cannot use' diagnostic.
1070865
if (!baseObjTy->isEqual(instanceTy))
@@ -3437,7 +3232,7 @@ diagnoseInstanceMethodAsCurriedMemberOnType(CalleeCandidateInfo &CCI,
34373232
} else {
34383233
TC.diagnose(UDE->getLoc(), diag::could_not_use_instance_member_on_type,
34393234
instanceType, UDE->getName(), instanceType, false)
3440-
.highlight(baseExpr->getSourceRange());
3235+
.highlight(baseExpr->getSourceRange());
34413236
}
34423237
return true;
34433238
}
@@ -5177,7 +4972,9 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
51774972
auto diag = diagnose(arg->getStartLoc(),
51784973
diag::missing_init_on_metatype_initialization);
51794974
diag.highlight(fnExpr->getSourceRange());
5180-
} else {
4975+
}
4976+
4977+
if (!fnType->is<ExistentialMetatypeType>()) {
51814978
auto diag = diagnose(arg->getStartLoc(),
51824979
diag::cannot_call_non_function_value, fnType);
51834980
diag.highlight(fnExpr->getSourceRange());
@@ -5625,6 +5422,36 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
56255422
// If we have an argument list (i.e., a scalar, or a non-zero-element tuple)
56265423
// then diagnose with some specificity about the arguments.
56275424
bool isInitializer = isa<TypeExpr>(fnExpr);
5425+
if (!fnType->is<AnyMetatypeType>() &&
5426+
((isa<TupleExpr>(argExpr) &&
5427+
cast<TupleExpr>(argExpr)->getNumElements() == 1) ||
5428+
(isa<ParenExpr>(argExpr) &&
5429+
!isa<LoadExpr>(cast<ParenExpr>(argExpr)->getSubExpr())))) {
5430+
if (auto ctorRef = dyn_cast<UnresolvedDotExpr>(fnExpr)) {
5431+
if (ctorRef->getName().isSimpleName(DeclBaseName::createConstructor())) {
5432+
// Diagnose 'super.init', which can only appear inside another
5433+
// initializer, specially.
5434+
if (isa<SuperRefExpr>(ctorRef->getBase())) {
5435+
diagnose(fnExpr->getLoc(),
5436+
diag::super_initializer_not_in_initializer);
5437+
calleeInfo.suggestPotentialOverloads(fnExpr->getLoc());
5438+
return true;
5439+
}
5440+
5441+
// Suggest inserting a call to 'type(of:)' to construct another object
5442+
// of the same dynamic type.
5443+
SourceRange fixItRng = ctorRef->getNameLoc().getSourceRange();
5444+
5445+
// Surround the caller in `type(of:)`.
5446+
diagnose(fnExpr->getLoc(), diag::init_not_instance_member)
5447+
.fixItInsert(fixItRng.Start, "type(of: ")
5448+
.fixItInsertAfter(fixItRng.End, ")");
5449+
calleeInfo.suggestPotentialOverloads(fnExpr->getLoc());
5450+
return true;
5451+
}
5452+
}
5453+
}
5454+
56285455
if (isa<TupleExpr>(argExpr) &&
56295456
cast<TupleExpr>(argExpr)->getNumElements() == 0) {
56305457
// Emit diagnostics that say "no arguments".

0 commit comments

Comments
 (0)