require type checking on CustomConst

[ss2165]

Closes #318

[acl-cqc]

Ok - so the I-have-some-YAML implementation of this (maybe there are two such: one which embeds a type, one without - I mean the one with) says:
if typ == myEmbeddedType {Ok(())} else {Err(....)}
right?

You could make this return String for errors, and then wrap those in CustomCheckFail when we call this? It'd make it clear whose code an error was coming from...

[ss2165]

part one: yes

part two: yes, but with some wrapping i think, will do

[ss2165]

ah no actually, CustomCheckFail is only there as a catch all in case the existing error modes don't match up, in most cases ValueCheckFail is the correct mode and can be reused.

[acl-cqc]

If there's no way for a CustomConst to synthesize its type, there's no way to tell that a CustomConst is Hashable or otherwise. If we don't want to add a synthesizing method, do we want to add a method returning a TypeTag ?

[acl-cqc]

We don't need such (#322 will work without), but that doesn't mean we shouldn't have it ;). I guess it could return Option, and maybe should, whereupon we can't rely on it, so it doesn't reduce complexity (we still have to handle all the cases).

[acl-cqc]

Closes #318 this now makes it impossible for use of static values of types from resources that do not provide binary CustomConst.

Well, they can use serde_yaml, right? At present that probably doesn't all work, but that'll come shortly, I think the next PR after this. And how was it possible to have static values w/out binary CustomConst before - surely you'd have to provide an implementation of CustomConst, with custom_type() method?

Is this what we want?

Thing is that if we don't have a check_type then we need to be able to get back to the TypeDef. (I had been thinking of this for YAML...). So either embed a CustomType (as before this PR), or a name, or something.

So I think it is, but I haven't really understood the drawback, or the alternative.

This may be better to go in after #322, tho.

[acl-cqc]

On the error type (continued from #325 (comment)). The thing is that ValueCheckFail is the least informative message. It tells you something failed, but nothing as to why.

At the same time, it's also one of the most informative (perhaps alongside TypeMismatch), in that it does at least tell you which part of the value/type didn't match up. But really, this is available in all cases (it's the value and typ of the innermost call to value.check_type(typ)).

So, ideally, I'd like to see every message include the value and type, so ValueCheckFailed really is then "no further information". Whether we do that by adding value+type to each, or by structuring ConstTypeError differently (maybe as a struct, value + type + "enum FurtherDetails"), or what, I don't know. But that's not for this PR.

In this PR, well, I think it would be strange for an impl CustomConst to return a ConstTypeError::TupleWrongLength, put it that way. How about you make CustomCheckFail include value and type, and string message (or Option<String>). The trait method CustomConst::check_type (maybe rename to check_custom given the arguments are different) then returns that string message, and the calling code (in typecheck_const or wherever) then wraps that up with the value+type of the innermost call.

The situation where this fails I guess is if your impl CustomConst itself calls typecheck_const and wants to feed that error back out. Well, heck, you could even have a ConstTypeError::CustomCheckFailed (or variant) with a nested/inner ConstTypeError, if you wanted...

[acl-cqc]

Looks great, thanks. I like struct CustomCheckFail!

Happy for you to remove ConstTypeError::TypeMismatch too