Manage statement exitContext with try-catch?

Users
1

Hello everyone,

I’m working on a PR for #17277 to add a way to assert if a procedure throws an error, and I think the standard idea with a proc assertThrows(someProc, errType: Error?, ...args?) will work well.

But I thought it’d be cool to have, rather than the above, something like

manage test.raises(SomeError) {
  someProc(arg1, arg2=arg2); // throws SomeError
}
// OR
manage test.raises(DivideByZeroError) {
  var bar = 1.0 / 0.0;
}

This is similar to pytest.raises and would be helpful if the user wanted to test if a procedure throws an error while still allowing them to use keyword arguments (handy as # of args grows), or even a group of statements.

Unfortunately, I’m still a bit green with chapel and especially about the rules of using a manage statement. I’m struggling to get the following to compile:

module Foo {

  class AssertionError: Error {  // thanks UnitTest
    proc init(msg: string = "") {
      super.init(msg);
    }
  }

  class RaiseErrorManager: contextManager {
    type errType;

    proc enterContext() {}

    proc exitContext(in err: owned Error?) throws {
      try {
        if err then throw err;
        throw new owned AssertionError("manage stmt didn't throw any error!");
      }
      catch e: errType {  // anticipated error
        return;
      }
      catch e {  // unanticipated error
        throw e;
      }
    }
  }

  class SomeError : Error {
    proc init() {}
  }


  proc main() {

    proc foo() throws {
      throw new owned SomeError();
    }

    manage new owned RaiseErrorManager(SomeError) {
      foo();
    }
  }
}

I’m not sure if contextManager supports a generic subclass like this (I was unable to return a type from enterContext as a usable resource), or if try-catch is possible in the exitContext?

External Issue: [Bug]: generic with try-catch handling
2

Hi @jmag722, a few things to note here.

First of all, I think using a context manager for this is quite elegant and nicely avoids the FCF problem with assertThrows. I think this idea has promise, but you should be aware of some of the edge cases around context managers and throwing errors, see [Feature Request]: Adjust the `manage` statement to better handle `throws` · Issue #28148 · chapel-lang/chapel · GitHub.

As two sort of design nits:

  • I think it makes more sense for RaiseErrorManager to be a record, not a class. You don't really need the features a class will provide (unless I missed some subtlety here) and a record will have lower memory impact and better usability for users of this interface.
  • While I think this is great, I do want to make sure you know that this won't work for things like divide by zero. Those kinds of errors are halts, which result in uncatchable program exit. To handle that we need a different mechanism. Just spitballing, one idea is to follow rusts approach and annotate the test function has "should panic". Then an external test runner like mason test can check if it panics or not.

As far as getting this to compile, you have definitely found a bug here. The bug actually doesn't have to do with context managers, its with try/catch and generics

record R {
  type T;

  proc doit() {
    try! {
      throw new T();
    } catch e: T {
      writeln("caught error of type ", T:string);
    }
  }
}
var r = new R(Error);
r.doit();

This code fails to compile with the same error as your snippet.

I also found a slightly different error when creating that reproducer

proc foo(type T) {
  try! {
    throw new T();
  } catch e: T {
    writeln("caught error of type ", T:string);
  }
}
foo(Error);

This also fails to compile when it really should.

Unfortunately, I don't yet have a good workaround for you. I definitely encourage you to open issues for those bugs (or I can on your behalf if you prefer) to see if we can get these bugs patched soon.

-Jade

1 Like
3

Putting more examples here as I try and find a workaround

This example with a record also doesn't work, but it gets past the internal compiler error

record R {
  type T;

  proc doit() {
    type locT = T;
    try! {
      throw new locT();
    } catch e: locT {
      writeln("caught error of type ", T:string);
    }
  }
}
var r = new R(Error);
r.doit();

This error has to do with copy initializers, and by using a class we can avoid some of it

Using a class gets the same error as the simple function case

class R {
  type T;

  proc doit() {
    type locT = T;
    try! {
      throw new locT();
    } catch e: locT { // error: mismatched number of arguments in call to 'chpl_error'
      writeln("caught error of type ", T:string);
    }
  }
}
var r = new R(Error);
r.doit();

Also note that using this pattern of a local type alias works around the issue in the context manager case, but then there is a bug with the compiler inserting unprotected calls to exitContext.

module Foo {

  class AssertionError: Error {  // thanks UnitTest
    proc init(msg: string = "") {
      super.init(msg);
    }
  }

  class RaiseErrorManager: contextManager { // error: call to throwing function exitContext without throws, try, or try! (relaxed mode)
    type errType;

    proc enterContext() {}

    proc exitContext(in err: owned Error?) throws {
      type ET = errType;
      try {
        if err then throw err;
        throw new owned AssertionError("manage stmt didn't throw any error!");
      }
      catch e: ET {  // anticipated error
        writeln("caught SomeError as expected: ", e.message());
        return;
      }
      catch e {  // unanticipated error
        throw e;
      }
    }
  }

  class SomeError : Error {
    proc init() {}
  }

  proc main(){

    proc foo() throws {
      throw new SomeError();
    }

    try! {
      manage new RaiseErrorManager(SomeError) {
        foo();
      }
    }
  }
}

Factoring out that error code also doesn't work

module Foo {

  class AssertionError: Error {  // thanks UnitTest
    proc init(msg: string = "") {
      super.init(msg);
    }
  }

  proc checkError(type T, in err: owned Error?) throws {
    type ET = T;
    try {
      if err then throw err;
      throw new owned AssertionError("manage stmt didn't throw any error!");
    }
    catch e: ET {  // anticipated error
      writeln("caught SomeError as expected: ", e.message());
      return;
    }
    catch e {  // unanticipated error
      throw e;
    }
  }

  class RaiseErrorManager: contextManager {
    type errType;

    proc enterContext() {}

    proc exitContext(in err: owned Error?) throws {
      try checkError(errType, err);
    }
  }

  class SomeError : Error {
    proc init() {}
  }

  proc main(){

    proc foo() throws {
      throw new SomeError();
    }

    try! {
      manage new RaiseErrorManager(SomeError) {
        foo();
      }
    }
  }
}

This seems very related to [Feature Request]: Adjust the `manage` statement to better handle `throws` · Issue #28148 · chapel-lang/chapel · GitHub, and likely [Bug]: throwing hash functions generate an unavoidable error in error handling relaxed mode · Issue #24882 · chapel-lang/chapel · GitHub too

-Jade

1 Like
4

Hi @jmag722 , thanks filing this bug report!

And thank you Jade for the quick investigation and reproducers here. I’d be interested in investigating these compiler errors, unless you’re set on them.

1 Like
5

Thank you Jade! Not opposed to using a record either, I need to better understand the use case for each haha. And I guess that DivideByZeroError would need to test for that type of condition separately in an if statement or something if a halt isn’t catchable.

I’ve gone ahead and created an issue for this.

1 Like