View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0000491||ascend||compiler||public||2011-03-01 11:59||2013-02-26 13:39|
|Target Version||0.9.9||Fixed in Version|
|Summary||0000491: need to raise error if invalid extrel name found in model|
|Description||Currently, the only error comes from 'visitinst.c' which warns that a NULL instance has been found.|
This error can occur for example if an old version of an external library is loaded, while the model expects a newer version of the library with some extra functions present.
No error has the result that relations are missing and the system is non-square: we need a more direct error to be triggered.
|Tags||No tags attached.|
this is probably best handled as a semantic check in typedef.c rather
than directly in the parser. syntactically the misuse is well-formed,
but some loaded .so has presumably restricted the number arguments
so the 'parse' succeeds, but the definition is rejected at the typedef
checks stage with a precise error message.
If this doesn't make sense for some reason or you need a pointer to
some routines in typedef, let me know. I'm in an all-day class
thur/fri this week, so hard to get hold of.
On Wed, Mar 2, 2011 at 11:18 PM, John Pye <firstname.lastname@example.org> wrote:
> > Hi Ben
> > I am working on one of the last bugs I want to fix before our 0.9.8
> > release which I want to make to coincide with our GSOC application.
> > The bug is this one:
> > http://ascendbugs.cheme.cmu.edu/view.php?id=491
> > As part of that I am reproducing my earlier python test suite for
> > blackbox into the CUnit test suite.
> > I found that one of the first test cases is not doing what I expected at
> > the C layer:
> > http://ascendcode.cheme.cmu.edu/viewvc/code/trunk/ascend/compiler/test/test_blackbox.c?view=markup
> > The model being parsed is this one:
> > http://ascendcode.cheme.cmu.edu/viewvc/code/trunk/models/test/blackbox/parsefail1.a4c?revision=2258&view=markup
> > But this should give some kind of error when the model is being loaded,
> > because the blackbox knows that the statement contains the wrong number
> > of arguments.
> > Should that result in an error code being returned from zz_parse, or
> > from SimsCreateInstance, or is there some other way that I should be
> > tracking that error?
> > I also need to think about how we correctly track an attempt to
> > reference an invalidly-named external relation... that should also
> > happen during compilation or instantiation, not sure which.
> > The GUI doesn't currently do a good job of trapping this kind of error,
> > so I want to fix that.
> > Cheers
> > JP
maybe it is inappropriate to use the zz_parse routine in ascParse.y as the means for returning errors about incorrect invocation of blackbox external relations.
maybe we can instead use some checks during the Instantiate call, to check for various errors in the blackbox invocation (wrong number of parameters/args, missing blackbox function name, etc).
still not clear about the mechnanism by which we can return these errors properly to the user.
This is a follow up to some earlier discussion we had about bug 0000491.
This is the failure in the compiler_blackbox test suite relating to checking for mis-calls to blackbox external relations as per the models in models/test/blackbox, and the test suite ‘compiler_blackbox’. It seems that we have no robust way of catching instantiation errors relating to wrong parameter lists on blackboxes. I have pasted the output below.
What is happening here is that somewhere inside Pass2InstantiateModel, an error is being thrown by the initialization function for the black box (saying that the number of input/output etc arguments is not correct). The error is being caught by Pass2ExecuteBlackBoxEXTLoop. Some code around there says “/* don't report an error here, because return 1 is possible even with eventual success at later compiler passes. */”. The error code (1) is returned though, and it should basically propagate back up the chain. I can follow the error code back up to “Pass2ExecuteRelationStatements”, but at this point, our function seems to swallow error codes, and not report. All it outputs in response to an error is whether or not something is “*changed”, and sets a bitfield value (this setting of a bit value doesn’t seem to bubble back up anywhere, at least in the form of any further error messages).
Do you have a suggestion for how to correctly return and error flag from the black box instantiation process, so that we can write CUnit tests that reliably catch when something is wrong with these blackbox instances? Perhaps there is some kind of error-checking routine that I can call that I haven’t been calling up to now?
I checked that “NumberPendingInstances” is not a solution to this problem. There is the earlier comment that visitinst.c alerts us about a NULL instance being found, but by then I think it might be too late. In any case, it seems that the “Instantiate” routine should really check this for the user…?
I think you previously mentioned that I should address this one by some explicit TypeDesc checks. You don’t think we can do this as part of the instantiation process somehow? What TypeDesc checks do you think we need, and where would be the efficient place to insert them? Is something in the Instantiate process not going to be more efficient with this?
Test: parsefail1 ...ascend\compiler\test\test_blackbox.c:67 (load_model): Parse completed
ascend\compiler\instantiate.c:12371 (NewInstantiateModel): starting...
ascend\compiler\instantiate.c:12384 (NewInstantiateModel): Phase 1 models = 0
ascend\compiler\instantiate.c:12184 (Pass2InstantiateModel): starting...
ascend\compiler\instantiate.c:11547 (Pass2ProcessPendingInstancesAnon): Classification: 0 (for relation sharing)
ERROR: models\test\blackbox\bboxtest.c:234:CheckArgsOK: Number of arguments does not match the external function prototype(array_of_realatom[set],array_of_realatom[set],real_constant
models\test\blackbox\bboxtest.c:150 (bboxtest_preslv): Problem with arguments
PROGRAM ERROR: ascend\compiler\instantiate.c:5511:Pass2ExecuteBlackBoxEXTLoop: Error in blackbox initfn
ascend\compiler\instantiate.c:11346 (Pass2ExecuteRelationStatements): Got error code here, clearing bit in blist, setting '*changed' to 1
ascend\compiler\instantiate.c:11590 (Pass2ProcessPendingInstancesAnon): Making tokens: 39 (for relations)
ascend\compiler\instantiate.c:11597 (Pass2ProcessPendingInstancesAnon): build/link: 0 (for bintokens)
ascend\compiler\instantiate.c:12198 (Pass2InstantiateModel): Relations in the instance U 0 + C 0 = T 0.
ascend\compiler\instantiate.c:12211 (Pass2InstantiateModel): ...done
ascend\compiler\instantiate.c:12414 (NewInstantiateModel): Phase 2 relations = 97
ascend\compiler\instantiate.c:12437 (NewInstantiateModel): Phase 3 logicals = 8
ascend\compiler\instantiate.c:12453 (NewInstantiateModel): Phase 4 when-case = 11
ascend\compiler\instantiate.c:12467 (NewInstantiateModel): Phase 5 defaults = 9
ascend\compiler\instantiate.c:12486 (NewInstantiateModel): Total = 125
ascend\compiler\test\test_blackbox.c:80:Got new failure 'sim==NULL' in suite 'compiler_blackbox'
1. ascend\compiler\test\test_blackbox.c:80 - sim==NULL
I can’t remember the whole checking conversation right now for bbox args. Whatever I said was probably right… ;-)
Typedesc checking is always more efficient than instantiation checking, but the kinds of things that can be checked are limited.
In general, the instantiator only has a very small set of things it can conclusively prove are incorrect for all possible scenarios.
The best we can probably do ad hoc would be to keep a new global blackbox processing warning message list and display it at the end of phase 2.
|2011-03-01 11:59||john||New Issue|
|2011-03-01 11:59||john||Status||new => assigned|
|2011-03-01 11:59||john||Assigned To||=> john|
|2011-03-04 15:19||john||Note Added: 0000685|
|2011-03-07 12:00||john||Note Added: 0000687|
|2011-03-30 09:42||john||Target Version||0.9.8 => 1.0|
|2012-01-31 11:17||john||Note Added: 0000808|
|2012-01-31 11:18||john||Note Added: 0000809|
|2013-02-26 13:39||john||Target Version||1.0 => 0.9.9|