Open bradcray opened 3 years ago
@mppf / @dlongnecke-cray: I was originally optimistic that David Longnecker's PR this week would improve the situation for these patterns as well (since they originated from a user), but no such luck. I was curious whether it was clear to either of you what would be required to resolve them and whether it's a heavy or light lift. Thanks.
It's crazy to me that there was already a compiler error (in the form of inserting a runtime error) for this case, and we just didn't hit it because we were segfaulting instead.
I'm going to have to learn a bit more about array runtime types to contribute to this effort.
Conjecture:
From reading lowerRuntimeTypeInit
in the compiler, it seems like the problem is that the tuple _defaultOf
has no way to access the RTT of its array elements, and thus can't default initialize them.
So it seems like we'd have to generate a second _defaultOf
or the like that is passed the runtime type as an argument to use when initializing the tuple/record?
It's crazy to me that there was already a compiler error (in the form of inserting a runtime error) for this case, and we just didn't hit it because we were segfaulting instead.
I assumed that the compiler error was added after the segfault was filed (since I never filed a future to track the behavior), but admittedly, I didn't compare the timestamps.
Right, the compiler doesn't currently track runtime types for type fields of aggregates. For example, in record R { type t; }
, it could be instantiated with an array - but the record type itself won't contain a runtime type. It has been this way for a very long time. We have a bunch of other issues about this - #15929 #11549.
Of course, we can try to plug the issue by making specific fixes (e.g. in this case, have the compiler track the runtime type information for the tuple and pass that to defaultOf) but I think the more principled solution is to adjust the compiler to start considering types containing runtime types to have runtime types. Barring that, #11549 proposes that we at least make these cases into compilation errors.
but I think the more principled solution is to adjust the compiler to start considering types containing runtime types to have runtime types
I was just writing a post to say after thinking about it a bit more, this is the only thing that makes sense to do!
E.g.
proc foo() type {
return ([0, 1, 2], [0, 1, 2]);
}
// The only way foo() can communicate a dynamic type is if it has one as well, right?
var t: foo();
I agree that this is the way, thanks for explaining the situation Michael. In ancient history, there was a routine in module code that generated the runtime type for a given type (and it was used in a weird way by the compiler to generate both type and value information). Is that still the case do you know? If so, I wonder whether we can just have the compiler generate more overloads of it or write a fancy version of it which uses Reflection to iterate over fields, recurse, etc. (that'd be a stress test for reflection...).
Is that still the case do you know?
Yes
If so, I wonder whether we can just have the compiler generate more overloads of it
I think it's probably going to go better to have the compiler directly support such runtime types but you could say I am biased in that direction. But anyway, yes, we could conceptually think about generating those same functions.
Summary of Problem
[this is an update to issue #15759, whose behavior has improved from a segfault to a user-facing error]
Chapel currently doesn't support default initialization of tuples of arrays. This is something that we should ultimately support for the sake of orthogonality. I originally ran into this issue while looking into @dataPulverizer's https://github.com/dataPulverizer/KernelMatrixBenchmark effort. At that time, these patterns resulted in segfaults, where now they result in the execution-time error:
This is definitely an improvement, but having them work would be even better.
Steps to Reproduce
Associated Future Test(s):
test/arrays/bugs/arrOfTupleOfArr2.chpl
#17051test/arrays/bugs/arrOfTupleOfArr3.chpl
#17051test/arrays/bugs/arrOfTupleOfArr4.chpl
#17051test/arrays/bugs/arrOfTupleOfArr5.chpl
#17051Configuration Information
chpl --version
:chpl version 1.24.0 pre-release (a721b8db24)