Bug? Inline function locals/params shadow namespace members - Palette.text resolves to local text

[Christoph Hart]

@David-Healey hmm a namespace can contain only const regs or inline functions…

[David Healey]

@Christoph-Hart What about regular functions? (paint routines, broadcasters, and callbacks I'm thinking)

[David Healey]

@Christoph-Hart Changing return parseFactor(nullptr); to return parseFactor(ns); solves the crash but I don't know if that negates the purpose of the validator.

[Christoph Hart]

@David-Healey normal functions are var declarations in disguise and previously leaked to the global namespace without warning - that‘s one of the reasons why I added the yellow warning stuff

[David Healey]

@Christoph-Hart Does that mean no paint routines, mouse callbacks, etc. are allowed in namespaces?

[David Healey]

I think I've narrowed it down.

I have a namespace called CoreLookAndFeel that is included with every project. And it contains "overrides" like this

		pnlBody.setPaintRoutine(function(g)
		{
			if (isDefined(LookAndFeel.drawBody))
				return LookAndFeel.drawBody();
	
			var a = this.getLocalBounds(0);

			g.fillAll(this.get("bgColour"));
	
			if (!isDefined(LookAndFeel.style.useNoise) || LookAndFeel.style.useNoise)
				g.addNoise({alpha: 0.025, scaleFactor: 2.0, area: obj.area, monochromatic: true});
		});

In the project that's crashing I do have a LookAndFeel namespace, but it doesn't contain a drawBody function or a style object. I think that's the trigger for the crash I'm seeing.

[Christoph Hart]

@David-Healey ah so basically Namespace.nonexistingProperty crashes?

[David Healey]

@Christoph-Hart That's what I thought, but it's inconsistent. I'll spend some more time poking it and see if I can get a reproducible example.

[David Healey]

@Christoph-Hart Just doing a little test and it seems that inline functions are still leaking?

[David Healey]

@Christoph-Hart Yep that's the issue. The isDefined returns the wrong result so the script then tries to call a function that doesn't exist.

namespace FirstNameSpace
{
}

namespace SecondNameSpace
{
	inline function myFunc()
	{
		if (isDefined(FirstNameSpace.myFunc))
			FirstNameSpace.myFunc();
	}	
}

SecondNameSpace.myFunc();

[Christoph Hart]

@David-Healey ah yes that's a bit more complicated - the resolver for the first namespace doesn't work so it basically ignores the namespace qualifier and resolves the function call to itself so it creates a recursive loop.

In order to reproduce the faulty behavior without the crash you can do this:

namespace FirstNameSpace
{
	const var myFunc2 = 90;

}

namespace SecondNameSpace
{
	inline function myFunc()
	{
		if (isDefined(FirstNameSpace.myFunc))
		{
			Console.print("exists");
		}
		else
		{
			Console.print("doesn't exist");
		}
			
	}	
}

SecondNameSpace.myFunc();

The suggested fix of passing in the current namespace is not a real fix, it might solve your immediate problem, but the correct option would be to return an undefined expression like this:

match(TokenTypes::identifier);
return parseSuffixes(new Expression(location));

then this code would work as expected (returning "doesn't exist"). However this might silently break existing code that used this construct to falsely refer to non-namespaces data through a namespace qualifier so I'm a bit hesitant to push this.

[David Healey]

@Christoph-Hart I've merged the new commit and it no longer crashes, just shows some ugly errors. Is this intended as a stop-gap solution? Because isDefined() still returns true for the none-existent function.

Edit: Oh I see, it just stops the execution at the "error"

Tried your suggestion and appears to be working here, maybe a preprocessor definition (default true) to protect older buggy projects?