Elseif is evaluated when is not needed

[fezfez]

namespace ZephirBug;

class bug1
{
    public function whatsisMyvar(myvar)
    {
        if is_string(myvar) {
            return "is a var";
        } elseif is_string(myvar[0]) {
            return "is an array and the first element is a string";
        }

        return "unkown";
    }
}

<?php
use ZephirBug\bug1;

$tmp = new bug1();

var_dump($tmp->whatsisMyvar("a string"));
var_dump($tmp->whatsisMyvar(['a string']));
var_dump($tmp->whatsisMyvar(10));

Expected output :

string(8) "is a var"
string(45) "is an array and the first element is a string"
string(6) "unkown"

Actual output :

PHP Notice:  Cannot use a scalar value as an array in zephirbug/bug1.zep on line 9 in bug1.php on line 6
string(8) "is a var"
string(45) "is an array and the first element is a string"
PHP Notice:  Cannot use a scalar value as an array in zephirbug/bug1.zep on line 9 in bug1.php on line 8
string(6) "unkown"

Zephir version 0.8.0a

[kse300489]

I confirm, current version and lower versions has this bug

[ovr]

Working on it!

[ovr]

Problem is here

Because, Compiler is trying to optimize conditions for (for/elseif/if) in IfStatement

but it write code before first if

[sergeyklay]

@fezfez

Zephir isn't a full copy of PHP. Particularly, it isn't a fully dynamic language. Though it supports some dynamic structures.

Often when you write your program on Zephir, the habit of programming in the manner of PHP only harms. You've faced exactly with a case in point. The problem here is the following:

When you write

        if is_string(myvar) {
            return "is a var";
        } elseif is_string(myvar[0]) {
            return "is an array and the first element is a string";
        }

you expect that the code would be executed in linear fashion, literally from top to bottom. Not all languages function in such manner. I'd like to give you an example in PHP:

TRUE || some_non_existent_function_here();

In this case (on PHP) the error will apparently never occur. The explanation here is simple: because of its dynamic nature PHP won’t resolve the expression in the second part of the condition, since the first part of that expression is TRUE. However, there is a pitfall here — you will never get to know about errors of such kind until you run the given above (broken) code.

Zephir, like other static languages, uses static typing checking. This method is in common use in programming languages. One of the most popular aspects of static typing is strong typing when the variable type can’t be changed later. It means that the compiler “knows” which particular variable type will be used. In dynamic typing, all types are determined during the program execution.

In short: In a statically typed language, type checking is performed at compile time; in a dynamically typed language type checking is performed at run time. In practice, weakly typed languages are usually dynamically typed.

Yes, for the record, we added type checking into Zephir like:

function foo (array parameter) {

}

But you use the dynamic type. Note the:

function foo (parameter) {

}

the same as:

function foo (var parameter) {

}

On top of everything else, Zephir tries to optimize code even before the compilation stage. The optimization process is crucially important. It enables creating programs with higher performing rate or lower memory consuming.

Note that when you use instructions of type myvar[0] you likely admit that a variable could be an array. As you get the value by index you admit that it is possible. But, of course, you are not sure of it.

Put it otherwise, you consider the operand type as a variable one, that means, it can be either a string, or an array. The problem is that Zephir hasn't got information about it. Zephir isn't able to figure out or suppose. If it sees that you use instructions of type myvar[0] it believes you. After all, you are the programmer, and you do write the code.

By optimizing the instruction of type is_string(myvar[0]) Zephir tries to build more efficient code:

// Initialize the &_0 with value of myvar[0]
zephir_array_fetch_long(&_0, myvar, 0, ...);

// ...
if (Z_TYPE_P(&_0) == IS_STRING) {
    // ...
}

Actually, when you come to think of it, you could write something like the following:

if  is_string(myvar[0]) {
    // ...
} else if is_int(myvar[0]) {
    // ...
} else if is_bool(myvar[0]) {
    // ...
} // ...

As you understand we can't fetch the value in each condition. Frankly speaking it is expensive. This particular hinders your code. The optimized code is put before conditional operators and the error occurs in this very string.

I've rewritten your code using pseudo code for a good understanding. It may seem that the program is executed this way:

1. if VAR is string
2. do work
3. otherwise
4. check first element of the VAR

But in fact it is executed this way:

1. initialize the BAR variable from the firts index of VAR
2. if VAR is string
3. do work
4. otherwise
5. check the BAR from #1

---

P.S. I don't think that we'll find enough time in the near future to invent a trick which could help with solving similar problems. Actually such solution would be quite expensive. I recommend to avoid using such approach as you described in your example. Instead, you can use something like this:

public function whatsisMyvar(myvar)
{
    if is_array(myvar) {
        return this->inspectArrayVar(myvar);
    } else if is_object(myvar) {
        return this->inspectObject(myvar);
    }

    // ...
}

or better with using switch/case approach.

Thank you for the report, and for helping us make Zephir better.

Cc: @coollider

[kse300489]

Ok. Pls add to documentation that elseif not supported. And beeter disable elseif in parser. This bug create much problems.

[sergeyklay]

Actually there is no problem with control flow like else if. Please read carefully my answer.

[kse300489]

No. This is problem, when code executed when it not must executed, it is exactly bug. If you can't do that else if currently working, convert it to nested if. Or disable in parser on show error when user use else if. I spent a lot of time with debugging and analyzing generated C code to find this bug.

[sergeyklay]

It is not a bug. It is a ugly coding style borrowed from PHP. Why do you call:

is_string(myvar[0])

Are you sure that this is array? No. Why you do not use:

is_array(myvar) && isset(myvar[0]) && is_string(myvar[0])

Actually it is language feature - attempt to optimize code before compilation. And please don't mix static and dynamic language approaches. Do you want to use dynamic approach? No problem, but Zephir does not support Pattern Matching yet. So you have implement it by hand.

What do you think about this code:

var a = "5";

if is_string(a) {
    return "string";
} else if is_array(a) {
    return "array";
}

• Is there a problem with elseif? No.
• Is there elseif not supported? No, it is supported.
• Is elseif is evaluated when is not needed? No.

[Jurigag]

But what he talks about here @sergeyklay that else if condition shouldn't be executed at all if first condition is true. Just method is not ending after return point. Simple as that.

[Ultimater]

In my opinion, the only concern here is the PHP Notices. In production you'd simply disable them anyways. But nobody wants a PHP extension throwing notices. Honestly, let people write better zephir code so no notices are generated in the first place. Alternatively, it wouldn't be a breaking change if we simply revised the way zephir handles elseif to do such in a safer fashion so it doesn't throw notices in the first place. In PHP there exists a @ for silencing such things. Perhaps there's a way we could use it from elseif?

[sergeyklay]

public function whatsisMyvar(myvar)
{
    if is_string(myvar) {
        return this->inspectString(myvar);
    } elseif is_array(myvar) {
        return this->inspectArray(myvar);
    } elseif ( /* ... */ ) {
        // ..
    } 
}

Do the magic without programming the programming language.

[sergeyklay]

Well, this is very tricky to fix, won't happen soon. If anyone wants to get started, I'd love that.