[V78B] {DESKTOP} Possible method optimization bug.

Monkey Forums/Monkey Bug Reports/[V78B] {DESKTOP} Possible method optimization bug.

ImmutableOctet(SKNG)

(Posted 2014) [#1]

So because of Monkey's whole compile only what's used thing, I went so long without noticing this bug.

Basically, I wrote my own vector classes a while ago, and obviously commands like 'Add', 'Subtract', and 'DotProduct' were needed, but apparently Monkey's GLFW/Desktop target doesn't like two specific methods I made. Essentially my vector is just a nice wrapper for arrays, and because of this, I back most operations by array-based implementations.

So, when I write something like this:

	Method Subtract:Void(V:AbstractVector<T>)
		Subtract(V.Data)
		
		Return
	End
	
	Method Subtract:Void(A:T[], ALength:Int=A.Length())
		For Local Index:Int = 0 Until Min(Data.Length(), ALength)
			Data[Index] -= A[Index]
		Next
		
		Return
	End

I expect Monkey to either optimize out that last bit when I never bother with the second argument, or just leave it as-is when compiling.

But apparently Trans decided to have conflicted ideals, and just try both:

void c_AbstractVector::p_Subtract(Array<Float > t_A,int t_ALength){
	DBG_ENTER("AbstractVector.Subtract")
	c_AbstractVector *self=this;
	DBG_LOCAL(self,"Self")
	DBG_LOCAL(t_A,"A")
	DBG_LOCAL(t_ALength,"ALength")
	DBG_INFO("E:/Other/Programming Languages/Monkey/Modules/vector/system.monkey<157>");
	for(int t_Index=0;t_Index<bb_math2_Min(m_Data.Length(),t_ALength);t_Index=t_Index+1){
		DBG_BLOCK();
		DBG_LOCAL(t_Index,"Index")
		DBG_INFO("E:/Other/Programming Languages/Monkey/Modules/vector/system.monkey<158>");
		m_Data.At(t_Index)-=t_A.At(t_Index);
	}
	return;
}

void c_AbstractVector::p_Subtract3(c_AbstractVector* t_V){
	DBG_ENTER("AbstractVector.Subtract")
	c_AbstractVector *self=this;
	DBG_LOCAL(self,"Self")
	DBG_LOCAL(t_V,"V")
	DBG_INFO("E:/Other/Programming Languages/Monkey/Modules/vector/system.monkey<151>");
	p_Subtract(t_V->m_Data,t_A.Length()); // HINT: 't_A' doesn't actually exist.
	return;
}

Here's the output from Trans:

TRANS monkey compiler V1.65
Parsing...
Semanting...
Translating...
Building...
g++  -Wno-free-nonheap-object -I../glfw/include -I../glfw/lib -I../glfw/lib/win32 -I../openal/include -I../stb  -c -o ../main.o ../main.cpp
../main.cpp: In member function 'void c_AbstractVector::p_Subtract3(c_AbstractVector*)':
../main.cpp:11042:25: error: 't_A' was not declared in this scope
  p_Subtract(t_V->m_Data,t_A.Length());
                         ^
../main.cpp: In member function 'Float c_AbstractVector::p_DotProduct2(c_AbstractVector*)':
../main.cpp:11070:36: error: 't_A' was not declared in this scope
  Float t_=p_DotProduct(t_V->m_Data,t_A.Length());
                                    ^
<builtin>: recipe for target '../main.o' failed
mingw32-make: *** [../main.o] Error 1
TRANS FAILED: Error executing 'mingw32-make CCOPTS="-Wno-free-nonheap-object" OUT="Debug/MonkeyGame"', return code=2

I have no idea how this happened, but I was able to easily fix it by just adding the length explicitly. That being said, this seems to be a problem with Trans. Or I'm not supposed to use a default-argument like that, but that doesn't really make sense based on what I usually can do.

dawlane

(Posted 2014) [#2]

I think something like this did crop up before in another post. Have you test this with one of the conditional looping methods such as REPEAT/UNTIL and WHILE/WEND. I think FOR loops in many language expect values to be know before use.

Nobuyuki

(Posted 2014) [#3]

You can make optional parameters non-constant? I thought the compiler was supposed to stop you from trying that. It certainly stops me from specifying object instances.

ImmutableOctet(SKNG)

(Posted 2014) [#4]

@dawlane It's a weird argument optimization thing, so I don't think you're on the same page here.

@Nobuyuki Yes, you surprisingly can, and it seems to work on all targets. The best example I can think of is 'DefaultFlags' for the 'Image' class in Mojo. I'm not too sure about arrays though, but I haven't actually had an issue with doing this until now.

marksibly

(Posted 2014) [#5]

> Method Subtract:Void(A:T[], ALength:Int=A.Length())

Default expressions aren't really supposed to be able to access parameters like this - I'll patch the compiler to deal with it.