Skip to content

Commit

Permalink
add split-init error when uninitialized var used as actual (#26497)
Browse files Browse the repository at this point in the history 
This adds detection of an uninitialized variable used as an actual to a
non-out formal. The previous error was a generic failure to resolve
function [NoMatchingCandidates] and this improves the message by
indicating that the candidate did not match because of the uninitialized
actual passed to a non-out formal.

resolves Cray/chapel-private#6624

TESTING:

- [x] paratest `[Summary: #Successes = 17721 | #Failures = 0 | #Futures
= 904]`
- [x] gasnet paratest `[Summary: #Successes = 17906 | #Failures = 0 |
#Futures = 915]`

[reviewed by @DanilaFe - thank you!]
  • Loading branch information
@arezaii
arezaii authored Jan 28, 2025
2 parents 6d21724 + 37ef570 commit 49651a3
Show file tree
Hide file tree
Showing 8 changed files with 191 additions and 33 deletions.
2 changes: 1 addition & 1 deletion frontend/include/chpl/resolution/resolution-error-classes-list.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ ERROR_CLASS(MultipleEnumElems, const uast::AstNode*, chpl::UniqueString, const u
ERROR_CLASS(MultipleInheritance, const uast::Class*, const uast::AstNode*, const uast::AstNode*)
ERROR_CLASS(MultipleQuestionArgs, const uast::FnCall*, const uast::AstNode*, const uast::AstNode*)
ERROR_CLASS(NestedClassFieldRef, const uast::TypeDecl*, const uast::TypeDecl*, const uast::AstNode*, ID)
ERROR_CLASS(NoMatchingCandidates, const uast::AstNode*, resolution::CallInfo, std::vector<resolution::ApplicabilityResult>)
ERROR_CLASS(NoMatchingCandidates, const uast::AstNode*, resolution::CallInfo, std::vector<resolution::ApplicabilityResult>, std::vector<const uast::VarLikeDecl*>)
ERROR_CLASS(NonClassInheritance, const uast::AggregateDecl*, const uast::AstNode*, const types::Type*)
ERROR_CLASS(NonIterable, const uast::AstNode*, const uast::AstNode*, types::QualifiedType, std::vector<std::tuple<uast::Function::IteratorKind, chpl::resolution::TheseResolutionResult>>)
ERROR_CLASS(NoMatchingEnumValue, const uast::AstNode*, const types::EnumType*, types::QualifiedType)
Expand Down
5 changes: 5 additions & 0 deletions frontend/include/chpl/resolution/resolution-types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1431,6 +1431,11 @@ class ApplicabilityResult {
PassingFailureReason formalReason() const { return formalReason_; }

int formalIdx() const { return formalIdx_; }

inline bool failedDueToWrongActual() const {
return candidateReason_ == resolution::FAIL_CANNOT_PASS &&
formalReason_ != resolution::FAIL_UNKNOWN_FORMAL_TYPE;
}
};

/** FormalActual holds information on a function formal and its binding (if any) */
Expand Down
75 changes: 63 additions & 12 deletions frontend/lib/resolution/Resolver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -984,6 +984,51 @@ static bool isCallToPtr(const AstNode* formalTypeExpr) {
return false;
}

// helper to gather bad actuals and report NoMatchingCandidates error
static void
handleRejectedCandidates(Context* context,
ResolutionResultByPostorderID& byPostorder,
std::vector<ApplicabilityResult>& rejected,
const resolution::CallInfo& ci,
const uast::Call*& call,
const std::vector<const uast::AstNode*>& actualAsts) {
// By performing some processing in the resolver, we can issue a nicer error
// explaining why each candidate was rejected.
std::vector<const uast::VarLikeDecl*> actualDecls;
actualDecls.resize(rejected.size());
// check each rejected candidate for uninitialized actuals
for (size_t i = 0; i < rejected.size(); i++) {
auto &candidate = rejected[i];
if (/* skip computing the formal-actual map because it will go poorly
with an unknown formal. */
!candidate.failedDueToWrongActual()) {
continue;
}
auto fn = candidate.initialForErr();
resolution::FormalActualMap fa(fn, ci);
auto& badPass = fa.byFormalIdx(candidate.formalIdx());
const uast::AstNode *actualExpr = nullptr;
const uast::VarLikeDecl *actualDecl = nullptr;
size_t actualIdx = badPass.actualIdx();
CHPL_ASSERT(0 <= actualIdx && actualIdx < actualAsts.size());
actualExpr = actualAsts[badPass.actualIdx()];

// look for a definition point of the actual for error reporting of
// uninitialized vars typically in the case of bad split-initialization
if (actualExpr && actualExpr->isIdentifier()) {
auto &resolvedExpr = byPostorder.byAst(actualExpr);
if (auto id = resolvedExpr.toId()) {
auto var = parsing::idToAst(context, id);
// should put a nullptr if not a VarLikeDecl
actualDecl = var->toVarLikeDecl();
}
}
actualDecls[i] = actualDecl;
}
CHPL_ASSERT(rejected.size() == actualDecls.size());
CHPL_REPORT(context, NoMatchingCandidates, call, ci, rejected, actualDecls);
}

static void varArgTypeQueryError(Context* context,
const AstNode* node,
ResolvedExpression& result) {
Expand Down Expand Up @@ -1898,9 +1943,11 @@ Resolver::issueErrorForFailedCallResolution(const uast::AstNode* astForErr,
context->error(astForErr, "Cannot resolve call to '%s': ambiguity",
ci.name().c_str());
} else {
std::vector<const uast::VarLikeDecl*> uninitializedActuals;
// could not find a most specific candidate
std::vector<ApplicabilityResult> rejected;
CHPL_REPORT(context, NoMatchingCandidates, astForErr, ci, rejected);
CHPL_ASSERT(rejected.size() == uninitializedActuals.size());
CHPL_REPORT(context, NoMatchingCandidates, astForErr, ci, rejected, uninitializedActuals);
}
} else {
context->error(astForErr, "Cannot establish type for call expression");
Expand Down Expand Up @@ -2022,6 +2069,7 @@ void Resolver::handleResolvedCallPrintCandidates(ResolvedExpression& r,
const CallScopeInfo& inScopes,
const QualifiedType& receiverType,
const CallResolutionResult& c,
std::vector<const uast::AstNode*>& actualAsts,
optional<ActionAndId> actionAndId) {
bool wasCallGenerated = (bool) actionAndId;
CHPL_ASSERT(!wasCallGenerated || receiverType.isUnknown());
Expand All @@ -2041,9 +2089,8 @@ void Resolver::handleResolvedCallPrintCandidates(ResolvedExpression& r,
}

if (!rejected.empty()) {
// There were candidates but we threw them out. We can issue a nicer
// error explaining why each candidate was rejected.
CHPL_REPORT(context, NoMatchingCandidates, call, ci, rejected);
// There were candidates but we threw them out. Report on those.
handleRejectedCandidates(context, byPostorder, rejected, ci, call, actualAsts);
return;
}
}
Expand Down Expand Up @@ -2392,28 +2439,31 @@ bool Resolver::resolveSpecialNewCall(const Call* call) {
bool isMethodCall = true;
const AstNode* questionArg = nullptr;
std::vector<CallInfoActual> actuals;
std::vector<const uast::AstNode*> actualAsts;

// Prepare receiver.
auto receiverInfo = CallInfoActual(calledType, USTR("this"));
actuals.push_back(std::move(receiverInfo));

actualAsts.push_back(newExpr->typeExpression());
// Remaining actuals.
prepareCallInfoActuals(call, actuals, questionArg);
prepareCallInfoActuals(call, actuals, questionArg, &actualAsts);
CHPL_ASSERT(!questionArg);

// The 'new' will produce an 'init' call as a side effect.
auto ci = CallInfo(USTR("init"), calledType, isMethodCall,
/* hasQuestionArg */ questionArg != nullptr,
/* isParenless */ false,
std::move(actuals));
CHPL_ASSERT(actualAsts.size() == (size_t)ci.numActuals());
auto inScope = scopeStack.back();
auto inPoiScope = poiScope;
auto inScopes = CallScopeInfo::forNormalCall(inScope, inPoiScope);

// note: the resolution machinery will get compiler generated candidates
auto crr = resolveGeneratedCall(context, call, ci, inScopes);
handleResolvedCallPrintCandidates(re, call, ci, inScopes, QualifiedType(), crr,
{ { AssociatedAction::NEW_INIT, call->id() } });
optional<ActionAndId> action({ AssociatedAction::NEW_INIT, call->id() });
handleResolvedCallPrintCandidates(re, call, ci, inScopes, QualifiedType(),
crr, actualAsts, action);


// there should be one or zero applicable candidates
Expand Down Expand Up @@ -2567,7 +2617,7 @@ bool Resolver::resolveSpecialKeywordCall(const Call* call) {
DomainType::getDefaultDistType(context), UniqueString());
actuals.push_back(std::move(defaultDistArg));
// Remaining given args from domain() call as written
prepareCallInfoActuals(call, actuals, questionArg);
prepareCallInfoActuals(call, actuals, questionArg, /*actualAsts*/ nullptr);
CHPL_ASSERT(!questionArg);

auto ci =
Expand Down Expand Up @@ -4478,11 +4528,12 @@ bool Resolver::enter(const Call* call) {

void Resolver::prepareCallInfoActuals(const Call* call,
std::vector<CallInfoActual>& actuals,
const AstNode*& questionArg) {
const AstNode*& questionArg,
std::vector<const uast::AstNode*>* actualAsts) {
CallInfo::prepareActuals(context, call, byPostorder,
/* raiseErrors */ true,
actuals, questionArg,
/* actualAsts */ nullptr);
actualAsts);
}

static const Type* getGenericType(Context* context, const Type* recv) {
Expand Down Expand Up @@ -4750,7 +4801,7 @@ void Resolver::handleCallExpr(const uast::Call* call) {
rejected);

// save the most specific candidates in the resolution result for the id
handleResolvedCallPrintCandidates(r, call, ci, inScopes, receiverType, c);
handleResolvedCallPrintCandidates(r, call, ci, inScopes, receiverType, c, actualAsts);

// handle type inference for variables split-inited by 'out' formals
adjustTypesForOutFormals(ci, actualAsts, c.mostSpecific());
Expand Down
4 changes: 3 additions & 1 deletion frontend/lib/resolution/Resolver.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ struct Resolver {
const CallScopeInfo& inScopes,
const types::QualifiedType& receiverType,
const CallResolutionResult& c,
std::vector<const uast::AstNode*>& actualAsts,
optional<ActionAndId> associatedActionAndId = {});

// If the variable with the passed ID has unknown or generic type,
Expand Down Expand Up @@ -597,7 +598,8 @@ struct Resolver {
// includes special handling for operators and tuple literals
void prepareCallInfoActuals(const uast::Call* call,
std::vector<CallInfoActual>& actuals,
const uast::AstNode*& questionArg);
const uast::AstNode*& questionArg,
std::vector<const uast::AstNode*>* actualAsts);

// prepare a CallInfo by inspecting the called expression and actuals
CallInfo prepareCallInfoNormalCall(const uast::Call* call);
Expand Down
53 changes: 34 additions & 19 deletions frontend/lib/resolution/resolution-error-classes-list.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -644,19 +644,17 @@ static void printRejectedCandidates(ErrorWriterBase& wr,
const char* passedThing,
const char* expectedThingArticle,
const char* expectedThing,
GetActual&& getActual) {
GetActual&& getActual,
const std::vector<const uast::VarLikeDecl*>& actualDecls) {
unsigned int printCount = 0;
static const unsigned int maxPrintCount = 2;
for (auto& candidate : rejected) {
if (printCount == maxPrintCount) break;
printCount++;

auto reason = candidate.reason();
wr.message("");
if (reason == resolution::FAIL_CANNOT_PASS &&
/* skip printing detailed info_ here because computing the formal-actual
map will go poorly with an unknown formal. */
candidate.formalReason() != resolution::FAIL_UNKNOWN_FORMAL_TYPE) {
auto reason = candidate.reason();
if (/* skip printing detailed info_ here because computing the formal-actual
map will go poorly with an unknown formal. */
candidate.failedDueToWrongActual()) {
auto fn = candidate.initialForErr();
resolution::FormalActualMap fa(fn, ci);
auto badPass = fa.byFormalIdx(candidate.formalIdx());
Expand All @@ -672,22 +670,39 @@ static void printRejectedCandidates(ErrorWriterBase& wr,
} else if (formalDecl->isTupleDecl()) {
formalName = "'" + buildTupleDeclName(formalDecl->toTupleDecl()) + "'";
}

bool actualPrinted = false;
const uast::VarLikeDecl* offendingActual = actualDecls.at(printCount);
if (badPass.formalType().isUnknown()) {
// The formal type can be unknown in an initial instantiation if it
// depends on the previous formals' types. In that case, don't print it
// and say something nicer.
wr.message("The instantiated type of ", expectedThing, " ", formalName,
" does not allow ", passedThing, "s of type '", badPass.actualType().type(), "'.");
} else if (badPass.actualType().isUnknown() &&
offendingActual &&
!offendingActual->initExpression() &&
!offendingActual->typeExpression()) {
auto formalKind = badPass.formalType().kind();
auto actualName = "'" + actualExpr->toIdentifier()->name().str() + "'";
wr.message("The actual ", actualName,
" expects to be split-initialized because it is declared without a type or initialization expression here:");
wr.codeForDef(offendingActual);
wr.message("The call to '", ci.name() ,"' occurs before any valid initialization points:");
wr.code(actualExpr, { actualExpr });
actualPrinted = true;
wr.message("The call to '", ci.name(), "' cannot initialize ",
actualName,
" because only 'out' formals can be used to split-initialize. However, ",
actualName, " is passed to formal ", formalName, " which has intent '", formalKind, "'.");

} else {
wr.message("The ", expectedThing, " ", formalName, " expects ", badPass.formalType(),
", but the ", passedThing, " was ", badPass.actualType(), ".");
}

if (actualExpr) {
if (!actualPrinted && actualExpr) {
wr.code(actualExpr, { actualExpr });
}

auto formalReason = candidate.formalReason();
if (formalReason == resolution::FAIL_INCOMPATIBLE_NILABILITY) {
auto formalDec = badPass.formalType().type()->toClassType()->decorator();
Expand Down Expand Up @@ -769,6 +784,7 @@ static void printRejectedCandidates(ErrorWriterBase& wr,
}
wr.code(candidate.idForErr());
}
printCount++;
}

if (printCount < rejected.size()) {
Expand Down Expand Up @@ -824,18 +840,17 @@ void ErrorInterfaceMissingAssociatedType::write(ErrorWriterBase& wr) const {
auto var = std::get<const uast::Variable*>(info_);
auto ci = std::get<resolution::CallInfo>(info_);
auto rejected = std::get<std::vector<resolution::ApplicabilityResult>>(info_);

auto actualDecls = std::vector<const uast::VarLikeDecl*>(rejected.size(), nullptr);
wr.heading(kind_, type_, implPoint, "unable to find matching candidates for associated type '", var->name(), "'.");
wr.codeForDef(var->id());
wr.message("Required by the interface '", interface->name(), "':");
wr.codeForDef(interface->id());
wr.note(implPoint, "while checking the implementation point here:");
wr.code<ID>(implPoint, { implPoint });
wr.message("Associated types are resolved as 'type' calls on types constrained by the interface.");

printRejectedCandidates(wr, implPoint, ci, rejected, "an", "actual", "a", "formal", [](int) -> const uast::AstNode* {
return nullptr;
});
}, actualDecls);
}

void ErrorInterfaceMissingFn::write(ErrorWriterBase& wr) const {
Expand All @@ -844,20 +859,19 @@ void ErrorInterfaceMissingFn::write(ErrorWriterBase& wr) const {
auto fn = std::get<const resolution::TypedFnSignature*>(info_);
auto ci = std::get<resolution::CallInfo>(info_);
auto rejected = std::get<std::vector<resolution::ApplicabilityResult>>(info_);

auto actualDecls = std::vector<const uast::VarLikeDecl*>(rejected.size(), nullptr);
wr.heading(kind_, type_, implPoint, "unable to find matching candidates for function '", fn->untyped()->name(), "'.");
wr.codeForDef(fn->id());
wr.message("Required by the interface '", interface->name(), "':");
wr.codeForDef(interface->id());
wr.note(implPoint, "while checking the implementation point here:");
wr.code<ID>(implPoint, { implPoint });

printRejectedCandidates(wr, implPoint, ci, rejected, "a", "required formal", "a", "cadidate formal", [fn](int idx) -> const uast::AstNode* {
printRejectedCandidates(wr, implPoint, ci, rejected, "a", "required formal", "a", "candidate formal", [fn](int idx) -> const uast::AstNode* {
if (idx >= 0 && idx < fn->numFormals()) {
return fn->untyped()->formalDecl(idx);
}
return nullptr;
});
}, actualDecls);
}

void ErrorInterfaceMultipleImplements::write(ErrorWriterBase& wr) const {
Expand Down Expand Up @@ -1313,6 +1327,7 @@ void ErrorNoMatchingCandidates::write(ErrorWriterBase& wr) const {
auto call = node->toCall();
auto& ci = std::get<resolution::CallInfo>(info_);
auto& rejected = std::get<std::vector<resolution::ApplicabilityResult>>(info_);
auto& actualDecls = std::get<std::vector<const uast::VarLikeDecl*>>(info_);

wr.heading(kind_, type_, node, "unable to resolve call to '", ci.name(), "': no matching candidates.");
wr.code(node);
Expand All @@ -1322,7 +1337,7 @@ void ErrorNoMatchingCandidates::write(ErrorWriterBase& wr) const {
return call->actual(idx);
}
return nullptr;
});
}, actualDecls);
}

void ErrorNonClassInheritance::write(ErrorWriterBase& wr) const {
Expand Down
5 changes: 5 additions & 0 deletions test/errors/resolution/noMatchingCandidates.1.good
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
noMatchingCandidates.chpl:6: error: unable to resolve call to 'fn': no matching candidates
noMatchingCandidates.chpl:2: note: the following candidate didn't match because an actual couldn't be passed to a formal
noMatchingCandidates.chpl:6: note: The actual 'x' expects to be split-initialized because it is declared without a type or initialization expression here
noMatchingCandidates.chpl:14: error: unable to resolve call to 'fn': no matching candidates
noMatchingCandidates.chpl:10: note: the following candidate didn't match because an actual couldn't be passed to a formal
45 changes: 45 additions & 0 deletions test/errors/resolution/noMatchingCandidates.2.good
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
─── error in noMatchingCandidates.chpl:6 [NoMatchingCandidates] ───
Unable to resolve call to 'fn': no matching candidates.
|
6 | fn(x);
|

The following candidate didn't match because an actual couldn't be passed to a formal:
|
2 | proc fn(const arg) {
| ⎺⎺⎺⎺⎺⎺⎺⎺⎺
3 | arg;
4 | }
|
The actual 'x' expects to be split-initialized because it is declared without a type or initialization expression here:
|
5 | var x;
| ⎺
|
The call to 'fn' occurs before any valid initialization points:
|
6 | fn(x);
| ⎺
|
The call to 'fn' cannot initialize 'x' because only 'out' formals can be used to split-initialize. However, 'x' is passed to formal 'arg' which has intent 'const'.

─── error in noMatchingCandidates.chpl:14 [NoMatchingCandidates] ───
Unable to resolve call to 'fn': no matching candidates.
|
14 | fn(x);
|

The following candidate didn't match because an actual couldn't be passed to a formal:
|
10 | proc fn(arg:string) {
| ⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺
11 | arg;
12 | }
|
The formal 'arg' expects a value of type 'string', but the actual was a value of type 'int(64)'.
|
14 | fn(x);
| ⎺
|
Formals with kind 'const ref' expect the actual to be a subtype, but 'int(64)' is not a subtype of 'string'.

Loading

0 comments on commit 49651a3

Please sign in to comment.