18825, "bmcdonald3", "[design] Reach consensus on halting/throwing in data structure methods", "2021-12-09T00:13:46Z"
Summary
Currently, map, list, and heap (and possibly others) all halt when attempting to access an element that does not exist in the data structure (e.g., map.getValue(val) halts if val is not in the map). This seems like limiting behavior, especially in a parallel context where the value may no longer exist in the time from checking if it exists with if map.contains(val) to getting the value with map.getValue(val). It would be great if we could set a precedence for what to do in these cases.
The 3 main questions
If we could converge on a precedence for:
- Halt or throw in a non-parallel safe method?
- Halt or throw in a parallel safe method?
- Should all such methods be in conformity, or is it alright to have a few that diverge from the pattern?
then that would be valuable for setting user expectations and when making decisions around the limitations/possibilities around adding new features, etc. to the modules. This should also bring some clarity to the parSafe discussion, where if we want different behavior that could bring some clarity there.
(lots of) Proposals
A. Halt in both parSafe and not parSafe contexts
This is what we have today currently. A couple of benefits with this approach are (1) the code looks cleaner to the caller (i.e., no try! or error handling) and (2) the code is more performant due to folding out the halts when compiled with --fast.
The checking being folded out when compiled with --fast could also be viewed as a downside, however, since the behavior is undefined if something does go wrong, which could result in some silent incorrect errors.
B. Throw in both parSafe and not parSafe contexts
The main benefit that I have been able to pick out here is (1) Users can decide what happens when their code does something unexpected, so it won't just crash their code
C. Throw in parallel safe context, halt in non-parallel safe context
Support for this case would support the position to move to 2 distinct parSafe and not parSafe data structures.
A benefit of this approach is that we are able to optimize our data structures in the non-parallel safe context where the undefined accesses are less likely, while still ensuring that errors can be handled in the non-parallel safe context.
D. Halt in parallel safe context, throw in non-parallel safe context
Support for this case would support the position to move to 2 distinct parSafe and not parSafe data structures. I can't think of a good argument for this approach, but including in case someone does.
E. Have no consensus and allow methods to either halt or throw based on individual judgements
This approach would allow greater flexibility in implementation, but would potentially add quite a bit of additional design discussion (which may not be ideal, depending on how you look at it...)
Related discussions
User issue: New argument in map.getValue() · Issue #17498 · chapel-lang/chapel · GitHub
Discussion of adding a throw method to map, which breaks current precedence: Adding an overload to `map.getValue()` to avoid halting · Issue #18786 · chapel-lang/chapel · GitHub
Parsafe discussion: [Map Module Review] Create proposals for serial/parallel/distributed maps · Issue #18494 · chapel-lang/chapel · GitHub