Check for bad terms in fast notation in get_variables

[jodavies]

These created numerators of 0. Fixes #567

[coveralls]

coverage: 50.107%. remained the same
when pulling 362336d on jodavies:issue-567
into 8599601 on vermaseren:master.

[tueda]

Should this check (*e < 0) be in poly::argument_to_poly() or poly::get_variables()?

[jodavies]

You are right, this check could equally go in get_variables and makes more sense there. I will move it.

[tueda]

Now the code looks fine.

Optionally, the code for triggering the error could be reused with goto, like:

		else if (*e < 0) {
term_too_complicated_error:
			MLOCK(ErrorMessageLock);
			MesPrint("ERROR: polynomials and polyratfuns must contain symbols only");
			MUNLOCK(ErrorMessageLock);
			Terminate(1);
		}
		else {
			for (int i=with_arghead ? ARGHEAD : 0; with_arghead ? i<e[0] : e[i]!=0; i+=e[i]) {
				if (i+1<i+e[i]-ABS(e[i+e[i]-1]) && e[i+1]!=SYMBOL) {
					goto term_too_complicated_error;
				}

[jodavies]

While here it would probably be fine, other areas of the code have been cumbersome to debug because you have to determine which of many goto have actually caused a termination. In my opinion it could be beneficial to actually remove this pattern from the code entirely, but that requires some effort also...

[vermaseren]

How about giving Terminate a second parameter that indicates which call it was? Then one can change things easily, using grep.

…

On 8 Nov 2024, at 10:07, jodavies ***@***.***> wrote: While here it would probably be fine, other areas of the code have been cumbersome to debug because you have to determine which of many goto have actually caused a termination. In my opinion it could be beneficial to actually remove this pattern from the code entirely, but that requires some effort also... — Reply to this email directly, view it on GitHub <#570 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABJPCEU344ZMQ7VLOMIODO3Z7R5O7AVCNFSM6AAAAABRIMFKASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINRUGE4DKMRUGI>. You are receiving this because you are subscribed to this thread.

[jodavies]

When this PR is done #526 , Terminate is replaced with a macro which prints the file and line information of every terminate call.

But still if you have several goto which land at a Terminate, you can't know which goto caused the crash without stepping through in a debugger (hard if the crash is in a long running script with no useful way to set a breakpoint) or putting print statements at each of the goto.