Everything

php

a way to fix the import issue

php.internals

Greg Beaver

18 years ago

Hi all,

Benjamin has in fact unearthed a bug in the implementation of import.
This code:

<?php
namespace Foo;
import Blah::Exception;
$a = new Exception;
?>

should in fact be implicitly importing Blah::Exception as if it were
Foo::Exception, rather than as ::Exception.  In other words, I would
actually expect the above code to be equivalent to:

<?php
namespace Foo;
import Blah::Exception as Foo::Exception;
$a = new Foo::Exception;
?>

This is a simple fix.
http://lxr.php.net/source/ZendEngine2/zend_compile.c#5122 should be
using zend_do_build_namespace_name() with CG(current_namespace) to
retrieve the actual classname to import, rather than just the last part
after ::.  With this change, the example code above would work without
conflicting with either the built-in exception or Foo::Exception unless
Foo::Exception is included from another file, which would give the
correct error.

I'm afraid I don't have time for a proper patch right now, but it is a
very simple fix, we would just need to add a few lines to put the
classname zval and CG(current_namespace) into znodes and pass to
zend_do_build_namespace_name(), then extract the created zval for name.

Greg

Stanislav Malyshev

18 years ago

> <?php
> namespace Foo;
> import Blah::Exception;
> $a = new Exception;
> ?>
>
> should in fact be implicitly importing Blah::Exception as if it were
> Foo::Exception, rather than as ::Exception.  In other words, I would
> actually expect the above code to be equivalent to:
>
> <?php
> namespace Foo;
> import Blah::Exception as Foo::Exception;
> $a = new Foo::Exception;
> ?>

No, not really. It's equivalent to:
<?
$a = new Blah::Exception();
?>

There's no class named Foo::Exception and import does not create it.
However, I think I see what you were meaning - that new Exception should
refer to Blah::Exception. I think it is true. Dmitry, could you look
into it? I.e. unqualified lookups inside namespace should also take
imports into account.

--
Stanislav Malyshev, Zend Software Architect
stas@zend.com   http://www.zend.com/
(408)253-8829   MSN: stas@zend.com

Dmitry Stogov

18 years ago

In your example :

<?php
namespace Foo;
import Blah::Exception;
$a = new Exception;
?>

"new Exception" refer to "Blah::Exception" and will fail if such class
doesn't exists.

"import Blah::Exception" creates an alias with short name "Exception" only
for current file (it doesn't creates "Foo::Exception")

May be I didn't understood the question. :)

Thanks. Dmitry.

Greg Beaver

18 years ago

Dmitry Stogov wrote:

> In your example :
>
> <?php
> namespace Foo;
> import Blah::Exception;
> $a = new Exception;
> ?>
>
> "new Exception" refer to "Blah::Exception" and will fail if such class
> doesn't exists.
>
> "import Blah::Exception" creates an alias with short name "Exception" only
> for current file (it doesn't creates "Foo::Exception")
>
> May be I didn't understood the question. :)

Yes, you do misunderstand I think :)

testme.php:
<?php
namespace Blah;
class Exception extends ::Exception {}
?>

test.php:
<?php
namespace Foo;
include 'testme.php';
import Blah::Exception;
$a = new Exception;
?>

result:
cellog@lot-49:~/workspace/php5$ sapi/cli/php -n test.php

Fatal error: Import name 'Exception' conflicts with defined class in
/home/cellog/workspace/php5/test.php on line 4

This is because of this check:

if (zend_hash_exists(CG(class_table), lcname, Z_STRLEN_P(name)+1)) {
zend_error(E_COMPILE_ERROR, "Import name '%s' conflicts with defined
class", Z_STRVAL_P(name));
}

we are comparing "Exception" to "Exception" in the class table, and so
we get a fatal error.  We should instead transparently import
Blah::Exception not as "Exception" but as "Foo::Exception" *only* for
the above comparison check.  In other words, import needs to honor
namespace when checking for class naming conflicts.

If you can create class "Exception" inside namespace Foo and refer to it
as "Exception" then import Blah::Exception should also allow referring
to "Exception" *if* "Foo::Exception" doesn't already exist.

Greg

Greg Beaver

18 years ago

Dmitry Stogov wrote:

> In your example :
>
> <?php
> namespace Foo;
> import Blah::Exception;
> $a = new Exception;
> ?>
>
> "new Exception" refer to "Blah::Exception" and will fail if such class
> doesn't exists.
>
> "import Blah::Exception" creates an alias with short name "Exception" only
> for current file (it doesn't creates "Foo::Exception")
>
> May be I didn't understood the question. :)

http://bugs.php.net/42859 has a further example

with patches for PHP 6 and PHP 5 in the report.

Greg

Dmitry Stogov

18 years ago

Hi Gregory,

It seems, your bug report and patches are right.
I'll look into them more careful little bit later.
Thank you very much.

Dmitry.