Parser meaningful error for wrong type definition

[Laurie Nevay]

If you define a region with

someName: region, prodCutPhotons=1*m;

The parser will give an error - unrecognised option prodCutPhotons. This is actually quite misleading and took me a while to spot this mistake is actually 'region' is wrong and 'cutsregion' is the 'type' of object that should be used.

It would be preferable for the parser to indicate this if possible.

I may also look at the requirement of 'region' being reserved as it's much more intuitive.

[Jochem Snuverink]

Interesting, I'll have a look.

[Jochem Snuverink]

the reason it complains like this, is because there is an option to define a new element type (which I don't know if it ever worked and we don't have an example, but I never removed it).

So the parser sees it now that you define a new element type "region" with the name "someName", and then since element has no prodCutPhotons option anymore it complains about it.

I see now that this is actually quite error-prone, and if you agree I will remove the option to define a new element type and put a meaningful error in instead.

[Laurie Nevay]

Thanks for looking into this! I see now. Is this feature required for inheritance of other objects? ie:

lhcdipole: sbend, magnetGeometryType="lhcleft";
d1: lhcdipole, l=14.3*m, angle=14*mrad;
d2: lhcdipole, l=7*m, angle=7*mrad;

That's the only concern - but I'm not familiar with the exact implementation of this part. I think as you suggest that feature should be removed - thanks.

[Jochem Snuverink]

ah yes you're right! I forgot this is the inheritance mechanism. Then we can't remove it. We could change the syntax of it perhaps with a keyword, so that it is less ambiguous, something like:

d2: inherit, lhcdipole, l=7m, angle=7mrad;

[Laurie Nevay]

Could we maintain the current behaviour but kill two birds with one stone?

d2: lhcdipole, l=14*m;

Gives error (with only this line used):

Error> no object / element "lhcdipole" defined

This would then work for region as well and would allow us to keep the inheritance syntax that matches madx just now.

[Jochem Snuverink]

That is a good idea and should be possible. I'll implement that.

[Jochem Snuverink]

actually note that that particular error message is already there, that line gives:

type lhcdipole has not been defined

The problem you had occurs when an parameter is added that is not part of element. While I thought it would be easy to get the more meaningful error first, it is not so straightforward because the parser is set up such that it works its way backwards.

[Jochem Snuverink]

to continue from the previous comment:

we will need to delay the exiting until the line is fully parsed (this may not always be possible, but in this case it will be). We could add a global boolean flag "willExit" that is checked after parsing a full line (statement ending in semicolon) and will exit if true. In this case (and in general) that will generate more meaningful output.

[Jochem Snuverink]

fix issue #161

→ <<cset c77774e0bc87>>

[Jochem Snuverink]

implemented as mentioned by delaying the exiting.

For now only in the Parameters struct as set_value will now not exit anymore if called by BDSIM classes. Right now there is no such call and preferably there shouldn't be. But we do so for the Options struct for example, so I didn't make it generic.