Skip to content

[CSDiag] Remove obsolete function diagnostics from visitApplyExpr #28783

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 3 commits into from
Dec 16, 2019
Merged

[CSDiag] Remove obsolete function diagnostics from visitApplyExpr #28783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a9106ca
[ConstraintSystem] Account for missing unwrap(s) in call to optional …
xedin Dec 5, 2019
d77e349
[ConstraintSystem] Lift a restriction on fixing of non-function calls…
xedin Dec 13, 2019
2db14a9
[CSDiag] NFC: Remove obsolete function diagnostics from `visitApplyExpr`
xedin Dec 13, 2019
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
223 changes: 0 additions & 223 deletions lib/Sema/CSDiag.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,12 +201,6 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
ContextualTypePurpose CTP,
Type suggestedType = Type());

/// Attempt to produce a diagnostic for a mismatch between a call's
/// type and its assumed contextual type.
bool diagnoseCallContextualConversionErrors(ApplyExpr *callEpxr,
Type contextualType,
ContextualTypePurpose CTP);

bool diagnoseImplicitSelfErrors(Expr *fnExpr, Expr *argExpr,
CalleeCandidateInfo &CCI,
ArrayRef<Identifier> argLabels);
Expand Down Expand Up @@ -1671,36 +1665,6 @@ namespace {
};
} // end anonymous namespace

/// Check if there failure associated with expression is related
/// to given contextual type.
bool FailureDiagnosis::diagnoseCallContextualConversionErrors(
ApplyExpr *callExpr, Type contextualType, ContextualTypePurpose CTP) {
if (!contextualType || contextualType->hasUnresolvedType())
return false;

auto typeCheckExpr = [&](Expr *expr, DeclContext *DC,
SmallPtrSetImpl<TypeBase *> &types) {
getPossibleTypesOfExpressionWithoutApplying(
expr, DC, types, FreeTypeVariableBinding::Disallow);
};

// First let's type-check expression without contextual type, and
// see if that's going to produce a type, if so, let's type-check
// again, this time using given contextual type.
SmallPtrSet<TypeBase *, 4> withoutContextual;
typeCheckExpr(callExpr, CS.DC, withoutContextual);

// If there are no types returned, it means that problem was
// nothing to do with contextual information, probably parameter/argument
// mismatch.
if (withoutContextual.empty())
return false;

Type exprType = withoutContextual.size() == 1 ? *withoutContextual.begin() : Type();
return diagnoseContextualConversionError(callExpr, contextualType, CTP,
exprType);
}

// Check if there is a structural problem in the function expression
// by performing type checking with the option to allow unresolved
// type variables. If that is going to produce a function type with
Expand Down Expand Up @@ -1731,45 +1695,8 @@ static bool shouldTypeCheckFunctionExpr(FailureDiagnosis &FD, DeclContext *DC,
return true;
}

// Check if any candidate of the overload set can accept a specified
// number of arguments, regardless of parameter type or label information.
static bool isViableOverloadSet(const CalleeCandidateInfo &CCI,
size_t numArgs) {
for (unsigned i = 0; i < CCI.size(); ++i) {
auto &&cand = CCI[i];
auto funcDecl = dyn_cast_or_null<AbstractFunctionDecl>(cand.getDecl());

// If we don't have a func decl or we haven't resolved its parameters,
// continue. The latter case can occur with `type(of:)`, which is introduced
// as a type variable.
if (!funcDecl || !cand.hasParameters())
continue;

auto params = cand.getParameters();
bool hasVariadicParameter = false;
auto pairMatcher = [&](unsigned argIdx, unsigned paramIdx) {
hasVariadicParameter |= params[paramIdx].isVariadic();
return true;
};

auto paramInfo = cand.getParameterListInfo(params);
InputMatcher IM(params, paramInfo);
auto result = IM.match(numArgs, pairMatcher);
if (result == InputMatcher::IM_Succeeded)
return true;
if (result == InputMatcher::IM_HasUnclaimedInput && hasVariadicParameter)
return true;
}
return false;
}

bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
if (diagnoseCallContextualConversionErrors(callExpr, CS.getContextualType(),
CS.getContextualTypePurpose()))
return true;

auto *fnExpr = callExpr->getFn();
auto originalFnType = CS.getType(callExpr->getFn());

if (shouldTypeCheckFunctionExpr(*this, CS.DC, fnExpr)) {
// Type check the function subexpression to resolve a type for it if
Expand All @@ -1792,162 +1719,12 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {

auto fnType = getFuncType(CS.getType(fnExpr));

// Let's see if this has to do with member vs. property error
// because sometimes when there is a member and a property declared
// on the nominal type with the same name. Type-checking function
// expression separately from arguments might produce solution for
// the property instead of the member.
if (!fnType->is<AnyFunctionType>() &&
isa<UnresolvedDotExpr>(callExpr->getFn())) {
fnExpr = callExpr->getFn();

SmallPtrSet<TypeBase *, 4> types;
getPossibleTypesOfExpressionWithoutApplying(fnExpr, CS.DC, types);

auto isFunctionType = [getFuncType](Type type) -> bool {
return type && getFuncType(type)->is<AnyFunctionType>();
};

auto fnTypes = std::find_if(types.begin(), types.end(), isFunctionType);
if (fnTypes != types.end()) {
auto funcType = getFuncType(*fnTypes);
// If there is only one function type, let's use it.
if (std::none_of(std::next(fnTypes), types.end(), isFunctionType))
fnType = funcType;
} else {
fnType = getFuncType(originalFnType);
}
}

// If we have a contextual type, and if we have an ambiguously typed function
// result from our previous check, we re-type-check it using this contextual
// type to inform the result type of the callee.
//
// We only do this as a second pass because the first pass we just did may
// return something of obviously non-function-type. If this happens, we
// produce better diagnostics below by diagnosing this here rather than trying
// to peel apart the failed conversion to function type.
if (CS.getContextualType() &&
(isUnresolvedOrTypeVarType(fnType) ||
(fnType->is<AnyFunctionType>() && fnType->hasUnresolvedType()))) {
// FIXME: Prevent typeCheckChildIndependently from transforming expressions,
// because if we try to typecheck OSR expression with contextual type,
// it'll end up converting it into DeclRefExpr based on contextual info,
// instead let's try to get a type without applying and filter callee
// candidates later on.
CalleeListener listener(CS.getContextualType());

if (isa<OverloadSetRefExpr>(fnExpr)) {
assert(!cast<OverloadSetRefExpr>(fnExpr)->getReferencedDecl() &&
"unexpected declaration reference");

ConcreteDeclRef decl = nullptr;
Type type = TypeChecker::getTypeOfExpressionWithoutApplying(
fnExpr, CS.DC, decl, FreeTypeVariableBinding::UnresolvedType,
&listener);

if (type)
fnType = getFuncType(type);
} else {
fnExpr = typeCheckChildIndependently(callExpr->getFn(), Type(),
CTP_CalleeResult, TCC_ForceRecheck,
&listener);
if (!fnExpr)
return true;

fnType = getFuncType(CS.getType(fnExpr));
}
}

// If we resolved a concrete expression for the callee, and it has
// non-function/non-metatype type, then we cannot call it!
if (!isUnresolvedOrTypeVarType(fnType) &&
!fnType->is<AnyFunctionType>() && !fnType->is<MetatypeType>()) {
auto arg = callExpr->getArg();

// If the argument is a trailing ClosureExpr (i.e. {....}) and it is on
// the line after the callee, then it's likely the user forgot to
// write "do" before their brace stmt.
// Note that line differences of more than 1 are diagnosed during parsing.
if (auto *PE = dyn_cast<ParenExpr>(arg)) {
if (PE->hasTrailingClosure() && isa<ClosureExpr>(PE->getSubExpr())) {
auto *closure = cast<ClosureExpr>(PE->getSubExpr());
auto &SM = CS.getASTContext().SourceMgr;
if (closure->hasAnonymousClosureVars() &&
closure->getParameters()->size() == 0 &&
1 + SM.getLineNumber(callExpr->getFn()->getEndLoc()) ==
SM.getLineNumber(closure->getStartLoc())) {
diagnose(closure->getStartLoc(), diag::brace_stmt_suggest_do)
.fixItInsert(closure->getStartLoc(), "do ");
return true;
}
}
}

auto isExistentialMetatypeType = fnType->is<ExistentialMetatypeType>();
if (isExistentialMetatypeType) {
auto diag = diagnose(arg->getStartLoc(),
diag::missing_init_on_metatype_initialization);
diag.highlight(fnExpr->getSourceRange());
return true;
} else {
auto diag = diagnose(arg->getStartLoc(),
diag::cannot_call_non_function_value, fnType);
diag.highlight(fnExpr->getSourceRange());

// If the argument is an empty tuple, then offer a
// fix-it to remove the empty tuple and use the value
// directly.
if (auto tuple = dyn_cast<TupleExpr>(arg)) {
if (tuple->getNumElements() == 0) {
diag.fixItRemove(arg->getSourceRange());
}
}
return true;
}
}

bool hasTrailingClosure = callArgHasTrailingClosure(callExpr->getArg());

// Collect a full candidate list of callees based on the partially type
// checked function.
CalleeCandidateInfo calleeInfo(fnExpr, hasTrailingClosure, CS);

// In the case that function subexpression was resolved independently in
// the first place, the resolved type may not provide the best diagnostic.
// We consider the number of arguments to decide whether we'd go with it or
// stay with the original one.
if (fnExpr != callExpr->getFn()) {
bool isInstanceMethodAsCurriedMemberOnType = false;
if (!calleeInfo.empty()) {
auto &&cand = calleeInfo[0];
auto decl = cand.getDecl();
if (decl && decl->isInstanceMember() && !cand.skipCurriedSelf &&
cand.getParameters().size() == 1)
isInstanceMethodAsCurriedMemberOnType = true;
}

// In terms of instance method as curried member on type, we should not
// take the number of arguments into account.
if (!isInstanceMethodAsCurriedMemberOnType) {
size_t numArgs = 1;
auto arg = callExpr->getArg();
if (auto tuple = dyn_cast<TupleExpr>(arg)) {
numArgs = tuple->getNumElements();
}

if (!isViableOverloadSet(calleeInfo, numArgs)) {
CalleeCandidateInfo calleeInfoOrig(callExpr->getFn(),
hasTrailingClosure, CS);
if (isViableOverloadSet(calleeInfoOrig, numArgs)) {
fnExpr = callExpr->getFn();
fnType = getFuncType(CS.getType(fnExpr));
calleeInfo = calleeInfoOrig;
}
}
}
}

// Filter list of the candidates based on the known function type.
if (auto fn = fnType->getAs<AnyFunctionType>()) {
using Closeness = CalleeCandidateInfo::ClosenessResultTy;
Expand Down
13 changes: 13 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5598,6 +5598,19 @@ bool ExtraneousCallFailure::diagnoseAsError() {
}
}

if (auto *UDE = dyn_cast<UnresolvedDotExpr>(anchor)) {
auto *baseExpr = UDE->getBase();
auto *call = cast<CallExpr>(getRawAnchor());

if (getType(baseExpr)->isAnyObject()) {
emitDiagnostic(anchor->getLoc(), diag::cannot_call_with_params,
UDE->getName().getBaseName().userFacingName(),
getType(call->getArg())->getString(),
isa<TypeExpr>(baseExpr));
return true;
}
}

auto diagnostic = emitDiagnostic(
anchor->getLoc(), diag::cannot_call_non_function_value, getType(anchor));
removeParensFixIt(diagnostic);
Expand Down
25 changes: 20 additions & 5 deletions lib/Sema/CSSimplify.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7427,6 +7427,26 @@ ConstraintSystem::simplifyApplicableFnConstraint(
// Track how many times we do this so that we can record a fix for each.
++unwrapCount;
}

// Let's account for optional members concept from Objective-C
// which forms a disjunction for member type to check whether
// it would be possible to use optional type directly or it has
// to be force unwrapped (because such types are imported as IUO).
if (unwrapCount > 0 && desugar2->is<TypeVariableType>()) {
auto *typeVar = desugar2->castTo<TypeVariableType>();
auto *locator = typeVar->getImpl().getLocator();
if (locator->isLastElement<LocatorPathElt::Member>()) {
auto *fix = ForceOptional::create(*this, origType2, desugar2,
getConstraintLocator(locator));
if (recordFix(fix, /*impact=*/unwrapCount))
return SolutionKind::Error;

// Since the right-hand side of the constraint has been changed
// we have to re-generate this constraint to use new type.
flags |= TMF_GenerateConstraints;
return formUnsolved();
}
}
}

// For a function, bind the output and convert the argument to the input.
Expand Down Expand Up @@ -7500,11 +7520,6 @@ ConstraintSystem::simplifyApplicableFnConstraint(
desugar2->is<AnyMetatypeType>())
return SolutionKind::Error;

if (auto objectTy = desugar2->lookThroughAllOptionalTypes()) {
if (objectTy->isAny() || objectTy->isAnyObject())
return SolutionKind::Error;
}

// If there are any type variables associated with arguments/result
// they have to be marked as "holes".
type1.visit([&](Type subType) {
Expand Down
16 changes: 12 additions & 4 deletions test/Constraints/iuo_objc.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,29 @@ func iuo_error(prop: IUOProperty) {
// expected-note@-2{{coalesce}}
// expected-note@-3{{force-unwrap}}
let _: Coat? = prop.iuo.optional()!
// expected-error@-1 {{cannot invoke 'optional' with no arguments}}
// expected-error@-1 {{value of optional type '(() -> Coat?)?' must be unwrapped to a value of type '() -> Coat?'}}
// expected-note@-2{{coalesce}}
// expected-note@-3{{force-unwrap}}
let _: Coat? = prop.iuo.optional!()
let _: Coat? = prop.iuo.optional!()!
let _: Coat? = prop.iuo!.optional()
// expected-error@-1 {{value of optional type '(() -> Coat?)?' must be unwrapped}}
// expected-note@-2{{coalesce}}
// expected-note@-3{{force-unwrap}}
let _: Coat? = prop.iuo!.optional()!
// expected-error@-1 {{cannot invoke 'optional' with no arguments}}
// expected-error@-1 {{value of optional type '(() -> Coat?)?' must be unwrapped to a value of type '() -> Coat?'}}
// expected-note@-2{{coalesce}}
// expected-note@-3{{force-unwrap}}
let _: Coat? = prop.iuo!.optional!()
let _: Coat? = prop.iuo!.optional!()!
let _: Coat = prop.iuo.optional()
// expected-error@-1 {{value of optional type '(() -> Coat)?' must be unwrapped}}
// expected-note@-2{{coalesce}}
// expected-note@-3{{force-unwrap}}
let _: Coat = prop.iuo.optional()!
// expected-error@-1 {{cannot invoke 'optional' with no arguments}}
// expected-error@-1 {{value of optional type '(() -> Coat?)?' must be unwrapped to a value of type '() -> Coat?'}}
// expected-note@-2{{coalesce}}
// expected-note@-3{{force-unwrap}}
let _: Coat = prop.iuo.optional!()
let _: Coat = prop.iuo.optional!()!
let _: Coat = prop.iuo!.optional()
Expand All @@ -34,7 +40,9 @@ func iuo_error(prop: IUOProperty) {
// expected-note@-3{{force-unwrap}}

let _: Coat = prop.iuo!.optional()!
// expected-error@-1 {{cannot invoke 'optional' with no arguments}}
// expected-error@-1 {{value of optional type '(() -> Coat?)?' must be unwrapped to a value of type '() -> Coat?'}}
// expected-note@-2{{coalesce}}
// expected-note@-3{{force-unwrap}}
let _: Coat = prop.iuo!.optional!()
let _: Coat = prop.iuo!.optional!()!

Expand Down