Bug 353218

Comment 1 Scott Greer 2011-07-27 10:56:43 EDT

Created attachment 200456 [details]
FVT testcase

Comment 2 Matt Heitz 2011-08-12 16:30:25 EDT

Jeff, it looks like you wrote ReorganizeCode so I'm hoping you can look into this before your trip.

I noticed that we treat Ifs differently than other statements which might contain expressions that use && or ||.  For example, the two lines below don't suffer from this problem because we don't pull out their function calls.

while(addAndReturnFalse(1) && addAndReturnFalse(2)) end
b boolean = addAndReturnFalse(1) && addAndReturnFalse(2);

Comment 3 Jeff Douglas 2011-08-12 18:04:34 EDT

Matt,

It's not true that while and boolean assignment don't have this problem. In fact they do.

Take this test case:

	function addAndReturnFalse(a int inout) returns(boolean)
		return (true);
	end
	
    	a int;
    	while(addAndReturnFalse(a) && addAndReturnFalse(a)) end
		b boolean = addAndReturnFalse(a) && addAndReturnFalse(a);
		if (addAndReturnFalse(a) && addAndReturnFalse(a)) end


Because the function being called has an inout parm, it means that the argument being passed must be replaced on function exit. If you generate that logic, you will see that unBox assignments are being done.

I also think that we need to break apart these additional statements because if the 1st part of the condition was false, then the next part should not be executed.

I think that while and boolean assignment must fall into the same logic as IF.

I'll try to work on this, but it is complicated.

Comment 4 Matt Heitz 2011-08-14 09:39:33 EDT

Oh, I guess I missed that (while and assignment to boolean are broken in the same way).  It's actually a good thing, I think.  It means you might be able deal with this situation by looking at BinaryExpressions not every place one of them might show up (If/While/etc.).

Comment 5 Jeff Douglas 2011-08-15 09:22:33 EDT

I've changed some code with this. It is better, but it still places the ezeUnbox statements for inout and out args, after the statement being processed. This means these statements would be placed after an IF, or WHILE or RETURN, etc. This is not really what is supposed to happen.

This is a complex problem. I know of only 2 solutions to it:

1) we could make the usage of invoking any function that has any inout or out parms to be used in an assignment statement only. In other words, give an error if the user tries to use a function with inout or out, that is in any other type of statement. That would completely eliminate the problem.

2) either the reorg code or the IR creation code would have to break up any of these types of expressions, into a series of IF statements. This would be very complicated and prone to errors.

Comment 6 Matt Heitz 2011-08-15 10:49:07 EDT

The first solution suggested in comment 5 is too restrictive.

Also, I was right in comment 2: the function calls in an If statement's condition are broken out, but that doesn't happen in all other cases.


function returnInout(parm int inout) returns (boolean) end

z1, z2 int;
if(returnInout(z1) && returnInout(z2)) end      // Both funcs always called.
while(returnInout(z1) && returnInout(z2)) end   // No problem.
b boolean = returnInout(z1) && returnInout(z2); // No problem.

Comment 7 Scott Greer 2011-09-12 09:14:54 EDT

I sort of lost track of this one;  is a fix still required, and if so, shouldn't we implement it soon for such a significant fix?

Comment 8 Matt Heitz 2011-09-12 09:20:00 EDT

Yeah, this really does need to be fixed.  Please figure out if the various problems mentioned here have been fixed or not.

Comment 9 Jeff Douglas 2011-09-24 14:55:22 EDT

Paul is supposed to write a preprocessing utility that will break out binary expressions into components. All of these problems are the same and will be solved once that utility has been written.

Comment 10 Jeff Douglas 2011-10-01 16:41:41 EDT

*** This bug has been marked as a duplicate of bug 357820 ***