Workaround an issue preventing users to catch errors from

[e-kayrakli]

JSON.fromJson used the pattern:

return reader.read(MyType);

this results in calling a compiler-generated deserialization initializer that does not throw. While working on Arkouda checkpointing that led to writing code in which there are uncatchable errors. I worked this around in Arkouda for the time being by having my own fromJson-like helper.

This PR adjusts the standard fromJson to use the following pattern, from which an error can be thrown and caught as expected:

var ret: MyType;
reader.read(ret);
return ret;

Test:

• local

[benharsh]

Could you add a warning to the docs about this requirement? Or maybe a compilerWarning guarded by !isDefaultInitializable(loadAs)?

[e-kayrakli]

Hmm... it appears that this PR added a test in which the type is not default-initializable. I don't know immediately why a simple-looking owned is not default-initializable, but this PR breaks that test because of that. I am also pinging @DanilaFe as the person who worked on this feature to see if he has any opinions on the matter.

(And that test is the only one that breaks on this branch)

[benharsh]

I think it's not default-initializable because it's non-nilable

[e-kayrakli]

I think it's not default-initializable because it's non-nilable

That was my assumption, but I couldn't really rationalize it. Default-initializability should be about the fields of the base type and not about the modifiers applied to it. But I haven't thought too deeply. Do you have a rationale in mind?

[benharsh]

Do you have a rationale in mind?

I think the ultimate answer here is that this is just how we've designed the language. You can't write something like var x : owned Foo; either.

To rationalize it, by not indicating a class as nilable, we are asserting that it will always be a value of an initialized class. That initialization has to come from somewhere, and we don't default-initialize class instances.

[e-kayrakli]

It looks like there are two alternatives here, then:

1. We do not merge this PR. JSON errors remain not catchable.
2. We put the new implementation in only for default initializable types, where the end result is that a JSON error can be caught if we are creating a default-initializable type, and can't be caught otherwise.

Assuming that this is already in the bug category to me, (2) seems that it can reduce the impact of the bug. I can create a bug report for the remaining case where the type is non-default-initializable. Does that make sense? That would be addressing your inline comment in a slightly different way, as well.

[benharsh]

I think something in the docs would be good, otherwise I'm fine with this going in. If you make a bug report, can you assign it to me?

I've been trying to think of ways to improve the situation here, but nothing so far. Perhaps I could figure something out for records, but classes complicate things quite a bit.