defined constants can conflict with callback pseudo-parameters

[bhaller]

In the course of investigating #573, I found another bug involving defined constants. Here's an example script:

initialize() {
	initializeMutationRate(1e-6);
	
	initializeMutationType("m1", 0.5, "f", 0.0);
	initializeGenomicElementType("g1", m1, 1.0);
	initializeGenomicElement(g1, 0, 99999);
	initializeRecombinationRate(1e-8);
	
	defineConstant("homozygous", 1:5);
}
1 early() {
	sim.addSubpop("p1", 50);
}
mutationEffect(m1) {
	if (homozygous)
		return 0.0;
	return effect;
}
20000 late() { }

The defined constant homozygous conflicts with the pseudo-parameter homozygous, and the local pseudo-parameter shadows the global defined constant. That is not the end of the world – it's better than if it were the other way around! – but according to the scoping rules it should not be allowed.

The question is, when should an error be raised about it? On entry to the mutationEffect() callback would be the logical spot, but I don't want to slow down the callback mechanism with checks like this; it needs to be as fast as possible. So the check needs to happen earlier. I think what needs to happen, actually, is that an error gets raised at the defineConstant() call, that homozygous has been reserved by the Context (i.e., SLiM) for its own use, and so it cannot be used as the name of a defined constant. Then SLiM can reserve all the names it uses, and this conflict is avoided. That's a little over-aggressive, since the homozygous is reserved even if the script doesn't contain a mutationEffect() callback, and so a conflict would never actually occur; but it seems better to just prohibit it, rather than have a script with homozygous work fine until such time as the user adds a mutationEffect() callback, and then suddenly the use of homozygous errors; that would kinda feel like "spooky action at a distance".

[petrelharp]

Oh dear. This change is going to break some scripts.

Would it be reasonable to have a list of (callback type, prohibited keyword)s? So that for instance homozygous is only prohibited if there is a mutationEffect callback in the script? That would avoid breaking all the scripts that use the homozygous for a variable name somewhere? I guess that is complicated because there might not be a callback at runtime but one gets created later, grrr. And, oh, wait - the issue is only if homozygous is a constant (or global) name, right? That's not going to be as common.

[petrelharp]

The defined constant homozygous conflicts with the pseudo-parameter homozygous, and the local pseudo-parameter shadows the global defined constant. That is not the end of the world – it's better than if it were the other way around! – but according to the scoping rules it should not be allowed.

I think you meant this the other way around: according to Wikipedia, if X shadows Y then it's Y that takes precedence, and here it's the local pseudo-parameter that takes precedence (as we'd hope).

[bhaller]

I'd be surprised if it breaks anybody. This restriction only applies to defined globals and defined constants. Local variables do not need additional restrictions due to this issue. If somebody is defining a global constant named homozygous, well, I'm OK with breaking their script, because I really doubt people are doing that, and if they are, well, it's not a big deal to rename the variable. (And note that I've been recommending the use of uppercase for global constants and variables for years.)

[bhaller]

And, oh, wait - the issue is only if homozygous is a constant (or global) name, right? That's not going to be as common.

Yep, exactly.

[bhaller]

The defined constant homozygous conflicts with the pseudo-parameter homozygous, and the local pseudo-parameter shadows the global defined constant. That is not the end of the world – it's better than if it were the other way around! – but according to the scoping rules it should not be allowed.

I think you meant this the other way around: according to Wikipedia, if X shadows Y then it's Y that takes precedence, and here it's the local pseudo-parameter that takes precedence (as we'd hope).

Are you sure? Link please? I would say "X shadows Y" means "X places a shadow upon Y, obscuring it". One would also say "Y is shadowed by X". In both cases, it is X that is visible, and Y that is not. That's how I've always heard this terminology used, I think, although I could certainly be wrong. :->

Anyhow, maybe there's a preference as to whether the pseudo-parameter or the global constant "wins", but either way it is bug-prone and should be prevented. Eidos scoping rules say that global constants are not allowed to conflict with anything else; it is just too bug-prone. The scoping rules do not say the same about global variables, but I think I'm going to snip this particular conflict for them as well, because it just smells like a source of very confusing bugs, and there's no legitimate reason to want to do it, I think.

[bhaller]

[Found it in Wikipedia, and I think that discussion supports my terminology? They suggest "X masks Y" rather than "X shadows Y", but in my experience people say both, and both mean "Y is shadowed by X", meaning that it is X, not Y, that is used within the local context.]