Skip to content

Commit

Permalink
Dyno: emit error for failed implicit 'super' calls (chapel-lang#26630)
Browse files Browse the repository at this point in the history
Closes Cray/chapel-private#6722, which asks to
emit an error when a call to 'super' is implicitly inserted but fails to
resolve:

```Chapel
class Parent {
  var x : int = 42;
  proc init(x: int) {
    this.x = x;
  }
}

class Child : Parent {
  var y : string;
  proc init() {
    // tries to resolve 'super.init()', but parent 'init' requires an argument!
    this.y = x:string;
  }
}
```

This PR performs a relatively major re-architecture of the way that
calls are resolved and noted as part of Dyno's resolver, which reduces
the effort (at call sites) to handle the results of these calls. This
stemmed from the observation that a variety of resolver functions for
call resolution have nearly 10 arguments, and thread through the same
state that's passed in when a call is first resolved. For instance, a
user might perform `resolveGeneratedCall`, then call
`handleResolvedCallPrintCandidates,` which will in turn call
`handleResolvedCallWithoutError`, etc. To invoke these helpers, the user
would need to re-pass the call info, scope information, a boolean
indicating if the call was generated, and much more to these functions.
Not only that, but these invocations to `handle*` were not always
performed, even though they are instrumental to tracking `compilerError`
information and noting PoI scope info.

This PR creates wrappers around the various call resolution procedures
from `resolution-queries.h` that bundle together the result of resolving
the call with the information needed to note it etc., via a new type.
This new type exposes methods analogous to
`handleResolvedCallPrintCandidates`, except they do not require
re-threading through of the 10 arguments that were previously needed.

This new type also has some state (in the form of `callName` and
`reportFunction`) which can be used to customize the behavior of error
reporting. In the case of the new error message added by this PR, the
reporting function is used to issue a `NoMatchingSuper` error, which
contains additional data and is used for a very detailed explanation of
what went wrong.

![Screen Shot 2025-01-30 at 4 50 33
PM](https://github.com/user-attachments/assets/32f60509-00ba-4b15-b2fc-badefe4abb90)

Reviewed by @benharsh -- thanks!

## Testing
- [x] dyno. tests
- [x] paratest
  • Loading branch information
DanilaFe authored Feb 3, 2025
2 parents 7998ae0 + 867b9b3 commit 1914f5f
Show file tree
Hide file tree
Showing 9 changed files with 506 additions and 270 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ ERROR_CLASS(MultipleInheritance, const uast::Class*, const uast::AstNode*, const
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>, std::vector<const uast::VarLikeDecl*>)
ERROR_CLASS(NoMatchingSuper, const uast::AstNode*, resolution::CallInfo, std::vector<resolution::ApplicabilityResult>, std::vector<const uast::VarLikeDecl*>, std::vector<std::pair<ID, ID>>)
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
34 changes: 30 additions & 4 deletions frontend/lib/resolution/InitResolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,28 @@ void InitResolver::resolveImplicitSuperInit() {
actuals.push_back(CallInfoActual(superQT, USTR("this")));
auto ci = CallInfo(USTR("init"), superQT, true, false, false, actuals);
auto inScopes = CallScopeInfo::forNormalCall(initResolver_.scopeStack.back(), initResolver_.poiScope);
auto c = resolveGeneratedCall(ctx_, fn_->body()->child(0), ci, inScopes);

updateSuperType(&c);
auto callContext = fn_->body();
auto c = initResolver_.resolveGeneratedCall(callContext, &ci, &inScopes);
c.callName = "super.init";
c.reportError = [this](const Resolver::CallResultWrapper& result,
std::vector<ApplicabilityResult>& rejected,
std::vector<const uast::VarLikeDecl*>& actualDecls) {
CHPL_REPORT(result.parent->context,
NoMatchingSuper, result.astForContext, *result.ci,
rejected, actualDecls, this->useOfSuperFields_);
};

// Capture the errors emitted here and defer them until we know we haven't
// found a "real" super.init call (which means a different error supercedes
// these).
auto cAndErrors = ctx_->runAndTrackErrors([&c](Context* ctx) {
c.noteResultPrintCandidates(nullptr);
return true;
});

errorsFromImplicitSuperInit = std::move(cAndErrors.errors());
updateSuperType(&c.result);
}
}

Expand Down Expand Up @@ -590,6 +609,10 @@ InitResolver::computeTypedSignature(const Type* newRecvType) {
const TypedFnSignature* InitResolver::finalize(void) {
if (fn_ == nullptr) CHPL_ASSERT(false && "Not handled yet!");

for (auto& error : errorsFromImplicitSuperInit) {
ctx_->report(std::move(error));
}

auto ret = initResolver_.typedSignature;

if (phase_ < PHASE_COMPLETE) {
Expand Down Expand Up @@ -758,6 +781,7 @@ bool InitResolver::handleCallToSuperInit(const FnCall* node,
updateSuperType(c);

this->explicitSuperInit = true;
this->errorsFromImplicitSuperInit.clear();

return true;
}
Expand Down Expand Up @@ -1015,12 +1039,14 @@ bool InitResolver::handleUseOfField(const AstNode* node) {
if (isSuperField) {
if (explicitSuperInit == false && phase_ != PHASE_COMPLETE) {
// Upon first use of parent field, need to insert a super.init() call
bool firstUseOfField = useOfSuperFields_.empty();
useOfSuperFields_.push_back({id, node->id()});

// TODO: store this as an associated action
if (useOfSuperFields_.empty()) {
if (firstUseOfField) {
this->resolveImplicitSuperInit();
}

useOfSuperFields_.push_back({id, node->id()});
return true;
} else {
isValidPreInitMention = true;
Expand Down
3 changes: 3 additions & 0 deletions frontend/lib/resolution/InitResolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ class InitResolver {
// Uses of parent fields before a super.init is seen.
// Stores field ID and ID of the uAST referencing the field.
std::vector<std::pair<ID, ID>> useOfSuperFields_;
// These errors are captured but deferred until we know we haven't found
// a "real" super.init call.
std::vector<owned<ErrorBase>> errorsFromImplicitSuperInit;

//initialization points to guide handling `=` operators
std::set<const uast::AstNode*> initPoints;
Expand Down
Loading

0 comments on commit 1914f5f

Please sign in to comment.