"wrapper functions for null function error function pointers" commit leads to segfaults

[GWRon]

Using bcc with the commit 600b543
leads to a segfault in my game and test applications (using parts of my game code):

Using the bcc revision of before that commit and everything works as expected.

The method it calls there has an overloaded variant, dunno if that can lead to it (need to investigate a bit tomorrow)
edit - nope, renamed the overloaded method and it still failed.

User hotcakes at our discord also mentioned something about the new bcc (weekly build):

this weeks build wasn't tagged by MSE but MaxIDE just doesn't boot for me :/ (including after a recompile (x86 release)) it runs and quits immediately
I see bcc was changed, copied the old version from 143.3.57.20240421 across and tried to recompile all modules and MaxIDE, works fine. Compiling with the new decl.bmx works fine of course, but whatever the new stuff in ctranslator is is using something that I'm guessing relies on a Windows 8 call probably for no good reason, so that's a shame.

It might be related to it.

[GWRon]

Seems the debugger was not able to "look into" the function so I dropped the content of that method call into that function.
Now it fails there:

This led me to the original source of the issue.

This is a short sample code leading to the error:

SuperStrict
Framework Brl.StandardIO


Type TTest
	Field valueModFunc:Double(value:Double)

	Method GetValue:Double(value:Double)
		if not valueModFunc
			return DefaultModFunc(value)
		else
			return valueModFunc(value)
		endif
	End Method


	Function DefaultModFunc:Double(value:Double)
		return 10 * value
	End Function
End Type


Local t:TTest= New TTest

print t.GetValue(123)

So it fails to identify a non assigned function pointer in a field ?

[GWRon]

The issue is still immanent. People experience issues (with the weekly builds) with MaxIDE and bcc for some weeks now.

I think we should at least kinda "discuss" how to approach the issue (if required) or maybe @woollybah could write if he is already investigating (simply assign the issue to you @woollybah ).

As I provided a short "failing" example it should be at least possible to replicate it on your own computers.

[HurryStarfish]

I am a bit confused as to how 600b543 is meant to work. I might be misunderstanding something here but as far as I can tell, it introduces wrapper functions for the Null function, resulting on Null being a different value for every function type. But doing this without huge breaking changes would require a whole lot of adjustments to the compiler which I'm not seeing in that commit.
Apart from = Null checks and boolean conversions (which seems to be the problem @GWRon is experiencing above), this affects every conversion between different function types as well as conversions between function types and pointer types.

Local f1:Int()
Local f2(s:String)
Local p1: Byte Ptr = f1
Local p2: Byte Ptr = f2
Print p1 = p2

still needs to work. So do similar comparisons in Extern functions whenever BlitzMax functions are passed to them, e.g.

Extern "C"
	Function Compare:Int(f1:Int(), f2(s:String))
	Function IsNullFunc:Int(f:Byte Ptr)
End Extern

Local f1:Int()
Local f2(s:String)
Print Compare(f1, f2)
Print IsNullFunc(f1)
Print IsNullFunc(f2)

#include <brl.mod/blitz.mod/blitz.h>

BBINT Compare(void (*f1)(), void (*f2)()) {
	return f1 == f2;
}

BBINT IsNullFunc(void (*f)()) {
	return f == &brl_blitz_NullFunctionError;
}

which has been working since all the way back in legacy BlitzMax.

Isn't there be an easier way to fix the casting issues in #659 that doesn't require all this?

[HurryStarfish]

On a related note, while writing the above I noticed that function type variance is currently broken. That is an unrelated bug, but such conversions would be relevant here as well (If f = f_ ...).