[Chapel Merge] Fixes to error handling with iterators

Notifications Commits
1

Branch: refs/heads/main
Revision: 475fa20
Author: vasslitvinov
Link: Unavailable
Log Message:

Merge pull request #19291 from vasslitvinov/iterator-error-handling

Fixes to error handling with iterators

This PR improves the handling of errors within a "halting" context,
i.e., when the throwing expression - either a throw statement or a
call of a throwing function - occurs outside of a "try" context, i.e.,
a try or try! block or else in a function that throws. It makes
the behavior more consistent with the "immediate" option in
#19388. For example, an error thrown in a "halting" context in an
iterator is now reported as "uncaught" in the iterator, not in the
loop over the iterator. An error thrown in a "halting" context in a
forall statement is reported as "uncaught" where it is thrown, not at
the forall statement. The PR fixes some bugs and resolves #19378.

IMPLEMENTATION DETAILS

A key change is to tune up decision making in
ErrorHandlingVisitor::enterCallExpr() where it chooses between
propagating the thrown error to the function's error_out, to an
enclosing try block, or halding immediately.

  • Calls to throwing functions in a halting context are lowered during
    the lowerErrorHandling pass by adding calls to
    chpl_uncaught_error, not chpl_propagate_error, as it used to be.

  • Calls to _waitEndCount() are a special case, as this function
    propagates errors from the associated task functions. They used to
    be lowered in a halting context always by adding calls
    chpl_propagate_error, like all other throwing functions. This PR
    changes that to call:

    • chpl_uncaught_error() when the corresponding source code
      appears lexically within a procedure, or
    • chpl_propagate_error() when it is within an iterator.

    Background: chpl_uncaught_error always causes the program to
    halt. chpl_propagate_error also always causes the program to halt,
    however it may be replaced during lowerIterators / iterator inlining
    to propagating the exception to a now-enclosing try block or
    throwing function.

  • Task functions that correspond to task constructs that, in the
    source code, occur in a halting context, are marked with
    FLAG_OUTSIDE_TRY. This is easiest to do before the task constructs
    get converted to task functions, so I put it in createTaskFunctions.

  • The new FLAG_FORALL_BREAK_LABEL helps find the forall statement's
    error handler just like FLAG_ERROR_LABEL helps find other handlers
    in handleChplPropagateErrorCall(), i.e., when lowering calls to
    chpl_propagate_error() / gChplPropagateError. These are inserted in
    lowerErrorHandling() to pass error handling information to
    lowerIterators(), where they are lowered away. The behavior of
    handleChplPropagateErrorCall() remains unchanged unless we are
    lowering a forall statement. This reduces the chance of introducing
    unwanted behavior.

  • The changes to the tests in
    test/parallel/forall/task-private-vars/throwing-init-exprs/
    are aimed to preserve the previously-observed behavior so that their
    (numerous) .good files do not need to change.

  • I added comments based on what I learned.

FUTURE WORK

(f1) Clarify the error handling section of the language spec
Error Handling — Chapel Documentation 1.26
that the relationship between a thrown error and its try/try! block is
lexical, not dynamic. "Lexical" is mentioned in one case and omitted
in another case. This PR and this writeup assume this is the desired
semantics.

(f2) Have a try block within a parallel iterator handle only the
exceptions thrown within its lexical scope, not those thrown in the
loop body. For example:

proc myIter(param tag) where tag == iterKind.standalone {
  try {
coforall ... {
  functionThatThrows();
  yield ...;
}
  }
}

When this iterator is invoked in a forall statement under the current
implementation, the above try-block will handle both the exceptions
thrown by functionThatThrows() and those thrown in the loop body of
the forall.

(f3) Report correct line numbers when an error is handled or not
handled in an iterator that is inlined. For example:

proc myIter(param tag) where tag == iterKind.standalone {
  ...
  functionThatThrows(); // assume no surrounding 'try'
  ...
}

When this iterator is invoked in a forall statement and the program
reports during execution that an exception is thrown at line NNN and
uncaught at line MMM, the line MMM points into the forall statement,
whereas it should point into myIter().

This problem is due to the inlining-based implementation of loops,
whereby the line numbers associated with the iterator code are
replaced with the loop's line number upon inlining the iterator into
the loop. For the same reason if we were stepping through the iterator
code in the debugger, it would be pointing at the loop line number,
not the iterator line numbers. Iterator line numbers can be observed
in both of these scenarios if compiling with --preserve-inlined-line-numbers.

(f4) Make it a compile-time error to have something thrown in a
defer statement -- until such time that we figure out how to make
the program behavior reasonable (and implement it). Currently it may
be a runtime error or might even propagate the error upstream.

r: @dlongnecke-cray

Modified Files:
A test/errhandling/parallel/forall-iterator-catches-1.bad

A test/errhandling/parallel/forall-iterator-catches-1.chpl
A test/errhandling/parallel/forall-iterator-catches-1.future
A test/errhandling/parallel/forall-iterator-catches-1.good
A test/errhandling/parallel/forall-iterator-catches-2.chpl
A test/errhandling/parallel/forall-iterator-catches-2.good
A test/errhandling/parallel/forall-iterator-throws-prototype.chpl
A test/errhandling/parallel/forall-iterator-throws-prototype.good
A test/errhandling/parallel/forall-throws-prototype.chpl
A test/errhandling/parallel/forall-throws-prototype.good
R test/errhandling/parallel/forall-in-function-throws.bad
R test/errhandling/parallel/forall-in-function-throws.future
M compiler/dyno/include/chpl/uast/PragmaList.h
M compiler/include/passes.h
M compiler/passes/createTaskFunctions.cpp
M compiler/passes/errorHandling.cpp
M compiler/resolution/lowerForalls.cpp
M compiler/resolution/lowerIterators.cpp
M test/parallel/forall/task-private-vars/throwing-init-exprs/int-no-tlvy.chpl
M test/parallel/forall/task-private-vars/throwing-init-exprs/int-tpvs.chpl
M test/parallel/forall/task-private-vars/throwing-init-exprs/nonpod-tpvs.good
M test/parallel/forall/task-private-vars/throwing-init-exprs/pod-no-tlvy.chpl
M test/parallel/forall/task-private-vars/throwing-init-exprs/pod-tpvs.chpl

Compare: https://github.com/chapel-lang/chapel/compare/f4fd786c0e49...475fa208f4bd